resolve: Partially convert ambiguous_glob_imports lint into a hard error

compiler/rustc_codegen_gcc/build_system/src/test.rs

@@ -826,8 +826,6 @@ fn valid_ui_error_pattern_test(file: &str) -> bool {
         "type-alias-impl-trait/auxiliary/cross_crate_ice.rs",
         "type-alias-impl-trait/auxiliary/cross_crate_ice2.rs",
         "macros/rfc-2011-nicer-assert-messages/auxiliary/common.rs",
-        "imports/ambiguous-1.rs",
-        "imports/ambiguous-4-extern.rs",
         "entry-point/auxiliary/bad_main_functions.rs",
     ]
     .iter()

compiler/rustc_lint_defs/src/builtin.rs

@@ -4436,7 +4436,7 @@ declare_lint! {
     ///
     /// ### Example
     ///
-    /// ```rust,compile_fail
+    /// ```rust,ignore (needs extern crate)
     /// #![deny(ambiguous_glob_imports)]
     /// pub fn foo() -> u32 {
     ///     use sub::*;
@@ -4452,8 +4452,6 @@ declare_lint! {
     /// }
     /// ```
     ///
-    /// {{produces}}
-    ///
     /// ### Explanation
     ///
     /// Previous versions of Rust compile it successfully because it

compiler/rustc_resolve/src/build_reduced_graph.rs

@@ -53,7 +53,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         ns: Namespace,
         binding: NameBinding<'ra>,
     ) {
-        if let Err(old_binding) = self.try_define_local(parent, ident, ns, binding, false) {
+        if let Err(old_binding) = self.try_define_local(parent, ident, ns, binding) {
             self.report_conflict(parent, ident, ns, old_binding, binding);
         }
     }
@@ -82,13 +82,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         vis: Visibility<DefId>,
         span: Span,
         expansion: LocalExpnId,
-        ambiguity: Option<(NameBinding<'ra>, AmbiguityKind)>,
+        ambiguity: Option<(NameBinding<'ra>, AmbiguityKind, bool)>,
     ) {
         let binding = self.arenas.alloc_name_binding(NameBindingData {
             kind: NameBindingKind::Res(res),
             ambiguity,
-            // External ambiguities always report the `AMBIGUOUS_GLOB_IMPORTS` lint at the moment.
-            warn_ambiguity: true,
             vis,
             span,
             expansion,
@@ -288,7 +286,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 AmbigModChildKind::GlobVsGlob => AmbiguityKind::GlobVsGlob,
                 AmbigModChildKind::GlobVsExpanded => AmbiguityKind::GlobVsExpanded,
             };
-            (self.arenas.new_res_binding(res, vis, span, expansion), ambig_kind)
+            // External ambiguities always report the `AMBIGUOUS_GLOB_IMPORTS` lint at the moment.
+            (self.arenas.new_res_binding(res, vis, span, expansion), ambig_kind, true)
         });
 
         // Record primary definitions.

compiler/rustc_resolve/src/diagnostics.rs

@@ -1137,7 +1137,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         .emit()
     }
 
-    fn def_path_str(&self, mut def_id: DefId) -> String {
+    pub(crate) fn def_path_str(&self, mut def_id: DefId) -> String {
         // We can't use `def_path_str` in resolve.
         let mut path = vec![def_id];
         while let Some(parent) = self.tcx.opt_parent(def_id) {
@@ -1995,6 +1995,25 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         }
     }
 
+    pub(crate) fn remove_same_import(
+        b1: NameBinding<'ra>,
+        b2: NameBinding<'ra>,
+    ) -> (NameBinding<'ra>, NameBinding<'ra>) {
+        if let NameBindingKind::Import { import: import1, binding: b1_next } = b1.kind
+            && let NameBindingKind::Import { import: import2, binding: b2_next } = b2.kind
+            && import1 == import2
+            && b1.ambiguity == b2.ambiguity
+        {
+            assert!(b1.ambiguity.is_none());
+            assert_eq!(b1.expansion, b2.expansion);
+            assert_eq!(b1.span, b2.span);
+            assert_eq!(b1.vis, b2.vis);
+            Self::remove_same_import(b1_next, b2_next)
+        } else {
+            (b1, b2)
+        }
+    }
+
     fn ambiguity_diagnostic(&self, ambiguity_error: &AmbiguityError<'ra>) -> errors::Ambiguity {
         let AmbiguityError { kind, ident, b1, b2, misc1, misc2, .. } = *ambiguity_error;
         let extern_prelude_ambiguity = || {
@@ -2003,6 +2022,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                     && entry.flag_binding.as_ref().and_then(|pb| pb.get().0.binding()) == Some(b2)
             })
         };
+        let (b1, b2) = Self::remove_same_import(b1, b2);
         let (b1, b2, misc1, misc2, swapped) = if b2.span.is_dummy() && !b1.span.is_dummy() {
             // We have to print the span-less alternative first, otherwise formatting looks bad.
             (b2, b1, misc2, misc1, true)

compiler/rustc_resolve/src/effective_visibilities.rs

@@ -125,27 +125,13 @@ 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 =
-                |binding: NameBinding<'ra>, warn: bool| binding.ambiguity.is_some() && !warn;
             let mut parent_id = ParentId::Def(module_id);
-            let mut warn_ambiguity = binding.warn_ambiguity;
             while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
                 self.update_import(binding, parent_id);
-
-                if is_ambiguity(binding, warn_ambiguity) {
-                    // Stop at the root ambiguity, further bindings in the chain should not
-                    // be reexported because the root ambiguity blocks any access to them.
-                    // (Those further bindings are most likely not ambiguities themselves.)
-                    break;
-                }
-
                 parent_id = ParentId::Import(binding);
                 binding = nested_binding;
-                warn_ambiguity |= nested_binding.warn_ambiguity;
             }
-            if !is_ambiguity(binding, warn_ambiguity)
-                && let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local())
-            {
+            if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
                 self.update_def(def_id, binding.vis.expect_local(), parent_id);
             }
         }