Rollup merge of #126954 - petrochenkov:globamb, r=compiler-errors

matthiaskrgr · web-flow · commit 8b3bbee26b60 · 2024-06-26T07:50:19.000+02:00
resolve: Tweak some naming around import ambiguities
diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs
@@ -99,7 +99,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
         // is the maximum value among visibilities of bindings corresponding to that def id.
         for (binding, eff_vis) in visitor.import_effective_visibilities.iter() {
             let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };
-            if !binding.is_ambiguity() {
+            if !binding.is_ambiguity_recursive() {
                 if let Some(node_id) = import.id() {
                     r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
                 }
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -325,8 +325,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 }
                 match (old_binding.is_glob_import(), binding.is_glob_import()) {
                     (true, true) => {
-                        // FIXME: remove `!binding.is_ambiguity()` after delete the warning ambiguity.
-                        if !binding.is_ambiguity()
+                        // FIXME: remove `!binding.is_ambiguity_recursive()` after delete the warning ambiguity.
+                        if !binding.is_ambiguity_recursive()
                             && let NameBindingKind::Import { import: old_import, .. } =
                                 old_binding.kind
                             && let NameBindingKind::Import { import, .. } = binding.kind
@@ -337,21 +337,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                             // imported from the same glob-import statement.
                             resolution.binding = Some(binding);
                         } else if res != old_binding.res() {
-                            let binding = if warn_ambiguity {
-                                this.warn_ambiguity(AmbiguityKind::GlobVsGlob, old_binding, binding)
-                            } else {
-                                this.ambiguity(AmbiguityKind::GlobVsGlob, old_binding, binding)
-                            };
-                            resolution.binding = Some(binding);
+                            resolution.binding = Some(this.new_ambiguity_binding(
+                                AmbiguityKind::GlobVsGlob,
+                                old_binding,
+                                binding,
+                                warn_ambiguity,
+                            ));
                         } else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
                             // We are glob-importing the same item but with greater visibility.
                             resolution.binding = Some(binding);
-                        } else if binding.is_ambiguity() {
-                            resolution.binding =
-                                Some(self.arenas.alloc_name_binding(NameBindingData {
-                                    warn_ambiguity: true,
-                                    ..(*binding).clone()
-                                }));
+                        } else if binding.is_ambiguity_recursive() {
+                            resolution.binding = Some(this.new_warn_ambiguity_binding(binding));
                         }
                     }
                     (old_glob @ true, false) | (old_glob @ false, true) => {
@@ -361,24 +357,26 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                             && nonglob_binding.expansion != LocalExpnId::ROOT
                             && glob_binding.res() != nonglob_binding.res()
                         {
-                            resolution.binding = Some(this.ambiguity(
+                            resolution.binding = Some(this.new_ambiguity_binding(
                                 AmbiguityKind::GlobVsExpanded,
                                 nonglob_binding,
                                 glob_binding,
+                                false,
                             ));
                         } else {
                             resolution.binding = Some(nonglob_binding);
                         }
 
-                        if let Some(old_binding) = resolution.shadowed_glob {
-                            assert!(old_binding.is_glob_import());
-                            if glob_binding.res() != old_binding.res() {
-                                resolution.shadowed_glob = Some(this.ambiguity(
+                        if let Some(old_shadowed_glob) = resolution.shadowed_glob {
+                            assert!(old_shadowed_glob.is_glob_import());
+                            if glob_binding.res() != old_shadowed_glob.res() {
+                                resolution.shadowed_glob = Some(this.new_ambiguity_binding(
                                     AmbiguityKind::GlobVsGlob,
-                                    old_binding,
+                                    old_shadowed_glob,
                                     glob_binding,
+                                    false,
                                 ));
-                            } else if !old_binding.vis.is_at_least(binding.vis, this.tcx) {
+                            } else if !old_shadowed_glob.vis.is_at_least(binding.vis, this.tcx) {
                                 resolution.shadowed_glob = Some(glob_binding);
                             }
                         } else {
@@ -397,29 +395,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         })
     }
 
-    fn ambiguity(
+    fn new_ambiguity_binding(
         &self,
-        kind: AmbiguityKind,
+        ambiguity_kind: AmbiguityKind,
         primary_binding: NameBinding<'a>,
         secondary_binding: NameBinding<'a>,
+        warn_ambiguity: bool,
     ) -> NameBinding<'a> {
-        self.arenas.alloc_name_binding(NameBindingData {
-            ambiguity: Some((secondary_binding, kind)),
-            ..(*primary_binding).clone()
-        })
+        let ambiguity = Some((secondary_binding, ambiguity_kind));
+        let data = NameBindingData { ambiguity, warn_ambiguity, ..*primary_binding };
+        self.arenas.alloc_name_binding(data)
     }
 
-    fn warn_ambiguity(
-        &self,
-        kind: AmbiguityKind,
-        primary_binding: NameBinding<'a>,
-        secondary_binding: NameBinding<'a>,
-    ) -> NameBinding<'a> {
-        self.arenas.alloc_name_binding(NameBindingData {
-            ambiguity: Some((secondary_binding, kind)),
-            warn_ambiguity: true,
-            ..(*primary_binding).clone()
-        })
+    fn new_warn_ambiguity_binding(&self, binding: NameBinding<'a>) -> NameBinding<'a> {
+        assert!(binding.is_ambiguity_recursive());
+        self.arenas.alloc_name_binding(NameBindingData { warn_ambiguity: true, ..*binding })
     }
 
     // Use `f` to mutate the resolution of the name in the module.
@@ -1381,8 +1371,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     target_bindings[ns].get(),
                 ) {
                     Ok(other_binding) => {
-                        is_redundant =
-                            binding.res() == other_binding.res() && !other_binding.is_ambiguity();
+                        is_redundant = binding.res() == other_binding.res()
+                            && !other_binding.is_ambiguity_recursive();
                         if is_redundant {
                             redundant_span[ns] =
                                 Some((other_binding.span, other_binding.is_import()));
@@ -1455,7 +1445,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     .resolution(import.parent_scope.module, key)
                     .borrow()
                     .binding()
-                    .is_some_and(|binding| binding.is_warn_ambiguity());
+                    .is_some_and(|binding| binding.warn_ambiguity_recursive());
                 let _ = self.try_define(
                     import.parent_scope.module,
                     key,
@@ -1480,7 +1470,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
             module.for_each_child(self, |this, ident, _, binding| {
                 let res = binding.res().expect_non_local();
-                let error_ambiguity = binding.is_ambiguity() && !binding.warn_ambiguity;
+                let error_ambiguity = binding.is_ambiguity_recursive() && !binding.warn_ambiguity;
                 if res != def::Res::Err && !error_ambiguity {
                     let mut reexport_chain = SmallVec::new();
                     let mut next_binding = binding;
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -3730,7 +3730,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
         let ls_binding = self.maybe_resolve_ident_in_lexical_scope(ident, ValueNS)?;
         let (res, binding) = match ls_binding {
             LexicalScopeBinding::Item(binding)
-                if is_syntactic_ambiguity && binding.is_ambiguity() =>
+                if is_syntactic_ambiguity && binding.is_ambiguity_recursive() =>
             {
                 // For ambiguous bindings we don't know all their definitions and cannot check
                 // whether they can be shadowed by fresh bindings or not, so force an error.
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -694,10 +694,12 @@ impl<'a> fmt::Debug for Module<'a> {
 }
 
 /// Records a possibly-private value, type, or module definition.
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug)]
 struct NameBindingData<'a> {
     kind: NameBindingKind<'a>,
     ambiguity: Option<(NameBinding<'a>, AmbiguityKind)>,
+    /// 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: bool,
     expansion: LocalExpnId,
     span: Span,
@@ -718,7 +720,7 @@ impl<'a> ToNameBinding<'a> for NameBinding<'a> {
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug)]
 enum NameBindingKind<'a> {
     Res(Res),
     Module(Module<'a>),
@@ -830,18 +832,18 @@ impl<'a> NameBindingData<'a> {
         }
     }
 
-    fn is_ambiguity(&self) -> bool {
+    fn is_ambiguity_recursive(&self) -> bool {
         self.ambiguity.is_some()
             || match self.kind {
-                NameBindingKind::Import { binding, .. } => binding.is_ambiguity(),
+                NameBindingKind::Import { binding, .. } => binding.is_ambiguity_recursive(),
                 _ => false,
             }
     }
 
-    fn is_warn_ambiguity(&self) -> bool {
+    fn warn_ambiguity_recursive(&self) -> bool {
         self.warn_ambiguity
             || match self.kind {
-                NameBindingKind::Import { binding, .. } => binding.is_warn_ambiguity(),
+                NameBindingKind::Import { binding, .. } => binding.warn_ambiguity_recursive(),
                 _ => false,
             }
     }

Original file line number	Diff line number	Diff line change
`@@ -99,7 +99,7 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {`
`99`	`99`	`// is the maximum value among visibilities of bindings corresponding to that def id.`
`100`	`100`	`for (binding, eff_vis) in visitor.import_effective_visibilities.iter() {`
`101`	`101`	`let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };`
`102`		`- if !binding.is_ambiguity() {`
	`102`	`+ if !binding.is_ambiguity_recursive() {`
`103`	`103`	`if let Some(node_id) = import.id() {`
`104`	`104`	`r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)`
`105`	`105`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3730,7 +3730,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {`
`3730`	`3730`	`let ls_binding = self.maybe_resolve_ident_in_lexical_scope(ident, ValueNS)?;`
`3731`	`3731`	`let (res, binding) = match ls_binding {`
`3732`	`3732`	`LexicalScopeBinding::Item(binding)`
`3733`		`- if is_syntactic_ambiguity && binding.is_ambiguity() =>`
	`3733`	`+ if is_syntactic_ambiguity && binding.is_ambiguity_recursive() =>`
`3734`	`3734`	`{`
`3735`	`3735`	`// For ambiguous bindings we don't know all their definitions and cannot check`
`3736`	`3736`	`// whether they can be shadowed by fresh bindings or not, so force an error.`
Original file line number	Diff line number	Diff line change
`@@ -694,10 +694,12 @@ impl<'a> fmt::Debug for Module<'a> {`
`694`	`694`	`}`
`695`	`695`
`696`	`696`	`/// Records a possibly-private value, type, or module definition.`
`697`		`-#[derive(Clone, Debug)]`
	`697`	`+#[derive(Clone, Copy, Debug)]`
`698`	`698`	`struct NameBindingData<'a> {`
`699`	`699`	`kind: NameBindingKind<'a>,`
`700`	`700`	`ambiguity: Option<(NameBinding<'a>, AmbiguityKind)>,`
	`701`	`+ /// Produce a warning instead of an error when reporting ambiguities inside this binding.`
	`702`	+ /// May apply to indirect ambiguities under imports, so `ambiguity.is_some()` is not required.
`701`	`703`	`warn_ambiguity: bool,`
`702`	`704`	`expansion: LocalExpnId,`
`703`	`705`	`span: Span,`
`@@ -718,7 +720,7 @@ impl<'a> ToNameBinding<'a> for NameBinding<'a> {`
`718`	`720`	`}`
`719`	`721`	`}`
`720`	`722`
`721`		`-#[derive(Clone, Debug)]`
	`723`	`+#[derive(Clone, Copy, Debug)]`
`722`	`724`	`enum NameBindingKind<'a> {`
`723`	`725`	`Res(Res),`
`724`	`726`	`Module(Module<'a>),`
`@@ -830,18 +832,18 @@ impl<'a> NameBindingData<'a> {`
`830`	`832`	`}`
`831`	`833`	`}`
`832`	`834`
`833`		`- fn is_ambiguity(&self) -> bool {`
	`835`	`+ fn is_ambiguity_recursive(&self) -> bool {`
`834`	`836`	`self.ambiguity.is_some()`
`835`	`837`	`\|\| match self.kind {`
`836`		`- NameBindingKind::Import { binding, .. } => binding.is_ambiguity(),`
	`838`	`+ NameBindingKind::Import { binding, .. } => binding.is_ambiguity_recursive(),`
`837`	`839`	`_ => false,`
`838`	`840`	`}`
`839`	`841`	`}`
`840`	`842`
`841`		`- fn is_warn_ambiguity(&self) -> bool {`
	`843`	`+ fn warn_ambiguity_recursive(&self) -> bool {`
`842`	`844`	`self.warn_ambiguity`
`843`	`845`	`\|\| match self.kind {`
`844`		`- NameBindingKind::Import { binding, .. } => binding.is_warn_ambiguity(),`
	`846`	`+ NameBindingKind::Import { binding, .. } => binding.warn_ambiguity_recursive(),`
`845`	`847`	`_ => false,`
`846`	`848`	`}`
`847`	`849`	`}`