Rollup merge of #80765 - petrochenkov:traitsinscope, r=matthewjasper

resolve: Simplify collection of traits in scope

"Traits in scope" for a given location are collected by walking all scopes in type namespace, collecting traits in them and pruning traits that don't have an associated item with the given name and namespace.

Previously we tried to prune traits using some kind of hygienic resolution for associated items, but that was complex and likely incorrect, e.g. in #80762 correction to visibilites of trait items caused some traits to not be in scope anymore.
I previously had some comments and concerns about this in https://github.com/rust-lang/rust/pull/65351.

In this PR we are doing some much simpler pruning based on `Symbol` and `Namespace` comparisons, it should be enough to throw away 99.9% of unnecessary traits.
It is not necessary for pruning to be precise because for trait aliases, for example, we don't do any pruning at all, and precise hygienic resolution for associated items needs to be done in typeck anyway.

The somewhat unexpected effect is that trait imports introduced by macros 2.0 now bring traits into scope due to the removed hygienic check on associated item names.
I'm not sure whether it is desirable or not, but I think it's acceptable for now.
The old check was certainly incorrect because macros 2.0 did bring trait aliases into scope.
If doing this is not desirable, then we should come up with some other way to avoid bringing traits from macros 2.0 into scope, that would accommodate for trait aliases as well.

---

The PR also contains a couple of pure refactorings
- Scope walk is done by using `visit_scopes` instead of a hand-rolled version.
- Code is restructured to accomodate for rustdoc that also wants to query traits in scope, but doesn't want to filter them by associated items at all.

r? ```@matthewjasper```

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -115,7 +115,7 @@ fn nearest_parent_mod(&mut self, def_id: DefId) -> Module<'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 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 @@ fn smart_resolve_path_fragment(
                 // 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);
                 }
 
@@ -2371,12 +2370,12 @@ fn record_candidate_traits_for_expr_if_necessary(&mut self, expr: &'ast Expr) {
                 // 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);
             }
             _ => {
@@ -2385,64 +2384,13 @@ fn record_candidate_traits_for_expr_if_necessary(&mut self, expr: &'ast Expr) {
         }
     }
 
-    fn get_traits_containing_item(
+    fn traits_in_scope(&mut self, ident: Ident, ns: Namespace) -> Vec<TraitCandidate> {
-        &mut self,
+        self.r.traits_in_scope(
-        mut ident: Ident,
+            self.current_trait_ref.as_ref().map(|(module, _)| *module),
-        ns: Namespace,
+            &self.parent_scope,
-    ) -> Vec<TraitCandidate> {
+            ident.span.ctxt(),
-        debug!("(getting traits containing item) looking for '{}'", ident.name);
+            Some((ident.name, ns)),
-
+        )
-        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
     }
 }
 

compiler/rustc_resolve/src/lib.rs

@@ -44,9 +44,9 @@
 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 @@ pub fn resolve_crate(&mut self, krate: &Crate) {
         self.crate_loader.postprocess(krate);
     }
 
-    fn get_traits_in_module_containing_item(
+    pub fn traits_in_scope(
         &mut self,
-        ident: Ident,
+        current_trait: Option<Module<'a>>,
-        ns: Namespace,
-        module: Module<'a>,
-        found_traits: &mut Vec<TraitCandidate>,
         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,
+        module: Module<'a>,
+        assoc_item: Option<(Symbol, Namespace)>,
+        found_traits: &mut Vec<TraitCandidate>,
     ) {
-        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() {
-        for &(trait_name, binding) in traits.as_ref().unwrap().iter() {
+            if self.trait_may_have_item(trait_binding.module(), assoc_item) {
-            // Traits have pseudo-modules that can be used to search for the given ident.
+                let def_id = trait_binding.res().def_id();
-            if let Some(module) = binding.module() {
+                let import_ids = self.find_transitive_imports(&trait_binding.kind, *trait_name);
-                let mut ident = ident;
+                found_traits.push(TraitCandidate { def_id, import_ids });
-                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,
+        }
+    }
+
     fn find_transitive_imports(
         &mut self,
         mut kind: &NameBindingKind<'_>,
@@ -3227,34 +3254,6 @@ fn extern_prelude_get(
         })
     }
 
-    /// 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 @@
     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,5 +1,6 @@
-// Make sure that underscore imports have the same hygiene considerations as
+// Make sure that underscore imports have the same hygiene considerations as other imports.
-// 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`
+    (&()).deref();
-    (&mut ()).deref_mut();      //~ ERROR no method named `deref_mut`
+    (&mut ()).deref_mut();
 }

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

@@ -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`.