From 35d282ce09af0fed75857489c62fa51123cd3bc2 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Fri, 13 May 2022 17:14:27 -0700 Subject: [PATCH 1/5] Testing perf of reverting the move of `resolve_path` from `rustc_buitin_macros` to `rustc_expand`. --- Cargo.lock | 2 +- compiler/rustc_builtin_macros/src/lib.rs | 2 +- .../rustc_builtin_macros/src/source_util.rs | 42 ++++++++++++++++++- compiler/rustc_expand/src/base.rs | 41 +----------------- compiler/rustc_passes/Cargo.toml | 2 +- .../rustc_passes/src/debugger_visualizer.rs | 2 +- 6 files changed, 47 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index afe1814de6720..5704816feda31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4183,9 +4183,9 @@ dependencies = [ "rustc_ast", "rustc_ast_pretty", "rustc_attr", + "rustc_builtin_macros", "rustc_data_structures", "rustc_errors", - "rustc_expand", "rustc_feature", "rustc_hir", "rustc_index", diff --git a/compiler/rustc_builtin_macros/src/lib.rs b/compiler/rustc_builtin_macros/src/lib.rs index 468ca7d7aa932..93481c58396fc 100644 --- a/compiler/rustc_builtin_macros/src/lib.rs +++ b/compiler/rustc_builtin_macros/src/lib.rs @@ -38,7 +38,7 @@ mod format; mod format_foreign; mod global_allocator; mod log_syntax; -mod source_util; +pub mod source_util; mod test; mod trace_macros; mod util; diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index 8bf3a0799b638..ea970fa918c00 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -3,15 +3,18 @@ use rustc_ast::ptr::P; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast_pretty::pprust; +use rustc_errors::PResult; use rustc_expand::base::{self, *}; use rustc_expand::module::DirOwnership; use rustc_parse::parser::{ForceCollect, Parser}; use rustc_parse::{self, new_parser_from_file}; use rustc_session::lint::builtin::INCOMPLETE_INCLUDE; +use rustc_session::parse::ParseSess; use rustc_span::symbol::Symbol; -use rustc_span::{self, Pos, Span}; +use rustc_span::{self, FileName, Pos, Span}; use smallvec::SmallVec; +use std::path::PathBuf; use std::rc::Rc; // These macros all relate to the file system; they either return @@ -223,3 +226,40 @@ pub fn expand_include_bytes( } } } + +/// Resolves a `path` mentioned inside Rust code, returning an absolute path. +/// +/// This unifies the logic used for resolving `include_X!`. +pub fn resolve_path( + parse_sess: &ParseSess, + path: impl Into, + span: Span, +) -> PResult<'_, PathBuf> { + let path = path.into(); + + // Relative paths are resolved relative to the file in which they are found + // after macro expansion (that is, they are unhygienic). + if !path.is_absolute() { + let callsite = span.source_callsite(); + let mut result = match parse_sess.source_map().span_to_filename(callsite) { + FileName::Real(name) => name + .into_local_path() + .expect("attempting to resolve a file path in an external file"), + FileName::DocTest(path, _) => path, + other => { + return Err(parse_sess.span_diagnostic.struct_span_err( + span, + &format!( + "cannot resolve relative path in non-file source `{}`", + parse_sess.source_map().filename_for_diagnostics(&other) + ), + )); + } + }; + result.pop(); + result.push(path); + Ok(result) + } else { + Ok(path) + } +} diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 9ea09f7d702ba..56c6b0cf7656b 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -10,7 +10,7 @@ use rustc_ast::{self as ast, Attribute, HasAttrs, Item, NodeId, PatKind}; use rustc_attr::{self as attr, Deprecation, Stability}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{self, Lrc}; -use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, PResult}; +use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan}; use rustc_lint_defs::builtin::PROC_MACRO_BACK_COMPAT; use rustc_lint_defs::BuiltinLintDiagnostics; use rustc_parse::{self, parser, to_token_stream, MACRO_ARGUMENTS}; @@ -20,7 +20,7 @@ use rustc_span::edition::Edition; use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId}; use rustc_span::source_map::SourceMap; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{FileName, Span, DUMMY_SP}; +use rustc_span::{Span, DUMMY_SP}; use smallvec::{smallvec, SmallVec}; use std::default::Default; @@ -1137,43 +1137,6 @@ impl<'a> ExtCtxt<'a> { } } -/// Resolves a `path` mentioned inside Rust code, returning an absolute path. -/// -/// This unifies the logic used for resolving `include_X!`. -pub fn resolve_path( - parse_sess: &ParseSess, - path: impl Into, - span: Span, -) -> PResult<'_, PathBuf> { - let path = path.into(); - - // Relative paths are resolved relative to the file in which they are found - // after macro expansion (that is, they are unhygienic). - if !path.is_absolute() { - let callsite = span.source_callsite(); - let mut result = match parse_sess.source_map().span_to_filename(callsite) { - FileName::Real(name) => name - .into_local_path() - .expect("attempting to resolve a file path in an external file"), - FileName::DocTest(path, _) => path, - other => { - return Err(parse_sess.span_diagnostic.struct_span_err( - span, - &format!( - "cannot resolve relative path in non-file source `{}`", - parse_sess.source_map().filename_for_diagnostics(&other) - ), - )); - } - }; - result.pop(); - result.push(path); - Ok(result) - } else { - Ok(path) - } -} - /// Extracts a string literal from the macro expanded version of `expr`, /// returning a diagnostic error of `err_msg` if `expr` is not a string literal. /// The returned bool indicates whether an applicable suggestion has already been diff --git a/compiler/rustc_passes/Cargo.toml b/compiler/rustc_passes/Cargo.toml index a3ef1981e84f9..c0eab32b26056 100644 --- a/compiler/rustc_passes/Cargo.toml +++ b/compiler/rustc_passes/Cargo.toml @@ -9,7 +9,7 @@ rustc_middle = { path = "../rustc_middle" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } -rustc_expand = { path = "../rustc_expand" } +rustc_builtin_macros = { path = "../rustc_builtin_macros" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } rustc_parse = { path = "../rustc_parse" } diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index f89092c57a37a..4813971e4c074 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -1,8 +1,8 @@ //! Detecting usage of the `#[debugger_visualizer]` attribute. use hir::CRATE_HIR_ID; +use rustc_builtin_macros::source_util::resolve_path; use rustc_data_structures::fx::FxHashSet; -use rustc_expand::base::resolve_path; use rustc_hir as hir; use rustc_hir::def_id::CrateNum; use rustc_hir::itemlikevisit::ItemLikeVisitor; From c1176899ecf74d35ad3e1e3f6fdedcc5d07ceb4b Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Mon, 16 May 2022 11:17:29 -0700 Subject: [PATCH 2/5] Revert changes of moving `resolve_path` function. Add 2 new symbols to test the overhead caused by adding new symbols. --- compiler/rustc_builtin_macros/src/lib.rs | 2 +- .../rustc_builtin_macros/src/source_util.rs | 42 +------------------ compiler/rustc_expand/src/base.rs | 41 +++++++++++++++++- compiler/rustc_passes/Cargo.toml | 2 +- .../rustc_passes/src/debugger_visualizer.rs | 2 +- compiler/rustc_span/src/symbol.rs | 2 + 6 files changed, 45 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/lib.rs b/compiler/rustc_builtin_macros/src/lib.rs index 93481c58396fc..468ca7d7aa932 100644 --- a/compiler/rustc_builtin_macros/src/lib.rs +++ b/compiler/rustc_builtin_macros/src/lib.rs @@ -38,7 +38,7 @@ mod format; mod format_foreign; mod global_allocator; mod log_syntax; -pub mod source_util; +mod source_util; mod test; mod trace_macros; mod util; diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index ea970fa918c00..8bf3a0799b638 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -3,18 +3,15 @@ use rustc_ast::ptr::P; use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast_pretty::pprust; -use rustc_errors::PResult; use rustc_expand::base::{self, *}; use rustc_expand::module::DirOwnership; use rustc_parse::parser::{ForceCollect, Parser}; use rustc_parse::{self, new_parser_from_file}; use rustc_session::lint::builtin::INCOMPLETE_INCLUDE; -use rustc_session::parse::ParseSess; use rustc_span::symbol::Symbol; -use rustc_span::{self, FileName, Pos, Span}; +use rustc_span::{self, Pos, Span}; use smallvec::SmallVec; -use std::path::PathBuf; use std::rc::Rc; // These macros all relate to the file system; they either return @@ -226,40 +223,3 @@ pub fn expand_include_bytes( } } } - -/// Resolves a `path` mentioned inside Rust code, returning an absolute path. -/// -/// This unifies the logic used for resolving `include_X!`. -pub fn resolve_path( - parse_sess: &ParseSess, - path: impl Into, - span: Span, -) -> PResult<'_, PathBuf> { - let path = path.into(); - - // Relative paths are resolved relative to the file in which they are found - // after macro expansion (that is, they are unhygienic). - if !path.is_absolute() { - let callsite = span.source_callsite(); - let mut result = match parse_sess.source_map().span_to_filename(callsite) { - FileName::Real(name) => name - .into_local_path() - .expect("attempting to resolve a file path in an external file"), - FileName::DocTest(path, _) => path, - other => { - return Err(parse_sess.span_diagnostic.struct_span_err( - span, - &format!( - "cannot resolve relative path in non-file source `{}`", - parse_sess.source_map().filename_for_diagnostics(&other) - ), - )); - } - }; - result.pop(); - result.push(path); - Ok(result) - } else { - Ok(path) - } -} diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 56c6b0cf7656b..9ea09f7d702ba 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -10,7 +10,7 @@ use rustc_ast::{self as ast, Attribute, HasAttrs, Item, NodeId, PatKind}; use rustc_attr::{self as attr, Deprecation, Stability}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{self, Lrc}; -use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan}; +use rustc_errors::{Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, PResult}; use rustc_lint_defs::builtin::PROC_MACRO_BACK_COMPAT; use rustc_lint_defs::BuiltinLintDiagnostics; use rustc_parse::{self, parser, to_token_stream, MACRO_ARGUMENTS}; @@ -20,7 +20,7 @@ use rustc_span::edition::Edition; use rustc_span::hygiene::{AstPass, ExpnData, ExpnKind, LocalExpnId}; use rustc_span::source_map::SourceMap; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::{FileName, Span, DUMMY_SP}; use smallvec::{smallvec, SmallVec}; use std::default::Default; @@ -1137,6 +1137,43 @@ impl<'a> ExtCtxt<'a> { } } +/// Resolves a `path` mentioned inside Rust code, returning an absolute path. +/// +/// This unifies the logic used for resolving `include_X!`. +pub fn resolve_path( + parse_sess: &ParseSess, + path: impl Into, + span: Span, +) -> PResult<'_, PathBuf> { + let path = path.into(); + + // Relative paths are resolved relative to the file in which they are found + // after macro expansion (that is, they are unhygienic). + if !path.is_absolute() { + let callsite = span.source_callsite(); + let mut result = match parse_sess.source_map().span_to_filename(callsite) { + FileName::Real(name) => name + .into_local_path() + .expect("attempting to resolve a file path in an external file"), + FileName::DocTest(path, _) => path, + other => { + return Err(parse_sess.span_diagnostic.struct_span_err( + span, + &format!( + "cannot resolve relative path in non-file source `{}`", + parse_sess.source_map().filename_for_diagnostics(&other) + ), + )); + } + }; + result.pop(); + result.push(path); + Ok(result) + } else { + Ok(path) + } +} + /// Extracts a string literal from the macro expanded version of `expr`, /// returning a diagnostic error of `err_msg` if `expr` is not a string literal. /// The returned bool indicates whether an applicable suggestion has already been diff --git a/compiler/rustc_passes/Cargo.toml b/compiler/rustc_passes/Cargo.toml index c0eab32b26056..a3ef1981e84f9 100644 --- a/compiler/rustc_passes/Cargo.toml +++ b/compiler/rustc_passes/Cargo.toml @@ -9,7 +9,7 @@ rustc_middle = { path = "../rustc_middle" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } rustc_errors = { path = "../rustc_errors" } -rustc_builtin_macros = { path = "../rustc_builtin_macros" } +rustc_expand = { path = "../rustc_expand" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } rustc_parse = { path = "../rustc_parse" } diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index 4813971e4c074..f89092c57a37a 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -1,8 +1,8 @@ //! Detecting usage of the `#[debugger_visualizer]` attribute. use hir::CRATE_HIR_ID; -use rustc_builtin_macros::source_util::resolve_path; use rustc_data_structures::fx::FxHashSet; +use rustc_expand::base::resolve_path; use rustc_hir as hir; use rustc_hir::def_id::CrateNum; use rustc_hir::itemlikevisit::ItemLikeVisitor; diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 2cc6eb0358567..6f281ce50386b 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -557,6 +557,7 @@ symbols! { debug_trait_builder, debug_tuple, debugger_visualizer, + debuggervisualizer, decl_macro, declare_lint_pass, decode, @@ -929,6 +930,7 @@ symbols! { native_link_modifiers_verbatim, native_link_modifiers_whole_archive, natvis_file, + natvisfile, ne, nearbyintf32, nearbyintf64, From 3c87c4d375e3810b9fbedb2ce9cce9c542692f51 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Mon, 16 May 2022 11:42:49 -0700 Subject: [PATCH 3/5] Update lock file. --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 5704816feda31..afe1814de6720 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4183,9 +4183,9 @@ dependencies = [ "rustc_ast", "rustc_ast_pretty", "rustc_attr", - "rustc_builtin_macros", "rustc_data_structures", "rustc_errors", + "rustc_expand", "rustc_feature", "rustc_hir", "rustc_index", From f78d7f8723c288e254f2a2940005ba5f136f2d48 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Tue, 17 May 2022 11:33:00 -0700 Subject: [PATCH 4/5] Revert adding additional symbols. --- compiler/rustc_span/src/symbol.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 6f281ce50386b..2cc6eb0358567 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -557,7 +557,6 @@ symbols! { debug_trait_builder, debug_tuple, debugger_visualizer, - debuggervisualizer, decl_macro, declare_lint_pass, decode, @@ -930,7 +929,6 @@ symbols! { native_link_modifiers_verbatim, native_link_modifiers_whole_archive, natvis_file, - natvisfile, ne, nearbyintf32, nearbyintf64, From 485e10558c1a5de4eeb0f8d28519ffb611e0ef29 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Tue, 17 May 2022 14:50:09 -0700 Subject: [PATCH 5/5] Update the debugger_visualizers to set `eval_always` to test performance implications. --- compiler/rustc_middle/src/query/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 6d7ec247d0452..7fe3caceb9f74 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1654,6 +1654,7 @@ rustc_queries! { /// Returns the debugger visualizers defined for this crate. query debugger_visualizers(_: CrateNum) -> Vec { storage(ArenaCacheSelector<'tcx>) + eval_always desc { "looking up the debugger visualizers for this crate" } separate_provide_extern }