Auto merge of #157710 - theemathas:no-dup-bound-in-dyn, r=<try>

Prohibit conflicting bounds in `dyn`, even with different generics.

Author: bors

compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_trait.rs

@@ -1,5 +1,7 @@
+use std::assert_matches;
+
 use rustc_ast::TraitObjectSyntax;
-use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_errors::codes::*;
 use rustc_errors::{
     Applicability, Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, StashKey,
@@ -11,8 +13,8 @@ use rustc_hir::{self as hir, HirId, LangItem};
 use rustc_lint_defs::builtin::{BARE_TRAIT_OBJECTS, UNUSED_ASSOCIATED_TYPE_BOUNDS};
 use rustc_middle::ty::elaborate::ClauseWithSupertraitSpan;
 use rustc_middle::ty::{
-    self, BottomUpFolder, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable,
-    TypeVisitableExt, Upcast,
+    self, AliasTermKind, BottomUpFolder, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt,
+    TypeFoldable, TypeVisitableExt, Upcast,
 };
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::{ErrorGuaranteed, Span};
@@ -69,7 +71,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 dummy_self,
                 &mut user_written_bounds,
                 PredicateFilter::SelfOnly,
-                OverlappingAsssocItemConstraints::Forbidden,
+                OverlappingAsssocItemConstraints::Allowed,
             );
             if let Err(GenericArgCountMismatch { invalid_args, .. }) = result.correct {
                 potential_assoc_items.extend(invalid_args);
@@ -87,6 +89,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             span,
         );
 
+        // Note: This only includes elaboration due to trait aliases.
+        // It does not include elaboration due to supertraits.
         let (mut elaborated_trait_bounds, elaborated_projection_bounds) =
             traits::expand_trait_aliases(tcx, user_written_bounds.iter().copied());
 
@@ -177,7 +181,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             );
             if let Some((old_proj, old_proj_span)) =
                 projection_bounds.insert(key, (proj, proj_span))
-                && tcx.anonymize_bound_vars(proj) != tcx.anonymize_bound_vars(old_proj)
+                && proj != old_proj
             {
                 let kind = tcx.def_descr(item_def_id);
                 let name = tcx.item_name(item_def_id);
@@ -204,6 +208,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         // We achieve a stable ordering by walking over the unsubstituted principal trait ref.
         let mut ordered_associated_items = vec![];
 
+        // Detect conflicting bounds from expanding supertraits.
+        //
+        // We need to prohibit conflicting bounds from the same item_def_id,
+        // even if they have different generics. This is because those generics might
+        // end up being instantiated with the same concrete type, causing unsoundness.
+        // See https://github.com/rust-lang/rust/issues/154662.
+        //
+        // This check could be more lenient, allowing conflicting bounds on different
+        // generics that we know for sure cannot be instantiated into identical concrete
+        // types. But for now, we're being conservative.
+        let mut seen_projection_bounds_ignoring_generics = FxHashMap::default();
+
         if let Some((principal_trait, ref spans)) = principal_trait {
             let principal_trait = principal_trait.map_bound(|trait_pred| {
                 assert_eq!(trait_pred.polarity, ty::PredicatePolarity::Positive);
@@ -240,6 +256,38 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                         );
                     }
                     ty::ClauseKind::Projection(pred) => {
+                        // See the comment above `let seen_projection_bounds_ignoring_generics`
+                        let kind = pred.projection_term.kind;
+                        assert_matches!(
+                            kind,
+                            AliasTermKind::ProjectionTy { .. }
+                                | AliasTermKind::ProjectionConst { .. },
+                            "Unexpected projection kind in lower_trait_object_ty"
+                        );
+                        let term = pred.term;
+                        // This clause specifies that the `kind` is equal to `term`.
+                        // We record this, and check for duplicates.
+                        if let Some(old_term) =
+                            seen_projection_bounds_ignoring_generics.insert(kind, term)
+                            && old_term != term
+                        {
+                            let name = tcx.item_name(kind.def_id());
+                            self.dcx()
+                                .struct_span_err(
+                                    span,
+                                    format!(
+                                        "conflicting {} bindings for `{}`",
+                                        kind.descr(),
+                                        name,
+                                    ),
+                                )
+                                // FIXME: Improve diagnostics by pointing to
+                                // where the bound is specified.
+                                .with_note(format!("`{name}` is specified to be `{old_term}`"))
+                                .with_note(format!("`{name}` is also specified to be `{term}`"))
+                                .emit();
+                        }
+
                         let pred = bound_predicate.rebind(pred);
                         // A `Self` within the original bound will be instantiated with a
                         // `trait_object_dummy_self`, so check for that.
@@ -285,6 +333,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 }
             }
         }
+        drop(seen_projection_bounds_ignoring_generics);
 
         // Flag assoc item bindings that didn't really need to be specified.
         for &(projection_bound, span) in projection_bounds.values() {

tests/ui/associated-type-bounds/conflicting-bounds-different-generics-simple.rs

@@ -0,0 +1,33 @@
+// We previously accepted conflicting associated type bounds with different generics,
+// which resulted in some weirdness.
+// See https://github.com/rust-lang/rust/issues/154662
+
+trait Dummy {
+    type DummyAssoc1;
+    type DummyAssoc2;
+}
+struct DummyStruct;
+impl Dummy for DummyStruct {
+    type DummyAssoc1 = i16;
+    type DummyAssoc2 = i16;
+}
+
+trait Super<T> {
+    type Assoc;
+}
+
+trait Sub<D: Dummy>: Super<D::DummyAssoc1, Assoc = i32> + Super<D::DummyAssoc2, Assoc = i64> {}
+
+fn require_trait<D: Dummy, U: Super<D::DummyAssoc1> + ?Sized>() {}
+
+fn use_dyn<D: Dummy>() {
+    require_trait::<D, dyn Sub<D>>();
+    //^ ERROR conflicting associated type bindings for `Assoc`
+}
+
+fn main() {
+    // This ends up proving that `dyn Sub<DummyStruct>` implements `Super<i16>`.
+    // However, `dyn Sub<DummyStruct>` has bounds for both `Assoc = i32` and `Assoc = i64`,
+    // which is nonsense.
+    use_dyn::<DummyStruct>();
+}

tests/ui/associated-type-bounds/conflicting-bounds-different-generics-simple.stderr

@@ -0,0 +1,11 @@
+error: conflicting associated type bindings for `Assoc`
+  --> $DIR/conflicting-bounds-different-generics-simple.rs:24:24
+   |
+LL |     require_trait::<D, dyn Sub<D>>();
+   |                        ^^^^^^^^^^
+   |
+   = note: `Assoc` is specified to be `i64`
+   = note: `Assoc` is also specified to be `i32`
+
+error: aborting due to 1 previous error
+

tests/ui/associated-type-bounds/conflicting-bounds-different-generics-unsound.rs

@@ -0,0 +1,54 @@
+// We previously accepted conflicting associated type bounds with different generics,
+// which resulted in unsoundness.
+// See https://github.com/rust-lang/rust/issues/154662
+
+type Payload = Box<i32>;
+type Src<'a> = &'a Payload;
+type Dst = &'static Payload;
+
+trait Super<T> {
+    type Assoc;
+}
+
+trait Sub<'a, A1, A2>: Super<A1, Assoc = Src<'a>> + Super<A2, Assoc = Dst> {}
+
+trait Callback<A1, A2> {
+    fn callback<U: Super<A1> + Super<A2> + ?Sized>(
+        payload: <U as Super<A1>>::Assoc,
+    ) -> <U as Super<A2>>::Assoc;
+}
+struct CallbackStruct;
+impl Callback<i16, i16> for CallbackStruct {
+    fn callback<U: Super<i16> + ?Sized>(payload: U::Assoc) -> U::Assoc {
+        payload
+    }
+}
+
+fn require_trait<
+    'a,
+    A1,
+    A2,
+    U: Super<A1, Assoc = Src<'a>> + Super<A2, Assoc = Dst> + ?Sized,
+    C: Callback<A1, A2>,
+>(
+    payload: Src<'a>,
+) -> Dst {
+    C::callback::<U>(payload)
+}
+
+fn use_dyn<'a, A1, A2, C: Callback<A1, A2>>(payload: Src<'a>) -> Dst {
+    require_trait::<'a, A1, A2, dyn Sub<'a, A1, A2>, C>(payload)
+    //^ ERROR conflicting associated type bindings for `Assoc`
+}
+
+fn extend<'a>(payload: Src<'a>) -> Dst {
+    // `dyn Sub<'a, i16, i16>` has both an `Assoc = Src<'a>` bound and an `Assoc = Dst` bound.
+    use_dyn::<i16, i16, CallbackStruct>(payload)
+}
+
+fn main() {
+    let payload: Box<Payload> = Box::new(Box::new(1));
+    let wrong: &'static Payload = extend(&*payload);
+    drop(payload);
+    println!("{wrong}");
+}

tests/ui/associated-type-bounds/conflicting-bounds-different-generics-unsound.stderr

@@ -0,0 +1,11 @@
+error: conflicting associated type bindings for `Assoc`
+  --> $DIR/conflicting-bounds-different-generics-unsound.rs:40:33
+   |
+LL |     require_trait::<'a, A1, A2, dyn Sub<'a, A1, A2>, C>(payload)
+   |                                 ^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `Assoc` is specified to be `&'static Box<i32>`
+   = note: `Assoc` is also specified to be `&'a Box<i32>`
+
+error: aborting due to 1 previous error
+

tests/ui/associated-type-bounds/duplicate-bound-err.rs

@@ -1,10 +1,6 @@
 //@ edition: 2024
 
-#![feature(
-    min_generic_const_args,
-    type_alias_impl_trait,
-    return_type_notation
-)]
+#![feature(min_generic_const_args, type_alias_impl_trait, return_type_notation)]
 #![expect(incomplete_features)]
 #![allow(refining_impl_trait_internal)]
 
@@ -77,27 +73,21 @@ fn uncallable(_: impl Iterator<Item = i32, Item = u32>) {}
 
 fn uncallable_const(_: impl Trait<ASSOC = 3, ASSOC = 4>) {}
 
-fn uncallable_rtn(
-    _: impl Trait<foo(..): Trait<ASSOC = 3>, foo(..): Trait<ASSOC = 4>>
-) {}
+fn uncallable_rtn(_: impl Trait<foo(..): Trait<ASSOC = 3>, foo(..): Trait<ASSOC = 4>>) {}
 
 type MustFail = dyn Iterator<Item = i32, Item = u32>;
-//~^ ERROR [E0719]
-//~| ERROR conflicting associated type bindings
+//~^ ERROR conflicting associated type bindings
 
 trait Trait2 {
     type const ASSOC: u32;
 }
 
 type MustFail2 = dyn Trait2<ASSOC = 3u32, ASSOC = 4u32>;
-//~^ ERROR [E0719]
-//~| ERROR conflicting associated constant bindings
+//~^ ERROR conflicting associated constant bindings
 
-type MustFail3 = dyn Iterator<Item = i32, Item = i32>;
-//~^ ERROR [E0719]
+type Allowed = dyn Iterator<Item = i32, Item = i32>;
 
-type MustFail4 = dyn Trait2<ASSOC = 3u32, ASSOC = 3u32>;
-//~^ ERROR [E0719]
+type Allowed2 = dyn Trait2<ASSOC = 3u32, ASSOC = 3u32>;
 
 trait Trait3 {
     fn foo() -> impl Iterator<Item = i32, Item = u32>;