Rollup merge of #135498 - compiler-errors:dyn-upcasting-completeness, r=lcnr

Prefer lower `TraitUpcasting` candidates in selection

Fixes #135463. The underlying cause is this ambiguity, but it's more clear (and manifests as a coercion error, rather than a MIR validation error) when it's written the way I did in the UI test.

Sorry this is cursed r? lcnr

Author: GuillaumeGomez

compiler/rustc_next_trait_solver/src/solve/trait_goals.rs

@@ -741,12 +741,14 @@ where
                 a_data.principal(),
             ));
         } else if let Some(a_principal) = a_data.principal() {
-            for new_a_principal in
-                elaborate::supertraits(self.cx(), a_principal.with_self_ty(cx, a_ty)).skip(1)
+            for (idx, new_a_principal) in
+                elaborate::supertraits(self.cx(), a_principal.with_self_ty(cx, a_ty))
+                    .enumerate()
+                    .skip(1)
             {
                 responses.extend(self.consider_builtin_upcast_to_principal(
                     goal,
-                    CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting),
+                    CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting(idx)),
                     a_data,
                     a_region,
                     b_data,

compiler/rustc_trait_selection/src/solve/select.rs

@@ -117,6 +117,10 @@ fn candidate_should_be_dropped_in_favor_of<'tcx>(
             CandidateSource::BuiltinImpl(BuiltinImplSource::Object(a)),
             CandidateSource::BuiltinImpl(BuiltinImplSource::Object(b)),
         ) => a >= b,
+        (
+            CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting(a)),
+            CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting(b)),
+        ) => a >= b,
         // Prefer dyn candidates over non-dyn candidates. This is necessary to
         // handle the unsoundness between `impl<T: ?Sized> Any for T` and `dyn Any: Any`.
         (

compiler/rustc_trait_selection/src/traits/select/confirmation.rs

@@ -1090,7 +1090,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             )?
             .expect("did not expect ambiguity during confirmation");
 
-        Ok(ImplSource::Builtin(BuiltinImplSource::TraitUpcasting, nested))
+        Ok(ImplSource::Builtin(BuiltinImplSource::TraitUpcasting(idx), nested))
     }
 
     fn confirm_builtin_unsize_candidate(

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -1895,6 +1895,18 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
             Some(None) => {}
             None => return None,
         }
+        // Same for upcasting.
+        let upcast_bound = candidates
+            .iter()
+            .filter_map(|c| {
+                if let TraitUpcastingUnsizeCandidate(i) = c.candidate { Some(i) } else { None }
+            })
+            .try_reduce(|c1, c2| if has_non_region_infer { None } else { Some(c1.min(c2)) });
+        match upcast_bound {
+            Some(Some(index)) => return Some(TraitUpcastingUnsizeCandidate(index)),
+            Some(None) => {}
+            None => return None,
+        }
 
         // Finally, handle overlapping user-written impls.
         let impls = candidates.iter().filter_map(|c| {

compiler/rustc_type_ir/src/solve/mod.rs

@@ -177,8 +177,9 @@ pub enum BuiltinImplSource {
     /// A built-in implementation of `Upcast` for trait objects to other trait objects.
     ///
     /// This can be removed when `feature(dyn_upcasting)` is stabilized, since we only
-    /// use it to detect when upcasting traits in hir typeck.
-    TraitUpcasting,
+    /// use it to detect when upcasting traits in hir typeck. The index is only used
+    /// for winnowing.
+    TraitUpcasting(usize),
     /// Unsizing a tuple like `(A, B, ..., X)` to `(A, B, ..., Y)` if `X` unsizes to `Y`.
     ///
     /// This can be removed when `feature(tuple_unsizing)` is stabilized, since we only

tests/ui/traits/trait-upcasting/prefer-lower-candidates.rs

@@ -0,0 +1,29 @@
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
+//@ check-pass
+
+// Ensure we don't have ambiguity when upcasting to two supertraits
+// that are identical modulo normalization.
+
+#![feature(trait_upcasting)]
+
+trait Supertrait<T> {
+    fn method(&self) {}
+}
+impl<T> Supertrait<T> for () {}
+
+trait Identity {
+    type Selff;
+}
+impl<Selff> Identity for Selff {
+    type Selff = Selff;
+}
+trait Trait<P>: Supertrait<()> + Supertrait<<P as Identity>::Selff> {}
+
+impl<P> Trait<P> for () {}
+
+fn main() {
+    let x: &dyn Trait<()> = &();
+    let x: &dyn Supertrait<()> = x;
+}