Skip to content

Commit

Permalink
Rollup merge of #135498 - compiler-errors:dyn-upcasting-completeness,…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
GuillaumeGomez authored Jan 15, 2025
2 parents b1d047b + bf545ce commit b1035d7
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 6 deletions.
8 changes: 5 additions & 3 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_trait_selection/src/solve/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_type_ir/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions tests/ui/traits/trait-upcasting/prefer-lower-candidates.rs
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit b1035d7

Please sign in to comment.