Rollup merge of #150939 - impasse, r=estebank

resolve: Relax some asserts in glob overwriting and add tests

Fixes #150927.
Fixes #150928.
Fixes #150929.
Fixes #150976.
Fixes #150977.
Fixes #151008.
Fixes #151037.
Fixes rust-lang/rust-clippy#16398.
r? @yaahc

Author: JonathanBrouwer

compiler/rustc_resolve/src/diagnostics.rs

@@ -212,12 +212,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         &mut self,
         ident: Ident,
         ns: Namespace,
-        new_binding: Decl<'ra>,
         old_binding: Decl<'ra>,
+        new_binding: Decl<'ra>,
     ) {
         // Error on the second of two conflicting names
         if old_binding.span.lo() > new_binding.span.lo() {
-            return self.report_conflict(ident, ns, old_binding, new_binding);
+            return self.report_conflict(ident, ns, new_binding, old_binding);
         }
 
         let container = match old_binding.parent_module.unwrap().kind {

compiler/rustc_resolve/src/imports.rs

@@ -300,13 +300,16 @@ 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.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());
+        if d1.ambiguity.get() != d2.ambiguity.get() {
+            assert!(d1.ambiguity.get().is_some());
+            assert!(d2.ambiguity.get().is_none());
+        }
+        // Visibility of the new import declaration may be different,
+        // because it already incorporates the visibility of the source binding.
+        // `warn_ambiguity` of a re-fetched glob can also change in both directions.
         remove_same_import(d1_next, d2_next)
     } else {
         (d1, d2)
@@ -348,8 +351,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     /// decide which one to keep.
     fn select_glob_decl(
         &self,
-        glob_decl: Decl<'ra>,
         old_glob_decl: Decl<'ra>,
+        glob_decl: Decl<'ra>,
         warn_ambiguity: bool,
     ) -> Decl<'ra> {
         assert!(glob_decl.is_glob_import());
@@ -369,14 +372,18 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         // with the re-fetched decls.
         // This is probably incorrect in corner cases, and the outdated decls still get
         // propagated to other places and get stuck there, but that's what we have at the moment.
-        let (deep_decl, old_deep_decl) = remove_same_import(glob_decl, old_glob_decl);
+        let (old_deep_decl, deep_decl) = remove_same_import(old_glob_decl, glob_decl);
         if deep_decl != glob_decl {
             // Some import layers have been removed, need to overwrite.
             assert_ne!(old_deep_decl, old_glob_decl);
             // 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 old_glob_decl.ambiguity.get().is_some() && glob_decl.ambiguity.get().is_none() {
+                // Do not lose glob ambiguities when re-fetching the glob.
+                glob_decl.ambiguity.set_unchecked(old_glob_decl.ambiguity.get());
+            }
             if glob_decl.is_ambiguity_recursive() {
                 glob_decl.warn_ambiguity.set_unchecked(true);
             }
@@ -436,7 +443,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 match (old_decl.is_glob_import(), decl.is_glob_import()) {
                     (true, true) => {
                         resolution.glob_decl =
-                            Some(this.select_glob_decl(decl, old_decl, warn_ambiguity));
+                            Some(this.select_glob_decl(old_decl, decl, warn_ambiguity));
                     }
                     (old_glob @ true, false) | (old_glob @ false, true) => {
                         let (glob_decl, non_glob_decl) =
@@ -446,7 +453,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                             && old_glob_decl != glob_decl
                         {
                             resolution.glob_decl =
-                                Some(this.select_glob_decl(glob_decl, old_glob_decl, false));
+                                Some(this.select_glob_decl(old_glob_decl, glob_decl, false));
                         } else {
                             resolution.glob_decl = Some(glob_decl);
                         }

tests/ui/hygiene/cross-crate-redefine.rs

@@ -8,7 +8,7 @@ extern crate use_by_macro;
 use use_by_macro::*;
 
 my_struct!(define);
-//~^ ERROR the name `MyStruct` is defined multiple times
 my_struct!(define);
+//~^ ERROR the name `MyStruct` is defined multiple times
 
 fn main() {}

tests/ui/hygiene/cross-crate-redefine.stderr

@@ -1,11 +1,10 @@
 error[E0428]: the name `MyStruct` is defined multiple times
-  --> $DIR/cross-crate-redefine.rs:10:1
+  --> $DIR/cross-crate-redefine.rs:11:1
    |
-LL | my_struct!(define);
-   | ^^^^^^^^^^^^^^^^^^ `MyStruct` redefined here
-LL |
 LL | my_struct!(define);
    | ------------------ previous definition of the type `MyStruct` here
+LL | my_struct!(define);
+   | ^^^^^^^^^^^^^^^^^^ `MyStruct` redefined here
    |
    = note: `MyStruct` must be defined only once in the type namespace of this module
    = note: this error originates in the macro `my_struct` (in Nightly builds, run with -Z macro-backtrace for more info)

tests/ui/imports/overwrite-deep-glob.rs

@@ -0,0 +1,22 @@
+//@ check-pass
+
+mod openssl {
+    pub use self::handwritten::*;
+
+    mod handwritten {
+        mod m1 {
+            pub struct S {}
+        }
+        mod m2 {
+            #[derive(Default)]
+            pub struct S {}
+        }
+
+        pub use self::m1::*; //~ WARN ambiguous glob re-exports
+        pub use self::m2::*;
+    }
+}
+
+pub use openssl::*;
+
+fn main() {}

tests/ui/imports/overwrite-deep-glob.stderr

@@ -0,0 +1,12 @@
+warning: ambiguous glob re-exports
+  --> $DIR/overwrite-deep-glob.rs:15:17
+   |
+LL |         pub use self::m1::*;
+   |                 ^^^^^^^^^^^ the name `S` in the type namespace is first re-exported here
+LL |         pub use self::m2::*;
+   |                 ----------- but the name `S` in the type namespace is also re-exported here
+   |
+   = note: `#[warn(ambiguous_glob_reexports)]` on by default
+
+warning: 1 warning emitted
+

tests/ui/imports/overwrite-different-ambig-2.rs

@@ -0,0 +1,24 @@
+mod m1 {
+    mod inner {
+        pub struct S {}
+    }
+    pub use self::inner::*;
+
+    #[derive(Debug)]
+    pub struct S {}
+}
+
+mod m2 {
+    pub struct S {}
+}
+
+// First we have a glob ambiguity in this glob (with `m2::*`).
+// Then we re-fetch `m1::*` because non-glob `m1::S` materializes from derive,
+// and we need to make sure that the glob ambiguity is not lost during re-fetching.
+use m1::*;
+use m2::*;
+
+fn main() {
+    let _: m1::S = S {}; //~ ERROR `S` is ambiguous
+                         //~| WARN this was previously accepted
+}

tests/ui/imports/overwrite-different-ambig-2.stderr

@@ -0,0 +1,49 @@
+error: `S` is ambiguous
+  --> $DIR/overwrite-different-ambig-2.rs:22:20
+   |
+LL |     let _: m1::S = S {};
+   |                    ^ 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: `S` could refer to the struct imported here
+  --> $DIR/overwrite-different-ambig-2.rs:18:5
+   |
+LL | use m1::*;
+   |     ^^^^^
+   = help: consider adding an explicit import of `S` to disambiguate
+note: `S` could also refer to the struct imported here
+  --> $DIR/overwrite-different-ambig-2.rs:19:5
+   |
+LL | use m2::*;
+   |     ^^^^^
+   = help: consider adding an explicit import of `S` to disambiguate
+   = note: `#[deny(ambiguous_glob_imports)]` (part of `#[deny(future_incompatible)]`) on by default
+
+error: aborting due to 1 previous error
+
+Future incompatibility report: Future breakage diagnostic:
+error: `S` is ambiguous
+  --> $DIR/overwrite-different-ambig-2.rs:22:20
+   |
+LL |     let _: m1::S = S {};
+   |                    ^ 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: `S` could refer to the struct imported here
+  --> $DIR/overwrite-different-ambig-2.rs:18:5
+   |
+LL | use m1::*;
+   |     ^^^^^
+   = help: consider adding an explicit import of `S` to disambiguate
+note: `S` could also refer to the struct imported here
+  --> $DIR/overwrite-different-ambig-2.rs:19:5
+   |
+LL | use m2::*;
+   |     ^^^^^
+   = help: consider adding an explicit import of `S` to disambiguate
+   = note: `#[deny(ambiguous_glob_imports)]` (part of `#[deny(future_incompatible)]`) on by default
+

tests/ui/imports/overwrite-different-ambig.rs

@@ -0,0 +1,25 @@
+//@ check-pass
+//@ edition:2024
+
+mod a {
+    mod b {
+        mod c {
+            pub struct E;
+        }
+        mod d {
+            mod c {
+                pub struct E;
+            }
+            mod d {
+                #[derive(Debug)]
+                pub struct E;
+            }
+            pub use c::*;
+            use d::*;
+        }
+        use c::*;
+        use d::*;
+    }
+}
+
+fn main() {}

tests/ui/imports/overwrite-different-vis.rs

@@ -0,0 +1,21 @@
+//@ check-pass
+
+mod b {
+    pub mod http {
+        pub struct HeaderMap;
+    }
+
+    pub(crate) use self::http::*;
+    #[derive(Debug)]
+    pub struct HeaderMap;
+}
+
+mod a {
+    pub use crate::b::*;
+
+    fn check_type() {
+        let _: HeaderMap = crate::b::HeaderMap;
+    }
+}
+
+fn main() {}