resolve: re-export ambiguity as warning

bvanjoi · bvanjoi · commit 626c1936fe49 · 2023-08-10T11:59:55.000+08:00
diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs
@@ -1869,6 +1869,9 @@ pub fn report_ambiguity_error<'a, G: EmissionGuarantee>(
     for help_msg in ambiguity.b1_help_msgs {
         db.help(help_msg);
     }
+    if ambiguity.extern_crate {
+        return;
+    }
     db.span_note(ambiguity.b2_span, ambiguity.b2_note_msg);
     for help_msg in ambiguity.b2_help_msgs {
         db.help(help_msg);
diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs
@@ -469,6 +469,9 @@ impl<HCX> ToStableHashKey<HCX> for LintId {
 
 #[derive(Debug)]
 pub struct AmbiguityErrorDiag {
+    /// Is this a ambiguity binding come from
+    /// extern crate?
+    pub extern_crate: bool,
     pub msg: String,
     pub span: Span,
     pub label_span: Span,
diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs
@@ -1048,7 +1048,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         let res = Res::Def(self.def_kind(id), self.local_def_id(id));
         let vis = self.get_visibility(id);
 
-        ModChild { ident, res, vis, reexport_chain: Default::default() }
+        ModChild { ident, res, vis, reexport_chain: Default::default(), ambiguity: false }
     }
 
     /// Iterates over all named children of the given module,
diff --git a/compiler/rustc_middle/src/metadata.rs b/compiler/rustc_middle/src/metadata.rs
@@ -43,4 +43,6 @@ pub struct ModChild {
     /// Reexport chain linking this module child to its original reexported item.
     /// Empty if the module child is a proper item.
     pub reexport_chain: SmallVec<[Reexport; 2]>,
+    /// Is this an ambiguous binding?
+    pub ambiguity: bool,
 }
diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs
@@ -9,7 +9,7 @@ use crate::def_collector::collect_definitions;
 use crate::imports::{ImportData, ImportKind};
 use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
 use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
-use crate::{errors, BindingKey, MacroData, NameBindingData};
+use crate::{errors, AmbiguityKind, BindingKey, MacroData, NameBindingData};
 use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
 use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
 use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
@@ -62,6 +62,46 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span,
     }
 }
 
+impl<'a, Id: Into<DefId>> ToNameBinding<'a>
+    for (Module<'a>, ty::Visibility<Id>, Span, LocalExpnId, bool)
+{
+    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
+        let mut binding = NameBindingData {
+            kind: NameBindingKind::Module(self.0),
+            ambiguity: None,
+            warn_ambiguity: false,
+            vis: self.1.to_def_id(),
+            span: self.2,
+            expansion: self.3,
+        };
+        if self.4 {
+            binding.ambiguity =
+                Some((arenas.alloc_name_binding(binding.clone()), AmbiguityKind::ExternalCrate));
+            binding.warn_ambiguity = true;
+        }
+        arenas.alloc_name_binding(binding)
+    }
+}
+
+impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span, LocalExpnId, bool) {
+    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
+        let mut binding = NameBindingData {
+            kind: NameBindingKind::Res(self.0),
+            ambiguity: None,
+            warn_ambiguity: false,
+            vis: self.1.to_def_id(),
+            span: self.2,
+            expansion: self.3,
+        };
+        if self.4 {
+            binding.ambiguity =
+                Some((arenas.alloc_name_binding(binding.clone()), AmbiguityKind::ExternalCrate));
+            binding.warn_ambiguity = true;
+        }
+        arenas.alloc_name_binding(binding)
+    }
+}
+
 impl<'a, 'tcx> Resolver<'a, 'tcx> {
     /// Defines `name` in namespace `ns` of module `parent` to be `def` if it is not yet defined;
     /// otherwise, reports an error.
@@ -71,7 +111,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     {
         let binding = def.to_name_binding(self.arenas);
         let key = self.new_disambiguated_key(ident, ns);
-        if let Err(old_binding) = self.try_define(parent, key, binding, false) {
+        if let Err(old_binding) = self.try_define(parent, key, binding, binding.warn_ambiguity) {
             self.report_conflict(parent, ident, ns, old_binding, binding);
         }
     }
@@ -932,7 +972,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
     /// Builds the reduced graph for a single item in an external crate.
     fn build_reduced_graph_for_external_crate_res(&mut self, child: &ModChild) {
         let parent = self.parent_scope.module;
-        let ModChild { ident, res, vis, ref reexport_chain } = *child;
+        let ModChild { ident, res, vis, ref reexport_chain, ambiguity } = *child;
         let span = self.r.def_span(
             reexport_chain
                 .first()
@@ -945,7 +985,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
         match res {
             Res::Def(DefKind::Mod | DefKind::Enum | DefKind::Trait, def_id) => {
                 let module = self.r.expect_module(def_id);
-                self.r.define(parent, ident, TypeNS, (module, vis, span, expansion));
+                self.r.define(parent, ident, TypeNS, (module, vis, span, expansion, ambiguity));
             }
             Res::Def(
                 DefKind::Struct
@@ -959,7 +999,9 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
                 _,
             )
             | Res::PrimTy(..)
-            | Res::ToolMod => self.r.define(parent, ident, TypeNS, (res, vis, span, expansion)),
+            | Res::ToolMod => {
+                self.r.define(parent, ident, TypeNS, (res, vis, span, expansion, ambiguity))
+            }
             Res::Def(
                 DefKind::Fn
                 | DefKind::AssocFn
@@ -968,9 +1010,9 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
                 | DefKind::AssocConst
                 | DefKind::Ctor(..),
                 _,
-            ) => self.r.define(parent, ident, ValueNS, (res, vis, span, expansion)),
+            ) => self.r.define(parent, ident, ValueNS, (res, vis, span, expansion, ambiguity)),
             Res::Def(DefKind::Macro(..), _) | Res::NonMacroAttr(..) => {
-                self.r.define(parent, ident, MacroNS, (res, vis, span, expansion))
+                self.r.define(parent, ident, MacroNS, (res, vis, span, expansion, ambiguity))
             }
             Res::Def(
                 DefKind::TyParam
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
@@ -136,12 +136,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         for ambiguity_error in &self.ambiguity_errors {
             let diag = self.ambiguity_diagnostics(ambiguity_error);
             if ambiguity_error.warning {
-                let NameBindingKind::Import { import, .. } = ambiguity_error.b1.0.kind else {
-                    unreachable!()
-                };
+                let root_id =
+                    if let NameBindingKind::Import { import, .. } = ambiguity_error.b1.0.kind {
+                        import.root_id
+                    } else if ambiguity_error.kind == AmbiguityKind::ExternalCrate {
+                        krate.id
+                    } else {
+                        unreachable!()
+                    };
                 self.lint_buffer.buffer_lint_with_diagnostic(
                     AMBIGUOUS_GLOB_IMPORTS,
-                    import.root_id,
+                    root_id,
                     ambiguity_error.ident.span,
                     diag.msg.to_string(),
                     BuiltinLintDiagnostics::AmbiguousGlobImports { diag },
@@ -1604,6 +1609,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         let (b2_span, b2_note_msg, b2_help_msgs) = could_refer_to(b2, misc2, " also");
 
         AmbiguityErrorDiag {
+            extern_crate: kind == AmbiguityKind::ExternalCrate,
             msg: format!("`{ident}` is ambiguous"),
             span: ident.span,
             label_span: ident.span,
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
@@ -1407,11 +1407,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             };
             if self.is_accessible_from(binding.vis, scope) {
                 let imported_binding = self.import(binding, import);
-                let warn_ambiguity = self
-                    .resolution(import.parent_scope.module, key)
-                    .borrow()
-                    .binding()
-                    .is_some_and(|binding| binding.is_warn_ambiguity());
+                let warn_ambiguity = imported_binding.is_warn_ambiguity();
                 let _ = self.try_define(
                     import.parent_scope.module,
                     key,
@@ -1436,16 +1432,21 @@ 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;
-                if res != def::Res::Err && !error_ambiguity {
+                if res != def::Res::Err {
                     let mut reexport_chain = SmallVec::new();
                     let mut next_binding = binding;
                     while let NameBindingKind::Import { binding, import, .. } = next_binding.kind {
                         reexport_chain.push(import.simplify(this));
                         next_binding = binding;
                     }
-
-                    children.push(ModChild { ident, res, vis: binding.vis, reexport_chain });
+                    let ambiguity = binding.is_ambiguity();
+                    children.push(ModChild {
+                        ident,
+                        res,
+                        vis: binding.vis,
+                        reexport_chain,
+                        ambiguity,
+                    });
                 }
             });
 
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
@@ -729,6 +729,7 @@ enum AmbiguityKind {
     GlobVsGlob,
     GlobVsExpanded,
     MoreExpandedVsOuter,
+    ExternalCrate,
 }
 
 impl AmbiguityKind {
@@ -749,6 +750,7 @@ impl AmbiguityKind {
             AmbiguityKind::MoreExpandedVsOuter => {
                 "a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution"
             }
+            AmbiguityKind::ExternalCrate => "it has some error in extern crate",
         }
     }
 }
diff --git a/tests/ui/imports/ambiguous-2.rs b/tests/ui/imports/ambiguous-2.rs
@@ -6,4 +6,6 @@ extern crate ambiguous_1;
 
 fn main() {
     ambiguous_1::id();
+    //~^ WARNING `id` is ambiguous
+    //~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 }
diff --git a/tests/ui/imports/ambiguous-2.stderr b/tests/ui/imports/ambiguous-2.stderr
@@ -0,0 +1,18 @@
+warning: `id` is ambiguous
+  --> $DIR/ambiguous-2.rs:8:18
+   |
+LL |     ambiguous_1::id();
+   |                  ^^ ambiguous name
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #114095 <https://github.com/rust-lang/rust/issues/114095>
+   = note: ambiguous because of it has some error in extern crate
+note: `id` could refer to the function defined here
+  --> $DIR/auxiliary/../ambiguous-1.rs:24:9
+   |
+LL | pub use openssl::*;
+   |         ^^^^^^^
+   = note: `#[warn(ambiguous_glob_imports)]` on by default
+
+warning: 1 warning emitted
+
diff --git a/tests/ui/imports/ambiguous-4.rs b/tests/ui/imports/ambiguous-4.rs
@@ -5,5 +5,6 @@ extern crate ambiguous_4_extern;
 
 fn main() {
     ambiguous_4_extern::id();
-    // `warning_ambiguous` had been lost at metadata.
+    //~^ WARNING `id` is ambiguous
+    //~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 }
diff --git a/tests/ui/imports/ambiguous-4.stderr b/tests/ui/imports/ambiguous-4.stderr
@@ -0,0 +1,18 @@
+warning: `id` is ambiguous
+  --> $DIR/ambiguous-4.rs:7:25
+   |
+LL |     ambiguous_4_extern::id();
+   |                         ^^ ambiguous name
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #114095 <https://github.com/rust-lang/rust/issues/114095>
+   = note: ambiguous because of it has some error in extern crate
+note: `id` could refer to the function defined here
+  --> $DIR/auxiliary/../ambiguous-4-extern.rs:10:9
+   |
+LL | pub use evp::*;
+   |         ^^^
+   = note: `#[warn(ambiguous_glob_imports)]` on by default
+
+warning: 1 warning emitted
+
diff --git a/tests/ui/imports/extern-with-ambiguous-2.rs b/tests/ui/imports/extern-with-ambiguous-2.rs
@@ -12,5 +12,7 @@ mod s {
 use s::*;
 use extern_with_ambiguous_2_extern::*;
 use error::*;
+//~^ WARNING `error` is ambiguous
+//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 
 fn main() {}
diff --git a/tests/ui/imports/extern-with-ambiguous-2.stderr b/tests/ui/imports/extern-with-ambiguous-2.stderr
@@ -0,0 +1,25 @@
+warning: `error` is ambiguous
+  --> $DIR/extern-with-ambiguous-2.rs:14:5
+   |
+LL | use error::*;
+   |     ^^^^^ ambiguous name
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #114095 <https://github.com/rust-lang/rust/issues/114095>
+   = note: ambiguous because of multiple glob imports of a name in the same module
+note: `error` could refer to the module imported here
+  --> $DIR/extern-with-ambiguous-2.rs:12:5
+   |
+LL | use s::*;
+   |     ^^^^
+   = help: consider adding an explicit import of `error` to disambiguate
+note: `error` could also refer to the module imported here
+  --> $DIR/extern-with-ambiguous-2.rs:13:5
+   |
+LL | use extern_with_ambiguous_2_extern::*;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: consider adding an explicit import of `error` to disambiguate
+   = note: `#[warn(ambiguous_glob_imports)]` on by default
+
+warning: 1 warning emitted
+
diff --git a/tests/ui/imports/extern-with-ambiguous-3.rs b/tests/ui/imports/extern-with-ambiguous-3.rs
@@ -13,5 +13,7 @@ mod s {
 use s::*;
 use extern_with_ambiguous_3_extern::*;
 use error::*;
+//~^ WARNING `error` is ambiguous
+//~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 
 fn main() {}
diff --git a/tests/ui/imports/extern-with-ambiguous-3.stderr b/tests/ui/imports/extern-with-ambiguous-3.stderr
@@ -0,0 +1,25 @@
+warning: `error` is ambiguous
+  --> $DIR/extern-with-ambiguous-3.rs:15:5
+   |
+LL | use error::*;
+   |     ^^^^^ ambiguous name
+   |
+   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+   = note: for more information, see issue #114095 <https://github.com/rust-lang/rust/issues/114095>
+   = note: ambiguous because of multiple glob imports of a name in the same module
+note: `error` could refer to the module imported here
+  --> $DIR/extern-with-ambiguous-3.rs:13:5
+   |
+LL | use s::*;
+   |     ^^^^
+   = help: consider adding an explicit import of `error` to disambiguate
+note: `error` could also refer to the module imported here
+  --> $DIR/extern-with-ambiguous-3.rs:14:5
+   |
+LL | use extern_with_ambiguous_3_extern::*;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: consider adding an explicit import of `error` to disambiguate
+   = note: `#[warn(ambiguous_glob_imports)]` on by default
+
+warning: 1 warning emitted
+
diff --git a/tests/ui/imports/glob-conflict-cross-crate.rs b/tests/ui/imports/glob-conflict-cross-crate.rs
@@ -1,8 +1,13 @@
+// check-pass
 // aux-build:glob-conflict.rs
 
 extern crate glob_conflict;
 
 fn main() {
-    glob_conflict::f(); //~ ERROR cannot find function `f` in crate `glob_conflict`
-    glob_conflict::glob::f(); //~ ERROR cannot find function `f` in module `glob_conflict::glob`
+    glob_conflict::f();
+    //~^ WARNING `f` is ambiguous
+    //~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
+    glob_conflict::glob::f();
+    //~^ WARNING `f` is ambiguous
+    //~| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
 }
diff --git a/tests/ui/imports/glob-conflict-cross-crate.stderr b/tests/ui/imports/glob-conflict-cross-crate.stderr

Original file line number	Diff line number	Diff line change
`@@ -1048,7 +1048,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {`
`1048`	`1048`	`let res = Res::Def(self.def_kind(id), self.local_def_id(id));`
`1049`	`1049`	`let vis = self.get_visibility(id);`
`1050`	`1050`
`1051`		`- ModChild { ident, res, vis, reexport_chain: Default::default() }`
	`1051`	`+ ModChild { ident, res, vis, reexport_chain: Default::default(), ambiguity: false }`
`1052`	`1052`	`}`
`1053`	`1053`
`1054`	`1054`	`/// Iterates over all named children of the given module,`
Original file line number	Diff line number	Diff line change
`@@ -43,4 +43,6 @@ pub struct ModChild {`
`43`	`43`	`/// Reexport chain linking this module child to its original reexported item.`
`44`	`44`	`/// Empty if the module child is a proper item.`
`45`	`45`	`pub reexport_chain: SmallVec<[Reexport; 2]>,`
	`46`	`+ /// Is this an ambiguous binding?`
	`47`	`+ pub ambiguity: bool,`
`46`	`48`	`}`
Original file line number	Diff line number	Diff line change
`@@ -729,6 +729,7 @@ enum AmbiguityKind {`
`729`	`729`	`GlobVsGlob,`
`730`	`730`	`GlobVsExpanded,`
`731`	`731`	`MoreExpandedVsOuter,`
	`732`	`+ ExternalCrate,`
`732`	`733`	`}`
`733`	`734`
`734`	`735`	`impl AmbiguityKind {`
`@@ -749,6 +750,7 @@ impl AmbiguityKind {`
`749`	`750`	`AmbiguityKind::MoreExpandedVsOuter => {`
`750`	`751`	`"a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution"`
`751`	`752`	`}`
	`753`	`+ AmbiguityKind::ExternalCrate => "it has some error in extern crate",`
`752`	`754`	`}`
`753`	`755`	`}`
`754`	`756`	`}`
Original file line number	Diff line number	Diff line change
`@@ -6,4 +6,6 @@ extern crate ambiguous_1;`
`6`	`6`
`7`	`7`	`fn main() {`
`8`	`8`	`ambiguous_1::id();`
	`9`	+ //~^ WARNING `id` is ambiguous
	`10`	`+ //~\| WARNING this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!`
`9`	`11`	`}`