Skip to content

Commit

Permalink
Auto merge of #114036 - compiler-errors:upcast-to-fewer-assocs, r=lcnr
Browse files Browse the repository at this point in the history
Rework upcasting confirmation to support upcasting to fewer projections in target bounds

This PR implements a modified trait upcasting algorithm that is resilient to changes in the number of associated types in the bounds of the source and target trait objects.

It does this by equating each bound of the target trait ref individually against the bounds of the source trait ref, rather than doing them all together by constructing a new trait object.

#### The new way we do trait upcasting confirmation

1. Equate the target trait object's principal trait ref with one of the supertraits of the source trait object's principal.
https://github.com/rust-lang/rust/blob/fdcab310b2a57a4e9cc0b2629abd27afda49cd80/compiler/rustc_trait_selection/src/traits/select/mod.rs#L2509-L2525

2. Make sure that every auto trait in the *target* trait object is present in the source trait ref's bounds.
https://github.com/rust-lang/rust/blob/fdcab310b2a57a4e9cc0b2629abd27afda49cd80/compiler/rustc_trait_selection/src/traits/select/mod.rs#L2559-L2562

3. For each projection in the *target* trait object, make sure there is exactly one projection that equates with it in the source trait ref's bound. If there is more than one, bail with ambiguity.
https://github.com/rust-lang/rust/blob/fdcab310b2a57a4e9cc0b2629abd27afda49cd80/compiler/rustc_trait_selection/src/traits/select/mod.rs#L2526-L2557
    * Since there may be more than one that applies, we probe first to check that there is exactly one, then we equate it outside of a probe once we know that it's unique.

4. Make sure the lifetime of the source trait object outlives the lifetime of the target.

<details>
<summary>Meanwhile, this is how we used to do upcasting:</summary>

1. For each supertrait of the source trait object, take that supertrait, append the source object's projection bounds, and the *target* trait object's auto trait bounds, and make this into a new object type:
https://github.com/rust-lang/rust/blob/d12c6e947ceacf3b22c154caf9532b390d8dc88a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs#L915-L929

2. Then equate it with the target trait object:
https://github.com/rust-lang/rust/blob/d12c6e947ceacf3b22c154caf9532b390d8dc88a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs#L936

This will be a type mismatch if the target trait object has fewer projection bounds, since we compare the bounds structurally in relate:
https://github.com/rust-lang/rust/blob/d12c6e947ceacf3b22c154caf9532b390d8dc88a/compiler/rustc_middle/src/ty/relate.rs#L696-L698

</details>

Fixes #114035
Also fixes #114113, because I added a normalize call in the old solver.

r? types
  • Loading branch information
bors committed Aug 4, 2023
2 parents 34ccd04 + 7c942cc commit 4f7bb98
Show file tree
Hide file tree
Showing 15 changed files with 383 additions and 118 deletions.
28 changes: 28 additions & 0 deletions compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,3 +481,31 @@ impl<'tcx> ToTrace<'tcx> for ty::FnSig<'tcx> {
TypeTrace { cause: cause.clone(), values: Sigs(ExpectedFound::new(a_is_expected, a, b)) }
}
}

impl<'tcx> ToTrace<'tcx> for ty::PolyExistentialTraitRef<'tcx> {
fn to_trace(
cause: &ObligationCause<'tcx>,
a_is_expected: bool,
a: Self,
b: Self,
) -> TypeTrace<'tcx> {
TypeTrace {
cause: cause.clone(),
values: ExistentialTraitRef(ExpectedFound::new(a_is_expected, a, b)),
}
}
}

impl<'tcx> ToTrace<'tcx> for ty::PolyExistentialProjection<'tcx> {
fn to_trace(
cause: &ObligationCause<'tcx>,
a_is_expected: bool,
a: Self,
b: Self,
) -> TypeTrace<'tcx> {
TypeTrace {
cause: cause.clone(),
values: ExistentialProjection(ExpectedFound::new(a_is_expected, a, b)),
}
}
}
8 changes: 8 additions & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,12 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
(false, Mismatch::Fixed(self.tcx.def_descr(expected.def_id)))
}
ValuePairs::Regions(_) => (false, Mismatch::Fixed("lifetime")),
ValuePairs::ExistentialTraitRef(_) => {
(false, Mismatch::Fixed("existential trait ref"))
}
ValuePairs::ExistentialProjection(_) => {
(false, Mismatch::Fixed("existential projection"))
}
};
let Some(vals) = self.values_str(values) else {
// Derived error. Cancel the emitter.
Expand Down Expand Up @@ -2139,6 +2145,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
infer::Regions(exp_found) => self.expected_found_str(exp_found),
infer::Terms(exp_found) => self.expected_found_str_term(exp_found),
infer::Aliases(exp_found) => self.expected_found_str(exp_found),
infer::ExistentialTraitRef(exp_found) => self.expected_found_str(exp_found),
infer::ExistentialProjection(exp_found) => self.expected_found_str(exp_found),
infer::TraitRefs(exp_found) => {
let pretty_exp_found = ty::error::ExpectedFound {
expected: exp_found.expected.print_only_trait_path(),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ pub enum ValuePairs<'tcx> {
TraitRefs(ExpectedFound<ty::TraitRef<'tcx>>),
PolyTraitRefs(ExpectedFound<ty::PolyTraitRef<'tcx>>),
Sigs(ExpectedFound<ty::FnSig<'tcx>>),
ExistentialTraitRef(ExpectedFound<ty::PolyExistentialTraitRef<'tcx>>),
ExistentialProjection(ExpectedFound<ty::PolyExistentialProjection<'tcx>>),
}

impl<'tcx> ValuePairs<'tcx> {
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_middle/src/traits/solve/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,15 @@ pub struct GoalCandidate<'tcx> {
pub enum CandidateKind<'tcx> {
/// Probe entered when normalizing the self ty during candidate assembly
NormalizedSelfTyAssembly,
DynUpcastingAssembly,
/// A normal candidate for proving a goal
Candidate {
name: String,
result: QueryResult<'tcx>,
},
Candidate { name: String, result: QueryResult<'tcx> },
/// Used in the probe that wraps normalizing the non-self type for the unsize
/// trait, which is also structurally matched on.
UnsizeAssembly,
/// During upcasting from some source object to target object type, used to
/// do a probe to find out what projection type(s) may be used to prove that
/// the source type upholds all of the target type's object bounds.
UpcastProbe,
}
impl Debug for GoalCandidate<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_middle/src/traits/solve/inspect/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,11 @@ impl<'a, 'b> ProofTreeFormatter<'a, 'b> {
CandidateKind::NormalizedSelfTyAssembly => {
writeln!(self.f, "NORMALIZING SELF TY FOR ASSEMBLY:")
}
CandidateKind::DynUpcastingAssembly => {
writeln!(self.f, "ASSEMBLING CANDIDATES FOR DYN UPCASTING:")
CandidateKind::UnsizeAssembly => {
writeln!(self.f, "ASSEMBLING CANDIDATES FOR UNSIZING:")
}
CandidateKind::UpcastProbe => {
writeln!(self.f, "PROBING FOR PROJECTION COMPATIBILITY FOR UPCASTING:")
}
CandidateKind::Candidate { name, result } => {
writeln!(self.f, "CANDIDATE {name}: {result:?}")
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2734,8 +2734,9 @@ forward_display_to_print! {
// HACK(eddyb) these are exhaustive instead of generic,
// because `for<'tcx>` isn't possible yet.
ty::PolyExistentialPredicate<'tcx>,
ty::PolyExistentialProjection<'tcx>,
ty::PolyExistentialTraitRef<'tcx>,
ty::Binder<'tcx, ty::TraitRef<'tcx>>,
ty::Binder<'tcx, ty::ExistentialTraitRef<'tcx>>,
ty::Binder<'tcx, TraitRefPrintOnlyTraitPath<'tcx>>,
ty::Binder<'tcx, TraitRefPrintOnlyTraitName<'tcx>>,
ty::Binder<'tcx, ty::FnSig<'tcx>>,
Expand Down
130 changes: 93 additions & 37 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
Err(NoSolution) => vec![],
};

ecx.probe(|_| CandidateKind::DynUpcastingAssembly).enter(|ecx| {
ecx.probe(|_| CandidateKind::UnsizeAssembly).enter(|ecx| {
let a_ty = goal.predicate.self_ty();
// We need to normalize the b_ty since it's matched structurally
// in the other functions below.
Expand Down Expand Up @@ -526,7 +526,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
b_region: ty::Region<'tcx>,
) -> Vec<(CanonicalResponse<'tcx>, BuiltinImplSource)> {
let tcx = self.tcx();
let Goal { predicate: (a_ty, b_ty), .. } = goal;
let Goal { predicate: (a_ty, _b_ty), .. } = goal;

// All of a's auto traits need to be in b's auto traits.
let auto_traits_compatible =
Expand All @@ -535,51 +535,30 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
return vec![];
}

// Try to match `a_ty` against `b_ty`, replacing `a_ty`'s principal trait ref with
// the supertrait principal and subtyping the types.
let unsize_dyn_to_principal =
|ecx: &mut Self, principal: Option<ty::PolyExistentialTraitRef<'tcx>>| {
ecx.probe_candidate("upcast dyn to principle").enter(
|ecx| -> Result<_, NoSolution> {
// Require that all of the trait predicates from A match B, except for
// the auto traits. We do this by constructing a new A type with B's
// auto traits, and equating these types.
let new_a_data = principal
.into_iter()
.map(|trait_ref| trait_ref.map_bound(ty::ExistentialPredicate::Trait))
.chain(a_data.iter().filter(|a| {
matches!(a.skip_binder(), ty::ExistentialPredicate::Projection(_))
}))
.chain(
b_data
.auto_traits()
.map(ty::ExistentialPredicate::AutoTrait)
.map(ty::Binder::dummy),
);
let new_a_data = tcx.mk_poly_existential_predicates_from_iter(new_a_data);
let new_a_ty = Ty::new_dynamic(tcx, new_a_data, b_region, ty::Dyn);

// We also require that A's lifetime outlives B's lifetime.
ecx.eq(goal.param_env, new_a_ty, b_ty)?;
ecx.add_goal(goal.with(tcx, ty::OutlivesPredicate(a_region, b_region)));
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
},
)
};

let mut responses = vec![];
// If the principal def ids match (or are both none), then we're not doing
// trait upcasting. We're just removing auto traits (or shortening the lifetime).
if a_data.principal_def_id() == b_data.principal_def_id() {
if let Ok(resp) = unsize_dyn_to_principal(self, a_data.principal()) {
if let Ok(resp) = self.consider_builtin_upcast_to_principal(
goal,
a_data,
a_region,
b_data,
b_region,
a_data.principal(),
) {
responses.push((resp, BuiltinImplSource::Misc));
}
} else if let Some(a_principal) = a_data.principal() {
self.walk_vtable(
a_principal.with_self_ty(tcx, a_ty),
|ecx, new_a_principal, _, vtable_vptr_slot| {
if let Ok(resp) = unsize_dyn_to_principal(
ecx,
if let Ok(resp) = ecx.consider_builtin_upcast_to_principal(
goal,
a_data,
a_region,
b_data,
b_region,
Some(new_a_principal.map_bound(|trait_ref| {
ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)
})),
Expand Down Expand Up @@ -631,6 +610,83 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
}

fn consider_builtin_upcast_to_principal(
&mut self,
goal: Goal<'tcx, (Ty<'tcx>, Ty<'tcx>)>,
a_data: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
a_region: ty::Region<'tcx>,
b_data: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
b_region: ty::Region<'tcx>,
upcast_principal: Option<ty::PolyExistentialTraitRef<'tcx>>,
) -> QueryResult<'tcx> {
let param_env = goal.param_env;

// More than one projection in a_ty's bounds may match the projection
// in b_ty's bound. Use this to first determine *which* apply without
// having any inference side-effects. We process obligations because
// unification may initially succeed due to deferred projection equality.
let projection_may_match =
|ecx: &mut Self,
source_projection: ty::PolyExistentialProjection<'tcx>,
target_projection: ty::PolyExistentialProjection<'tcx>| {
source_projection.item_def_id() == target_projection.item_def_id()
&& ecx
.probe(|_| CandidateKind::UpcastProbe)
.enter(|ecx| -> Result<(), NoSolution> {
ecx.eq(param_env, source_projection, target_projection)?;
let _ = ecx.try_evaluate_added_goals()?;
Ok(())
})
.is_ok()
};

for bound in b_data {
match bound.skip_binder() {
// Check that a's supertrait (upcast_principal) is compatible
// with the target (b_ty).
ty::ExistentialPredicate::Trait(target_principal) => {
self.eq(param_env, upcast_principal.unwrap(), bound.rebind(target_principal))?;
}
// Check that b_ty's projection is satisfied by exactly one of
// a_ty's projections. First, we look through the list to see if
// any match. If not, error. Then, if *more* than one matches, we
// return ambiguity. Otherwise, if exactly one matches, equate
// it with b_ty's projection.
ty::ExistentialPredicate::Projection(target_projection) => {
let target_projection = bound.rebind(target_projection);
let mut matching_projections =
a_data.projection_bounds().filter(|source_projection| {
projection_may_match(self, *source_projection, target_projection)
});
let Some(source_projection) = matching_projections.next() else {
return Err(NoSolution);
};
if matching_projections.next().is_some() {
return self.evaluate_added_goals_and_make_canonical_response(
Certainty::AMBIGUOUS,
);
}
self.eq(param_env, source_projection, target_projection)?;
}
// Check that b_ty's auto traits are present in a_ty's bounds.
ty::ExistentialPredicate::AutoTrait(def_id) => {
if !a_data.auto_traits().any(|source_def_id| source_def_id == def_id) {
return Err(NoSolution);
}
}
}
}

// Also require that a_ty's lifetime outlives b_ty's lifetime.
self.add_goal(Goal::new(
self.tcx(),
param_env,
ty::Binder::dummy(ty::OutlivesPredicate(a_region, b_region)),
));

self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
}

/// We have the following builtin impls for arrays:
/// ```ignore (builtin impl example)
/// impl<T: ?Sized, const N: usize> Unsize<[T]> for [T; N] {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

match (source.kind(), target.kind()) {
// Trait+Kx+'a -> Trait+Ky+'b (upcasts).
(&ty::Dynamic(ref data_a, _, ty::Dyn), &ty::Dynamic(ref data_b, _, ty::Dyn)) => {
(
&ty::Dynamic(ref a_data, a_region, ty::Dyn),
&ty::Dynamic(ref b_data, b_region, ty::Dyn),
) => {
// Upcast coercions permit several things:
//
// 1. Dropping auto traits, e.g., `Foo + Send` to `Foo`
Expand All @@ -757,19 +760,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
//
// We always perform upcasting coercions when we can because of reason
// #2 (region bounds).
let auto_traits_compatible = data_b
let auto_traits_compatible = b_data
.auto_traits()
// All of a's auto traits need to be in b's auto traits.
.all(|b| data_a.auto_traits().any(|a| a == b));
.all(|b| a_data.auto_traits().any(|a| a == b));
if auto_traits_compatible {
let principal_def_id_a = data_a.principal_def_id();
let principal_def_id_b = data_b.principal_def_id();
let principal_def_id_a = a_data.principal_def_id();
let principal_def_id_b = b_data.principal_def_id();
if principal_def_id_a == principal_def_id_b {
// no cyclic
candidates.vec.push(BuiltinUnsizeCandidate);
} else if principal_def_id_a.is_some() && principal_def_id_b.is_some() {
// not casual unsizing, now check whether this is trait upcasting coercion.
let principal_a = data_a.principal().unwrap();
let principal_a = a_data.principal().unwrap();
let target_trait_did = principal_def_id_b.unwrap();
let source_trait_ref = principal_a.with_self_ty(self.tcx(), source);
if let Some(deref_trait_ref) = self.need_migrate_deref_output_trait_object(
Expand All @@ -785,9 +788,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
for (idx, upcast_trait_ref) in
util::supertraits(self.tcx(), source_trait_ref).enumerate()
{
if upcast_trait_ref.def_id() == target_trait_did {
candidates.vec.push(TraitUpcastingUnsizeCandidate(idx));
}
self.infcx.probe(|_| {
if upcast_trait_ref.def_id() == target_trait_did
&& let Ok(nested) = self.match_upcast_principal(
obligation,
upcast_trait_ref,
a_data,
b_data,
a_region,
b_region,
)
{
if nested.is_none() {
candidates.ambiguous = true;
}
candidates.vec.push(TraitUpcastingUnsizeCandidate(idx));
}
})
}
}
}
Expand Down
Loading

0 comments on commit 4f7bb98

Please sign in to comment.