resolve: Simplify collection of traits in scope

Author: petrochenkov

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];

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<TraitCandidate> {
-        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<TraitCandidate> {
+        self.r.traits_in_scope(
+            self.current_trait_ref.as_ref().map(|(module, _)| *module),
+            &self.parent_scope,
+            ident.span.ctxt(),
+            Some((ident.name, ns)),
+        )
     }
 }
 

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,49 +1477,76 @@ impl<'a> Resolver<'a> {
         self.crate_loader.postprocess(krate);
     }
 
-    fn get_traits_in_module_containing_item(
+    pub fn traits_in_scope(
+        &mut self,
+        current_trait: Option<Module<'a>>,
+        parent_scope: &ParentScope<'a>,
+        ctxt: SyntaxContext,
+        assoc_item: Option<(Symbol, Namespace)>,
+    ) -> Vec<TraitCandidate> {
+        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,
-        ident: Ident,
-        ns: Namespace,
         module: Module<'a>,
+        assoc_item: Option<(Symbol, Namespace)>,
         found_traits: &mut Vec<TraitCandidate>,
-        parent_scope: &ParentScope<'a>,
     ) {
-        assert!(ns == TypeNS || ns == ValueNS);
         module.ensure_traits(self);
         let traits = module.traits.borrow();
+        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 });
+            }
+        }
+    }
 
-        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?")
+    // 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<Module<'a>>,
+        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,
         }
     }
 
@@ -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<TraitCandidate> {
-        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,

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()
         })
     });
 

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();
+}

src/test/ui/underscore-imports/hygiene.rs

@@ -1,13 +1,13 @@
-// 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)]
 
 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();
 }