resolve: Update NameBindingData::ambiguity in place

petrochenkov · petrochenkov · commit a251ae2615bd · 2026-01-08T10:51:11.000+03:00
instead of creating fresh bindings, except in one case.
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -89,7 +89,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     ) {
         let decl = self.arenas.alloc_decl(DeclData {
             kind: DeclKind::Def(res),
-            ambiguity,
+            ambiguity: CmCell::new(ambiguity),
             // External ambiguities always report the `AMBIGUOUS_GLOB_IMPORTS` lint at the moment.
             warn_ambiguity: CmCell::new(true),
             vis: CmCell::new(vis),
diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs
@@ -100,7 +100,9 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
                 if let Some(node_id) = import.id() {
                     r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
                 }
-            } else if decl.ambiguity.is_some() && eff_vis.is_public_at_level(Level::Reexported) {
+            } else if decl.ambiguity.get().is_some()
+                && eff_vis.is_public_at_level(Level::Reexported)
+            {
                 exported_ambiguities.insert(*decl);
             }
         }
@@ -125,7 +127,8 @@ impl<'a, 'ra, 'tcx> EffectiveVisibilitiesVisitor<'a, 'ra, 'tcx> {
             // If the binding is ambiguous, put the root ambiguity binding and all reexports
             // leading to it into the table. They are used by the `ambiguous_glob_reexports`
             // lint. For all bindings added to the table this way `is_ambiguity` returns true.
-            let is_ambiguity = |decl: Decl<'ra>, warn: bool| decl.ambiguity.is_some() && !warn;
+            let is_ambiguity =
+                |decl: Decl<'ra>, warn: bool| decl.ambiguity.get().is_some() && !warn;
             let mut parent_id = ParentId::Def(module_id);
             let mut warn_ambiguity = decl.warn_ambiguity.get();
             while let DeclKind::Import { source_decl, .. } = decl.kind {
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -300,10 +300,10 @@ fn remove_same_import<'ra>(d1: Decl<'ra>, d2: Decl<'ra>) -> (Decl<'ra>, Decl<'ra
     if let DeclKind::Import { import: import1, source_decl: d1_next } = d1.kind
         && let DeclKind::Import { import: import2, source_decl: d2_next } = d2.kind
         && import1 == import2
-        && d1.ambiguity == d2.ambiguity
+        && d1.warn_ambiguity.get() == d2.warn_ambiguity.get()
     {
-        assert!(d1.ambiguity.is_none());
-        assert_eq!(d1.warn_ambiguity.get(), d2.warn_ambiguity.get());
+        assert_eq!(d1.ambiguity.get(), d2.ambiguity.get());
+        assert!(!d1.warn_ambiguity.get());
         assert_eq!(d1.expansion, d2.expansion);
         assert_eq!(d1.span, d2.span);
         assert_eq!(d1.vis(), d2.vis());
@@ -335,7 +335,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
 
         self.arenas.alloc_decl(DeclData {
             kind: DeclKind::Import { source_decl: decl, import },
-            ambiguity: None,
+            ambiguity: CmCell::new(None),
             warn_ambiguity: CmCell::new(false),
             span: import.span,
             vis: CmCell::new(vis),
@@ -359,9 +359,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // all these overwrites will be re-fetched by glob imports importing
         // from that module without generating new ambiguities.
         // - A glob decl is overwritten by a non-glob decl arriving later.
-        // - A glob decl is overwritten by an ambiguous glob decl.
-        //   FIXME: avoid this by putting `DeclData::ambiguity` under a
-        //   cell and updating it in place.
+        // - A glob decl is overwritten by its clone after setting ambiguity in it.
+        //   FIXME: avoid this by removing `warn_ambiguity`, or by triggering glob re-fetch
+        //   with the same decl in some way.
         // - A glob decl is overwritten by a glob decl re-fetching an
         //   overwritten decl from other module (the recursive case).
         // Here we are detecting all such re-fetches and overwrite old decls
@@ -372,21 +372,34 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         if deep_decl != glob_decl {
             // Some import layers have been removed, need to overwrite.
             assert_ne!(old_deep_decl, old_glob_decl);
-            assert_ne!(old_deep_decl, deep_decl);
-            assert!(old_deep_decl.is_glob_import());
+            // FIXME: reenable the asserts when `warn_ambiguity` is removed (#149195).
+            // assert_ne!(old_deep_decl, deep_decl);
+            // assert!(old_deep_decl.is_glob_import());
+            assert!(!deep_decl.is_glob_import());
             if glob_decl.is_ambiguity_recursive() {
                 glob_decl.warn_ambiguity.set_unchecked(true);
             }
             glob_decl
         } else if glob_decl.res() != old_glob_decl.res() {
-            self.new_decl_with_ambiguity(old_glob_decl, glob_decl, warn_ambiguity)
+            old_glob_decl.ambiguity.set_unchecked(Some(glob_decl));
+            old_glob_decl.warn_ambiguity.set_unchecked(warn_ambiguity);
+            if warn_ambiguity {
+                old_glob_decl
+            } else {
+                // Need a fresh decl so other glob imports importing it could re-fetch it
+                // and set their own `warn_ambiguity` to true.
+                // FIXME: remove this when `warn_ambiguity` is removed (#149195).
+                self.arenas.alloc_decl((*old_glob_decl).clone())
+            }
         } else if !old_glob_decl.vis().is_at_least(glob_decl.vis(), self.tcx) {
             // We are glob-importing the same item but with greater visibility.
             old_glob_decl.vis.set_unchecked(glob_decl.vis());
             old_glob_decl
         } else if glob_decl.is_ambiguity_recursive() && !old_glob_decl.is_ambiguity_recursive() {
             // Overwriting a non-ambiguous glob import with an ambiguous glob import.
-            self.new_decl_with_ambiguity(old_glob_decl, glob_decl, true)
+            old_glob_decl.ambiguity.set_unchecked(Some(glob_decl));
+            old_glob_decl.warn_ambiguity.set_unchecked(true);
+            old_glob_decl
         } else {
             old_glob_decl
         }
@@ -415,6 +428,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         self.update_local_resolution(module, key, warn_ambiguity, |this, resolution| {
             if let Some(old_decl) = resolution.best_decl() {
                 assert_ne!(decl, old_decl);
+                assert!(!decl.warn_ambiguity.get());
                 if res == Res::Err && old_decl.res() != Res::Err {
                     // Do not override real declarations with `Res::Err`s from error recovery.
                     return Ok(());
@@ -453,19 +467,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         })
     }
 
-    fn new_decl_with_ambiguity(
-        &self,
-        primary_decl: Decl<'ra>,
-        secondary_decl: Decl<'ra>,
-        warn_ambiguity: bool,
-    ) -> Decl<'ra> {
-        let ambiguity = Some(secondary_decl);
-        let warn_ambiguity = CmCell::new(warn_ambiguity);
-        let vis = primary_decl.vis.clone();
-        let data = DeclData { ambiguity, warn_ambiguity, vis, ..*primary_decl };
-        self.arenas.alloc_decl(data)
-    }
-
     // Use `f` to mutate the resolution of the name in the module.
     // If the resolution becomes a success, define it in the module's glob importers.
     fn update_local_resolution<T, F>(
@@ -671,7 +672,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 let Some(binding) = resolution.best_decl() else { continue };
 
                 if let DeclKind::Import { import, .. } = binding.kind
-                    && let Some(amb_binding) = binding.ambiguity
+                    && let Some(amb_binding) = binding.ambiguity.get()
                     && binding.res() != Res::Err
                     && exported_ambiguities.contains(&binding)
                 {
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -802,10 +802,10 @@ impl<'ra> fmt::Debug for Module<'ra> {
 }
 
 /// Data associated with any name declaration.
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 struct DeclData<'ra> {
     kind: DeclKind<'ra>,
-    ambiguity: Option<Decl<'ra>>,
+    ambiguity: CmCell<Option<Decl<'ra>>>,
     /// Produce a warning instead of an error when reporting ambiguities inside this binding.
     /// May apply to indirect ambiguities under imports, so `ambiguity.is_some()` is not required.
     warn_ambiguity: CmCell<bool>,
@@ -942,7 +942,7 @@ impl<'ra> DeclData<'ra> {
     }
 
     fn descent_to_ambiguity(self: Decl<'ra>) -> Option<(Decl<'ra>, Decl<'ra>)> {
-        match self.ambiguity {
+        match self.ambiguity.get() {
             Some(ambig_binding) => Some((self, ambig_binding)),
             None => match self.kind {
                 DeclKind::Import { source_decl, .. } => source_decl.descent_to_ambiguity(),
@@ -952,7 +952,7 @@ impl<'ra> DeclData<'ra> {
     }
 
     fn is_ambiguity_recursive(&self) -> bool {
-        self.ambiguity.is_some()
+        self.ambiguity.get().is_some()
             || match self.kind {
                 DeclKind::Import { source_decl, .. } => source_decl.is_ambiguity_recursive(),
                 _ => false,
@@ -1346,7 +1346,7 @@ impl<'ra> ResolverArenas<'ra> {
     ) -> Decl<'ra> {
         self.alloc_decl(DeclData {
             kind: DeclKind::Def(res),
-            ambiguity: None,
+            ambiguity: CmCell::new(None),
             warn_ambiguity: CmCell::new(false),
             vis: CmCell::new(vis),
             span,
@@ -2055,7 +2055,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         used: Used,
         warn_ambiguity: bool,
     ) {
-        if let Some(b2) = used_decl.ambiguity {
+        if let Some(b2) = used_decl.ambiguity.get() {
             let ambiguity_error = AmbiguityError {
                 kind: AmbiguityKind::GlobVsGlob,
                 ident,
diff --git a/tests/ui/imports/issue-55884-1.rs b/tests/ui/imports/issue-55884-1.rs
@@ -17,5 +17,5 @@ mod m {
 
 fn main() {
     use m::S; //~ ERROR `S` is ambiguous
-    let s = S {};
+    let s = S {}; //~ ERROR `S` is ambiguous
 }
diff --git a/tests/ui/imports/issue-55884-1.stderr b/tests/ui/imports/issue-55884-1.stderr
@@ -18,6 +18,26 @@ LL |     pub use self::m2::*;
    |             ^^^^^^^^^^^
    = help: consider adding an explicit import of `S` to disambiguate
 
-error: aborting due to 1 previous error
+error[E0659]: `S` is ambiguous
+  --> $DIR/issue-55884-1.rs:20:13
+   |
+LL |     let s = S {};
+   |             ^ ambiguous name
+   |
+   = note: ambiguous because of multiple glob imports of a name in the same module
+note: `S` could refer to the struct imported here
+  --> $DIR/issue-55884-1.rs:14:13
+   |
+LL |     pub use self::m1::*;
+   |             ^^^^^^^^^^^
+   = help: consider adding an explicit import of `S` to disambiguate
+note: `S` could also refer to the struct imported here
+  --> $DIR/issue-55884-1.rs:15:13
+   |
+LL |     pub use self::m2::*;
+   |             ^^^^^^^^^^^
+   = help: consider adding an explicit import of `S` to disambiguate
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0659`.

Original file line number	Diff line number	Diff line change
`@@ -17,5 +17,5 @@ mod m {`
`17`	`17`
`18`	`18`	`fn main() {`
`19`	`19`	use m::S; //~ ERROR `S` is ambiguous
`20`		`- let s = S {};`
	`20`	+ let s = S {}; //~ ERROR `S` is ambiguous
`21`	`21`	`}`