Rollup merge of #140684 - compiler-errors:unnecessary-assoc, r=lcnr

Only include `dyn Trait<Assoc = ...>` associated type bounds for `Self: Sized` associated types if they are provided

Since #136458, we began filtering out associated types with `Self: Sized` bounds when constructing the list of associated type bounds to put into our `dyn Trait` types. For example, given:

```rust
trait Trait {
    type Assoc where Self: Sized;
}
```

After #136458, even if a user writes `dyn Trait<Assoc = ()>`, the lowered ty would have an empty projection list, and thus be equivalent to `dyn Trait`. However, this has the side effect of no longer constraining any types in the RHS of `Assoc = ...`, not implying any WF implied bounds, and not requiring that they hold when unsizing.

After this PR, we include these bounds, but (still) do not require that they are provided. If the are not provided, they are skipped from the projections list.

This results in `dyn Trait` types that have differing numbers of projection bounds. This will lead to re-introducing type mismatches e.g. between `dyn Trait` and `dyn Trait<Assoc = ()>`. However, this is expected and doesn't suffer from any of the deduplication unsoundness from before #136458.

We may want to begin to ignore thse bounds in the future by bumping `unused_associated_type_bounds` to an FCW. I don't want to tangle that up into the fix that was originally intended in #136458, so I'm doing a "fix-forward" in this PR and deferring thinking about this for the future.

Fixes #140645

r? lcnr

Author: matthiaskrgr

compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs

@@ -172,7 +172,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
 
         let principal_trait = regular_traits.into_iter().next();
 
-        let mut needed_associated_types = vec![];
+        // A stable ordering of associated types from the principal trait and all its
+        // supertraits. We use this to ensure that different substitutions of a trait
+        // don't result in `dyn Trait` types with different projections lists, which
+        // can be unsound: <https://github.com/rust-lang/rust/pull/136458>.
+        // We achieve a stable ordering by walking over the unsubstituted principal
+        // trait ref.
+        let mut ordered_associated_types = vec![];
+
         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);
@@ -197,16 +204,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                         // FIXME(negative_bounds): Handle this correctly...
                         let trait_ref =
                             tcx.anonymize_bound_vars(bound_predicate.rebind(pred.trait_ref));
-                        needed_associated_types.extend(
+                        ordered_associated_types.extend(
                             tcx.associated_items(pred.trait_ref.def_id)
                                 .in_definition_order()
                                 // We only care about associated types.
                                 .filter(|item| item.is_type())
                                 // No RPITITs -- they're not dyn-compatible for now.
                                 .filter(|item| !item.is_impl_trait_in_trait())
-                                // If the associated type has a `where Self: Sized` bound,
-                                // we do not need to constrain the associated type.
-                                .filter(|item| !tcx.generics_require_sized_self(item.def_id))
                                 .map(|item| (item.def_id, trait_ref)),
                         );
                     }
@@ -278,14 +282,26 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             }
         }
 
+        // We compute the list of projection bounds taking the ordered associated types,
+        // and check if there was an entry in the collected `projection_bounds`. Those
+        // are computed by first taking the user-written associated types, then elaborating
+        // the principal trait ref, and only using those if there was no user-written.
+        // See note below about how we handle missing associated types with `Self: Sized`,
+        // which are not required to be provided, but are still used if they are provided.
         let mut missing_assoc_types = FxIndexSet::default();
-        let projection_bounds: Vec<_> = needed_associated_types
+        let projection_bounds: Vec<_> = ordered_associated_types
             .into_iter()
             .filter_map(|key| {
                 if let Some(assoc) = projection_bounds.get(&key) {
                     Some(*assoc)
                 } else {
-                    missing_assoc_types.insert(key);
+                    // If the associated type has a `where Self: Sized` bound, then
+                    // we do not need to provide the associated type. This results in
+                    // a `dyn Trait` type that has a different number of projection
+                    // bounds, which may lead to type mismatches.
+                    if !tcx.generics_require_sized_self(key.0) {
+                        missing_assoc_types.insert(key);
+                    }
                     None
                 }
             })

compiler/rustc_middle/src/ty/sty.rs

@@ -723,7 +723,10 @@ impl<'tcx> Ty<'tcx> {
         repr: DynKind,
     ) -> Ty<'tcx> {
         if cfg!(debug_assertions) {
-            let projection_count = obj.projection_bounds().count();
+            let projection_count = obj
+                .projection_bounds()
+                .filter(|item| !tcx.generics_require_sized_self(item.item_def_id()))
+                .count();
             let expected_count: usize = obj
                 .principal_def_id()
                 .into_iter()

tests/ui/traits/object/constrain-via-unnecessary-bound.rs

@@ -0,0 +1,24 @@
+//@ check-pass
+
+// Regression test for <https://github.com/rust-lang/rust/issues/140645>.
+// Test that we lower impossible-to-satisfy associated type bounds, which
+// may for example constrain impl parameters.
+
+pub trait Other {}
+
+pub trait Trait {
+    type Assoc
+    where
+        Self: Sized;
+}
+
+impl Other for dyn Trait {}
+// `dyn Trait<Assoc = ()>` is a different "nominal type" than `dyn Trait`.
+impl Other for dyn Trait<Assoc = ()> {}
+//~^ WARN unnecessary associated type bound for dyn-incompatible associated type
+
+// I hope it's clear that `dyn Trait` (w/o `Assoc`) wouldn't match this impl.
+impl<T> dyn Trait<Assoc = T> {}
+//~^ WARN unnecessary associated type bound for dyn-incompatible associated type
+
+fn main() {}

tests/ui/traits/object/constrain-via-unnecessary-bound.stderr

@@ -0,0 +1,19 @@
+warning: unnecessary associated type bound for dyn-incompatible associated type
+  --> $DIR/constrain-via-unnecessary-bound.rs:17:26
+   |
+LL | impl Other for dyn Trait<Assoc = ()> {}
+   |                          ^^^^^^^^^^ help: remove this bound
+   |
+   = note: this associated type has a `where Self: Sized` bound, and while the associated type can be specified, it cannot be used because trait objects are never `Sized`
+   = note: `#[warn(unused_associated_type_bounds)]` on by default
+
+warning: unnecessary associated type bound for dyn-incompatible associated type
+  --> $DIR/constrain-via-unnecessary-bound.rs:21:19
+   |
+LL | impl<T> dyn Trait<Assoc = T> {}
+   |                   ^^^^^^^^^ help: remove this bound
+   |
+   = note: this associated type has a `where Self: Sized` bound, and while the associated type can be specified, it cannot be used because trait objects are never `Sized`
+
+warning: 2 warnings emitted
+

tests/ui/traits/object/pretty.stderr

@@ -154,12 +154,12 @@ error[E0308]: mismatched types
   --> $DIR/pretty.rs:41:56
    |
 LL | fn dyn_has_gat(x: &dyn HasGat<u8, Assoc<bool> = ()>) { x }
-   |                                                     -  ^ expected `()`, found `&dyn HasGat<u8>`
+   |                                                     -  ^ expected `()`, found `&dyn HasGat<u8, Assoc<bool> = ()>`
    |                                                     |
-   |                                                     help: try adding a return type: `-> &dyn HasGat<u8>`
+   |                                                     help: try adding a return type: `-> &dyn HasGat<u8, Assoc<bool> = ()>`
    |
    = note: expected unit type `()`
-              found reference `&dyn HasGat<u8>`
+              found reference `&dyn HasGat<u8, Assoc<bool> = ()>`
 
 error: aborting due to 14 previous errors; 1 warning emitted