From b7071b2353199cd438f15dd3068a9d64d3fab93c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 6 Jan 2021 23:50:02 +0300 Subject: [PATCH] resolve: Simplify collection of traits in scope --- .../rustc_resolve/src/build_reduced_graph.rs | 2 +- compiler/rustc_resolve/src/late.rs | 72 ++-------- compiler/rustc_resolve/src/lib.rs | 131 +++++++++--------- .../passes/collect_intra_doc_links.rs | 9 +- src/test/ui/hygiene/traits-in-scope.rs | 53 +++++++ src/test/ui/underscore-imports/hygiene.rs | 10 +- src/test/ui/underscore-imports/hygiene.stderr | 27 ---- 7 files changed, 141 insertions(+), 163 deletions(-) create mode 100644 src/test/ui/hygiene/traits-in-scope.rs delete mode 100644 src/test/ui/underscore-imports/hygiene.stderr diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index e96fc185b7e..4ab14c158d3 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -115,7 +115,7 @@ impl<'a> Resolver<'a> { self.get_module(parent_id) } - crate fn get_module(&mut self, def_id: DefId) -> Module<'a> { + pub fn get_module(&mut self, def_id: DefId) -> Module<'a> { // If this is a local module, it will be in `module_map`, no need to recalculate it. if let Some(def_id) = def_id.as_local() { return self.module_map[&def_id]; diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 8544e1d8ee5..025bb24dfe8 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -14,7 +14,6 @@ use crate::{ResolutionError, Resolver, Segment, UseError}; use rustc_ast::ptr::P; use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::*; -use rustc_ast::{unwrap_or, walk_list}; use rustc_ast_lowering::ResolverAstLowering; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::DiagnosticId; @@ -1911,7 +1910,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // it needs to be added to the trait map. if ns == ValueNS { let item_name = path.last().unwrap().ident; - let traits = self.get_traits_containing_item(item_name, ns); + let traits = self.traits_in_scope(item_name, ns); self.r.trait_map.insert(id, traits); } @@ -2372,12 +2371,12 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // field, we need to add any trait methods we find that match // the field name so that we can do some nice error reporting // later on in typeck. - let traits = self.get_traits_containing_item(ident, ValueNS); + let traits = self.traits_in_scope(ident, ValueNS); self.r.trait_map.insert(expr.id, traits); } ExprKind::MethodCall(ref segment, ..) => { debug!("(recording candidate traits for expr) recording traits for {}", expr.id); - let traits = self.get_traits_containing_item(segment.ident, ValueNS); + let traits = self.traits_in_scope(segment.ident, ValueNS); self.r.trait_map.insert(expr.id, traits); } _ => { @@ -2386,64 +2385,13 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } } - fn get_traits_containing_item( - &mut self, - mut ident: Ident, - ns: Namespace, - ) -> Vec { - debug!("(getting traits containing item) looking for '{}'", ident.name); - - let mut found_traits = Vec::new(); - // Look for the current trait. - if let Some((module, _)) = self.current_trait_ref { - if self - .r - .resolve_ident_in_module( - ModuleOrUniformRoot::Module(module), - ident, - ns, - &self.parent_scope, - false, - module.span, - ) - .is_ok() - { - let def_id = module.def_id().unwrap(); - found_traits.push(TraitCandidate { def_id, import_ids: smallvec![] }); - } - } - - ident.span = ident.span.normalize_to_macros_2_0(); - let mut search_module = self.parent_scope.module; - loop { - self.r.get_traits_in_module_containing_item( - ident, - ns, - search_module, - &mut found_traits, - &self.parent_scope, - ); - let mut span_data = ident.span.data(); - search_module = unwrap_or!( - self.r.hygienic_lexical_parent(search_module, &mut span_data.ctxt), - break - ); - ident.span = span_data.span(); - } - - if let Some(prelude) = self.r.prelude { - if !search_module.no_implicit_prelude { - self.r.get_traits_in_module_containing_item( - ident, - ns, - prelude, - &mut found_traits, - &self.parent_scope, - ); - } - } - - found_traits + fn traits_in_scope(&mut self, ident: Ident, ns: Namespace) -> Vec { + self.r.traits_in_scope( + self.current_trait_ref.as_ref().map(|(module, _)| *module), + &self.parent_scope, + ident.span.ctxt(), + Some((ident.name, ns)), + ) } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 86bb1c77911..fa2b9b823a7 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -44,9 +44,9 @@ use rustc_index::vec::IndexVec; use rustc_metadata::creader::{CStore, CrateLoader}; use rustc_middle::hir::exports::ExportMap; use rustc_middle::middle::cstore::{CrateStore, MetadataLoaderDyn}; +use rustc_middle::span_bug; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, DefIdTree, ResolverOutputs}; -use rustc_middle::{bug, span_bug}; use rustc_session::lint; use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer}; use rustc_session::Session; @@ -1477,52 +1477,79 @@ impl<'a> Resolver<'a> { self.crate_loader.postprocess(krate); } - fn get_traits_in_module_containing_item( + pub fn traits_in_scope( &mut self, - ident: Ident, - ns: Namespace, - module: Module<'a>, - found_traits: &mut Vec, + current_trait: Option>, parent_scope: &ParentScope<'a>, + ctxt: SyntaxContext, + assoc_item: Option<(Symbol, Namespace)>, + ) -> Vec { + let mut found_traits = Vec::new(); + + if let Some(module) = current_trait { + if self.trait_may_have_item(Some(module), assoc_item) { + let def_id = module.def_id().unwrap(); + found_traits.push(TraitCandidate { def_id, import_ids: smallvec![] }); + } + } + + self.visit_scopes(ScopeSet::All(TypeNS, false), parent_scope, ctxt, |this, scope, _, _| { + match scope { + Scope::Module(module) => { + this.traits_in_module(module, assoc_item, &mut found_traits); + } + Scope::StdLibPrelude => { + if let Some(module) = this.prelude { + this.traits_in_module(module, assoc_item, &mut found_traits); + } + } + Scope::ExternPrelude | Scope::ToolPrelude | Scope::BuiltinTypes => {} + _ => unreachable!(), + } + None::<()> + }); + + found_traits + } + + fn traits_in_module( + &mut self, + module: Module<'a>, + assoc_item: Option<(Symbol, Namespace)>, + found_traits: &mut Vec, ) { - assert!(ns == TypeNS || ns == ValueNS); module.ensure_traits(self); let traits = module.traits.borrow(); - - for &(trait_name, binding) in traits.as_ref().unwrap().iter() { - // Traits have pseudo-modules that can be used to search for the given ident. - if let Some(module) = binding.module() { - let mut ident = ident; - if ident.span.glob_adjust(module.expansion, binding.span).is_none() { - continue; - } - if self - .resolve_ident_in_module_unadjusted( - ModuleOrUniformRoot::Module(module), - ident, - ns, - parent_scope, - false, - module.span, - ) - .is_ok() - { - let import_ids = self.find_transitive_imports(&binding.kind, trait_name); - let trait_def_id = module.def_id().unwrap(); - found_traits.push(TraitCandidate { def_id: trait_def_id, import_ids }); - } - } else if let Res::Def(DefKind::TraitAlias, _) = binding.res() { - // For now, just treat all trait aliases as possible candidates, since we don't - // know if the ident is somewhere in the transitive bounds. - let import_ids = self.find_transitive_imports(&binding.kind, trait_name); - let trait_def_id = binding.res().def_id(); - found_traits.push(TraitCandidate { def_id: trait_def_id, import_ids }); - } else { - bug!("candidate is not trait or trait alias?") + for (trait_name, trait_binding) in traits.as_ref().unwrap().iter() { + if self.trait_may_have_item(trait_binding.module(), assoc_item) { + let def_id = trait_binding.res().def_id(); + let import_ids = self.find_transitive_imports(&trait_binding.kind, *trait_name); + found_traits.push(TraitCandidate { def_id, import_ids }); } } } + // List of traits in scope is pruned on best effort basis. We reject traits not having an + // associated item with the given name and namespace (if specified). This is a conservative + // optimization, proper hygienic type-based resolution of associated items is done in typeck. + // We don't reject trait aliases (`trait_module == None`) because we don't have access to their + // associated items. + fn trait_may_have_item( + &mut self, + trait_module: Option>, + assoc_item: Option<(Symbol, Namespace)>, + ) -> bool { + match (trait_module, assoc_item) { + (Some(trait_module), Some((name, ns))) => { + self.resolutions(trait_module).borrow().iter().any(|resolution| { + let (&BindingKey { ident: assoc_ident, ns: assoc_ns, .. }, _) = resolution; + assoc_ns == ns && assoc_ident.name == name + }) + } + _ => true, + } + } + fn find_transitive_imports( &mut self, mut kind: &NameBindingKind<'_>, @@ -3227,34 +3254,6 @@ impl<'a> Resolver<'a> { }) } - /// This is equivalent to `get_traits_in_module_containing_item`, but without filtering by the associated item. - /// - /// This is used by rustdoc for intra-doc links. - pub fn traits_in_scope(&mut self, module_id: DefId) -> Vec { - let module = self.get_module(module_id); - module.ensure_traits(self); - let traits = module.traits.borrow(); - let to_candidate = - |this: &mut Self, &(trait_name, binding): &(Ident, &NameBinding<'_>)| TraitCandidate { - def_id: binding.res().def_id(), - import_ids: this.find_transitive_imports(&binding.kind, trait_name), - }; - - let mut candidates: Vec<_> = - traits.as_ref().unwrap().iter().map(|x| to_candidate(self, x)).collect(); - - if let Some(prelude) = self.prelude { - if !module.no_implicit_prelude { - prelude.ensure_traits(self); - candidates.extend( - prelude.traits.borrow().as_ref().unwrap().iter().map(|x| to_candidate(self, x)), - ); - } - } - - candidates - } - /// Rustdoc uses this to resolve things in a recoverable way. `ResolutionError<'a>` /// isn't something that can be returned because it can't be made to live that long, /// and also it's a private type. Fortunately rustdoc doesn't need to know the error, diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 9cba1b7b0f1..4b5eb4850a3 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -19,7 +19,7 @@ use rustc_session::lint::{ builtin::{BROKEN_INTRA_DOC_LINKS, PRIVATE_INTRA_DOC_LINKS}, Lint, }; -use rustc_span::hygiene::MacroKind; +use rustc_span::hygiene::{MacroKind, SyntaxContext}; use rustc_span::symbol::Ident; use rustc_span::symbol::Symbol; use rustc_span::DUMMY_SP; @@ -770,7 +770,12 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx let mut cache = cx.module_trait_cache.borrow_mut(); let in_scope_traits = cache.entry(module).or_insert_with(|| { cx.enter_resolver(|resolver| { - resolver.traits_in_scope(module).into_iter().map(|candidate| candidate.def_id).collect() + let parent_scope = &ParentScope::module(resolver.get_module(module), resolver); + resolver + .traits_in_scope(None, parent_scope, SyntaxContext::root(), None) + .into_iter() + .map(|candidate| candidate.def_id) + .collect() }) }); diff --git a/src/test/ui/hygiene/traits-in-scope.rs b/src/test/ui/hygiene/traits-in-scope.rs new file mode 100644 index 00000000000..548bb226b71 --- /dev/null +++ b/src/test/ui/hygiene/traits-in-scope.rs @@ -0,0 +1,53 @@ +// Macros with def-site hygiene still bring traits into scope. +// It is not clear whether this is desirable behavior or not. +// It is also not clear how to prevent it if it is not desirable. + +// check-pass + +#![feature(decl_macro)] +#![feature(trait_alias)] + +mod traits { + pub trait Trait1 { + fn simple_import(&self) {} + } + pub trait Trait2 { + fn renamed_import(&self) {} + } + pub trait Trait3 { + fn underscore_import(&self) {} + } + pub trait Trait4 { + fn trait_alias(&self) {} + } + + impl Trait1 for () {} + impl Trait2 for () {} + impl Trait3 for () {} + impl Trait4 for () {} +} + +macro m1() { + use traits::Trait1; +} +macro m2() { + use traits::Trait2 as Alias; +} +macro m3() { + use traits::Trait3 as _; +} +macro m4() { + trait Alias = traits::Trait4; +} + +fn main() { + m1!(); + m2!(); + m3!(); + m4!(); + + ().simple_import(); + ().renamed_import(); + ().underscore_import(); + ().trait_alias(); +} diff --git a/src/test/ui/underscore-imports/hygiene.rs b/src/test/ui/underscore-imports/hygiene.rs index a254f6eaa59..c4db6524538 100644 --- a/src/test/ui/underscore-imports/hygiene.rs +++ b/src/test/ui/underscore-imports/hygiene.rs @@ -1,5 +1,6 @@ -// Make sure that underscore imports have the same hygiene considerations as -// other imports. +// Make sure that underscore imports have the same hygiene considerations as other imports. + +// check-pass #![feature(decl_macro)] @@ -7,7 +8,6 @@ mod x { pub use std::ops::Deref as _; } - macro glob_import() { pub use crate::x::*; } @@ -35,6 +35,6 @@ fn main() { use crate::z::*; glob_import!(); underscore_import!(); - (&()).deref(); //~ ERROR no method named `deref` - (&mut ()).deref_mut(); //~ ERROR no method named `deref_mut` + (&()).deref(); + (&mut ()).deref_mut(); } diff --git a/src/test/ui/underscore-imports/hygiene.stderr b/src/test/ui/underscore-imports/hygiene.stderr deleted file mode 100644 index 29836137860..00000000000 --- a/src/test/ui/underscore-imports/hygiene.stderr +++ /dev/null @@ -1,27 +0,0 @@ -error[E0599]: no method named `deref` found for reference `&()` in the current scope - --> $DIR/hygiene.rs:38:11 - | -LL | (&()).deref(); - | ^^^^^ method not found in `&()` - | - = help: items from traits can only be used if the trait is in scope -help: the following trait is implemented but not in scope; perhaps add a `use` for it: - | -LL | use std::ops::Deref; - | - -error[E0599]: no method named `deref_mut` found for mutable reference `&mut ()` in the current scope - --> $DIR/hygiene.rs:39:15 - | -LL | (&mut ()).deref_mut(); - | ^^^^^^^^^ method not found in `&mut ()` - | - = help: items from traits can only be used if the trait is in scope -help: the following trait is implemented but not in scope; perhaps add a `use` for it: - | -LL | use std::ops::DerefMut; - | - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0599`.