From df9df19507ea4573ff141d253b2e95cd5b6cd0ff Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 6 Jan 2019 01:22:52 +0100 Subject: [PATCH 1/3] Always calculate glob map but only for glob uses Previously calculating glob map was *opt-in*, however it did record node id -> ident use for every use directive. This aims to see if we can unconditionally calculate the glob map and not regress performance. --- src/librustc/ty/mod.rs | 2 +- src/librustc_driver/driver.rs | 15 ++------------ src/librustc_driver/lib.rs | 2 -- src/librustc_driver/test.rs | 2 -- src/librustc_resolve/lib.rs | 23 +++++++--------------- src/librustc_save_analysis/dump_visitor.rs | 1 - src/librustc_save_analysis/lib.rs | 2 -- src/librustdoc/core.rs | 3 +-- src/librustdoc/test.rs | 2 -- 9 files changed, 11 insertions(+), 41 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index cfd99948e4370..26b4735d926a5 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -122,7 +122,7 @@ mod sty; /// *on-demand* infrastructure. #[derive(Clone)] pub struct CrateAnalysis { - pub glob_map: Option, + pub glob_map: hir::GlobMap, } #[derive(Clone)] diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index bfff592a5dd49..e026909d127cf 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -26,7 +26,7 @@ use rustc_passes::{self, ast_validation, hir_stats, loops, rvalue_promotion}; use rustc_plugin as plugin; use rustc_plugin::registry::Registry; use rustc_privacy; -use rustc_resolve::{MakeGlobMap, Resolver, ResolverArenas}; +use rustc_resolve::{Resolver, ResolverArenas}; use rustc_traits; use rustc_typeck as typeck; use syntax::{self, ast, attr, diagnostics, visit}; @@ -179,7 +179,6 @@ pub fn compile_input( registry, &crate_name, addl_plugins, - control.make_glob_map, |expanded_crate| { let mut state = CompileState::state_after_expand( input, @@ -394,7 +393,6 @@ pub struct CompileController<'a> { // FIXME we probably want to group the below options together and offer a // better API, rather than this ad-hoc approach. - pub make_glob_map: MakeGlobMap, // Whether the compiler should keep the ast beyond parsing. pub keep_ast: bool, // -Zcontinue-parse-after-error @@ -417,7 +415,6 @@ impl<'a> CompileController<'a> { after_hir_lowering: PhaseController::basic(), after_analysis: PhaseController::basic(), compilation_done: PhaseController::basic(), - make_glob_map: MakeGlobMap::No, keep_ast: false, continue_parse_after_error: false, provide: box |_| {}, @@ -739,7 +736,6 @@ pub fn phase_2_configure_and_expand( registry: Option, crate_name: &str, addl_plugins: Option>, - make_glob_map: MakeGlobMap, after_expand: F, ) -> Result where @@ -759,7 +755,6 @@ where registry, crate_name, addl_plugins, - make_glob_map, &resolver_arenas, &mut crate_loader, after_expand, @@ -785,11 +780,7 @@ where }, analysis: ty::CrateAnalysis { - glob_map: if resolver.make_glob_map { - Some(resolver.glob_map) - } else { - None - }, + glob_map: resolver.glob_map }, }), Err(x) => Err(x), @@ -805,7 +796,6 @@ pub fn phase_2_configure_and_expand_inner<'a, F>( registry: Option, crate_name: &str, addl_plugins: Option>, - make_glob_map: MakeGlobMap, resolver_arenas: &'a ResolverArenas<'a>, crate_loader: &'a mut CrateLoader<'a>, after_expand: F, @@ -937,7 +927,6 @@ where cstore, &krate, crate_name, - make_glob_map, crate_loader, &resolver_arenas, ); diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 889e1ec3b98c6..010ac28cdc944 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -57,7 +57,6 @@ extern crate syntax_pos; use driver::CompileController; use pretty::{PpMode, UserIdentifiedItem}; -use rustc_resolve as resolve; use rustc_save_analysis as save; use rustc_save_analysis::DumpHandler; use rustc_data_structures::sync::{self, Lrc, Ordering::SeqCst}; @@ -950,7 +949,6 @@ pub fn enable_save_analysis(control: &mut CompileController) { }); }; control.after_analysis.run_callback_on_error = true; - control.make_glob_map = resolve::MakeGlobMap::Yes; } impl RustcDefaultCalls { diff --git a/src/librustc_driver/test.rs b/src/librustc_driver/test.rs index 9c027f110eb4e..afcf08632a4f0 100644 --- a/src/librustc_driver/test.rs +++ b/src/librustc_driver/test.rs @@ -18,7 +18,6 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc_data_structures::sync::{self, Lrc}; use rustc_lint; use rustc_metadata::cstore::CStore; -use rustc_resolve::MakeGlobMap; use rustc_target::spec::abi::Abi; use syntax; use syntax::ast; @@ -134,7 +133,6 @@ fn test_env_with_pool( None, "test", None, - MakeGlobMap::No, |_| Ok(()), ).expect("phase 2 aborted") }; diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 7c05913467c54..797837c0e849d 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1536,9 +1536,7 @@ pub struct Resolver<'a> { extern_module_map: FxHashMap<(DefId, bool /* MacrosOnly? */), Module<'a>>, binding_parent_modules: FxHashMap>, Module<'a>>, - pub make_glob_map: bool, - /// Maps imports to the names of items actually imported (this actually maps - /// all imports, but only glob imports are actually interesting). + /// Maps glob imports to the names of items actually imported. pub glob_map: GlobMap, used_imports: FxHashSet<(NodeId, Namespace)>, @@ -1785,7 +1783,6 @@ impl<'a> Resolver<'a> { cstore: &'a CStore, krate: &Crate, crate_name: &str, - make_glob_map: MakeGlobMap, crate_loader: &'a mut CrateLoader<'a>, arenas: &'a ResolverArenas<'a>) -> Resolver<'a> { @@ -1869,7 +1866,6 @@ impl<'a> Resolver<'a> { extern_module_map: FxHashMap::default(), binding_parent_modules: FxHashMap::default(), - make_glob_map: make_glob_map == MakeGlobMap::Yes, glob_map: Default::default(), used_imports: FxHashSet::default(), @@ -1979,14 +1975,15 @@ impl<'a> Resolver<'a> { used.set(true); directive.used.set(true); self.used_imports.insert((directive.id, ns)); - self.add_to_glob_map(directive.id, ident); + self.add_to_glob_map(&directive, ident); self.record_use(ident, ns, binding, false); } } - fn add_to_glob_map(&mut self, id: NodeId, ident: Ident) { - if self.make_glob_map { - self.glob_map.entry(id).or_default().insert(ident.name); + #[inline] + fn add_to_glob_map(&mut self, directive: &ImportDirective<'_>, ident: Ident) { + if directive.is_glob() { + self.glob_map.entry(directive.id).or_default().insert(ident.name); } } @@ -4551,7 +4548,7 @@ impl<'a> Resolver<'a> { let import_id = match binding.kind { NameBindingKind::Import { directive, .. } => { self.maybe_unused_trait_imports.insert(directive.id); - self.add_to_glob_map(directive.id, trait_name); + self.add_to_glob_map(&directive, trait_name); Some(directive.id) } _ => None, @@ -5255,12 +5252,6 @@ fn err_path_resolution() -> PathResolution { PathResolution::new(Def::Err) } -#[derive(PartialEq,Copy, Clone)] -pub enum MakeGlobMap { - Yes, - No, -} - #[derive(Copy, Clone, Debug)] enum CrateLint { /// Do not issue the lint diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 891537309177e..52be67fb51228 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -1239,7 +1239,6 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { // Make a comma-separated list of names of imported modules. let glob_map = &self.save_ctxt.analysis.glob_map; - let glob_map = glob_map.as_ref().unwrap(); let names = if glob_map.contains_key(&id) { glob_map.get(&id).unwrap().iter().map(|n| n.to_string()).collect() } else { diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index d5b3070372ba5..4b0fb686ba523 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -1122,8 +1122,6 @@ pub fn process_crate<'l, 'tcx, H: SaveHandler>( mut handler: H, ) { tcx.dep_graph.with_ignore(|| { - assert!(analysis.glob_map.is_some()); - info!("Dumping crate {}", cratename); let save_ctxt = SaveContext { diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 8bb0dc9daa6a2..78dbf41bf21fd 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -451,7 +451,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt None, &name, None, - resolve::MakeGlobMap::No, &resolver_arenas, &mut crate_loader, |_| Ok(())); @@ -476,7 +475,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }).collect(), }; let analysis = ty::CrateAnalysis { - glob_map: if resolver.make_glob_map { Some(resolver.glob_map.clone()) } else { None }, + glob_map: resolver.glob_map.clone(), }; let mut arenas = AllArenas::new(); diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 1f19fa2e7f598..af47c7d5e8bfa 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -6,7 +6,6 @@ use rustc_driver::{self, driver, target_features, Compilation}; use rustc_driver::driver::phase_2_configure_and_expand; use rustc_metadata::cstore::CStore; use rustc_metadata::dynamic_lib::DynamicLibrary; -use rustc_resolve::MakeGlobMap; use rustc::hir; use rustc::hir::intravisit; use rustc::session::{self, CompileIncomplete, config}; @@ -100,7 +99,6 @@ pub fn run(mut options: Options) -> isize { None, "rustdoc-test", None, - MakeGlobMap::No, |_| Ok(()), ).expect("phase_2_configure_and_expand aborted in rustdoc!") }; From cc19a78ee846b34fae8b1caa19d84cf33d7ff67e Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Mon, 7 Jan 2019 09:09:03 +0100 Subject: [PATCH 2/3] Account for 2 removed lines in src/librustdoc/test.rs --- src/test/rustdoc-ui/failed-doctest-output.stdout | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/rustdoc-ui/failed-doctest-output.stdout b/src/test/rustdoc-ui/failed-doctest-output.stdout index 9fd46e94f030a..d584845e1f2ed 100644 --- a/src/test/rustdoc-ui/failed-doctest-output.stdout +++ b/src/test/rustdoc-ui/failed-doctest-output.stdout @@ -12,7 +12,7 @@ error[E0425]: cannot find value `no` in this scope 3 | no | ^^ not found in this scope -thread '$DIR/failed-doctest-output.rs - OtherStruct (line 17)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:321:13 +thread '$DIR/failed-doctest-output.rs - OtherStruct (line 17)' panicked at 'couldn't compile the test', src/librustdoc/test.rs:319:13 note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace. ---- $DIR/failed-doctest-output.rs - SomeStruct (line 11) stdout ---- @@ -21,7 +21,7 @@ thread '$DIR/failed-doctest-output.rs - SomeStruct (line 11)' panicked at 'test thread 'main' panicked at 'oh no', $DIR/failed-doctest-output.rs:3:1 note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace. -', src/librustdoc/test.rs:356:17 +', src/librustdoc/test.rs:354:17 failures: From b1b64bd196e513385f68e541da5a1b61fae986b7 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 13 Jan 2019 10:44:04 +0100 Subject: [PATCH 3/3] Use building RLS branch without `make_glob` --- src/tools/rls | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/rls b/src/tools/rls index 1a6361bd399a9..ae0d89a08d091 160000 --- a/src/tools/rls +++ b/src/tools/rls @@ -1 +1 @@ -Subproject commit 1a6361bd399a9466deba9b42ff2ff2ae365c5d0e +Subproject commit ae0d89a08d091ba1563b571739768a09d4cd3d69