Skip to content

Commit

Permalink
Auto merge of rust-lang#119744 - lcnr:assemble-only-rigid, r=compiler…
Browse files Browse the repository at this point in the history
…-errors

only assemble alias bound candidates for rigid aliases

fixes rust-lang/trait-system-refactor-initiative#77

This also causes `<Wrapper<?0> as Trait>::Unwrap: Trait` to always be ambig, as we now normalize the self type before checking whether it is an inference variable.

I cannot think of an approach to the underlying issues here which does not require the "may-define means must-define" restriction for opaque types. Going to go ahead with this and added this restriction to the tracking issue for the new solver to make sure we don't stabilize it without getting types + lang signoff here.

r? `@compiler-errors`
  • Loading branch information
bors committed Jan 30, 2024
2 parents 5c9c3c7 + ea4e5b8 commit c401f09
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 218 deletions.
161 changes: 26 additions & 135 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,82 +253,39 @@ pub(super) trait GoalKind<'tcx>:
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> Vec<(CanonicalResponse<'tcx>, BuiltinImplSource)>;

/// Consider the `Unsize` candidate corresponding to coercing a sized type
/// into a `dyn Trait`.
///
/// This is computed separately from the rest of the `Unsize` candidates
/// since it is only done once per self type, and not once per
/// *normalization step* (in `assemble_candidates_via_self_ty`).
fn consider_unsize_to_dyn_candidate(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx>;
}

impl<'tcx> EvalCtxt<'_, 'tcx> {
pub(super) fn assemble_and_evaluate_candidates<G: GoalKind<'tcx>>(
&mut self,
goal: Goal<'tcx, G>,
) -> Vec<Candidate<'tcx>> {
debug_assert_eq!(goal, self.resolve_vars_if_possible(goal));
if let Some(ambig) = self.assemble_self_ty_infer_ambiguity_response(goal) {
return vec![ambig];
}

let mut candidates = self.assemble_candidates_via_self_ty(goal, 0);

self.assemble_unsize_to_dyn_candidate(goal, &mut candidates);

self.assemble_blanket_impl_candidates(goal, &mut candidates);

self.assemble_param_env_candidates(goal, &mut candidates);

self.assemble_coherence_unknowable_candidates(goal, &mut candidates);

candidates
}

/// `?0: Trait` is ambiguous, because it may be satisfied via a builtin rule,
/// object bound, alias bound, etc. We are unable to determine this until we can at
/// least structurally resolve the type one layer.
///
/// It would also require us to consider all impls of the trait, which is both pretty
/// bad for perf and would also constrain the self type if there is just a single impl.
fn assemble_self_ty_infer_ambiguity_response<G: GoalKind<'tcx>>(
&mut self,
goal: Goal<'tcx, G>,
) -> Option<Candidate<'tcx>> {
if goal.predicate.self_ty().is_ty_var() {
debug!("adding self_ty_infer_ambiguity_response");
let dummy_candidate = |this: &mut EvalCtxt<'_, 'tcx>, certainty| {
let source = CandidateSource::BuiltinImpl(BuiltinImplSource::Misc);
let result = self
.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
.unwrap();
let mut dummy_probe = self.inspect.new_probe();
let result = this.evaluate_added_goals_and_make_canonical_response(certainty).unwrap();
let mut dummy_probe = this.inspect.new_probe();
dummy_probe.probe_kind(ProbeKind::TraitCandidate { source, result: Ok(result) });
self.inspect.finish_probe(dummy_probe);
Some(Candidate { source, result })
} else {
None
this.inspect.finish_probe(dummy_probe);
vec![Candidate { source, result }]
};

let Some(normalized_self_ty) =
self.try_normalize_ty(goal.param_env, goal.predicate.self_ty())
else {
debug!("overflow while evaluating self type");
return dummy_candidate(self, Certainty::OVERFLOW);
};

if normalized_self_ty.is_ty_var() {
debug!("self type has been normalized to infer");
return dummy_candidate(self, Certainty::AMBIGUOUS);
}
}

/// Assemble candidates which apply to the self type. This only looks at candidate which
/// apply to the specific self type and ignores all others.
///
/// Returns `None` if the self type is still ambiguous.
fn assemble_candidates_via_self_ty<G: GoalKind<'tcx>>(
&mut self,
goal: Goal<'tcx, G>,
num_steps: usize,
) -> Vec<Candidate<'tcx>> {
let goal =
goal.with(self.tcx(), goal.predicate.with_self_ty(self.tcx(), normalized_self_ty));
debug_assert_eq!(goal, self.resolve_vars_if_possible(goal));
if let Some(ambig) = self.assemble_self_ty_infer_ambiguity_response(goal) {
return vec![ambig];
}

let mut candidates = Vec::new();
let mut candidates = vec![];

self.assemble_non_blanket_impl_candidates(goal, &mut candidates);

Expand All @@ -338,61 +295,13 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

self.assemble_object_bound_candidates(goal, &mut candidates);

self.assemble_candidates_after_normalizing_self_ty(goal, &mut candidates, num_steps);
candidates
}
self.assemble_blanket_impl_candidates(goal, &mut candidates);

/// If the self type of a goal is an alias we first try to normalize the self type
/// and compute the candidates for the normalized self type in case that succeeds.
///
/// These candidates are used in addition to the ones with the alias as a self type.
/// We do this to simplify both builtin candidates and for better performance.
///
/// We generate the builtin candidates on the fly by looking at the self type, e.g.
/// add `FnPtr` candidates if the self type is a function pointer. Handling builtin
/// candidates while the self type is still an alias seems difficult. This is similar
/// to `try_structurally_resolve_type` during hir typeck (FIXME once implemented).
///
/// Looking at all impls for some trait goal is prohibitively expensive. We therefore
/// only look at implementations with a matching self type. Because of this function,
/// we can avoid looking at all existing impls if the self type is an alias.
#[instrument(level = "debug", skip_all)]
fn assemble_candidates_after_normalizing_self_ty<G: GoalKind<'tcx>>(
&mut self,
goal: Goal<'tcx, G>,
candidates: &mut Vec<Candidate<'tcx>>,
num_steps: usize,
) {
let tcx = self.tcx();
let &ty::Alias(_, alias) = goal.predicate.self_ty().kind() else { return };

candidates.extend(self.probe(|_| ProbeKind::NormalizedSelfTyAssembly).enter(|ecx| {
if tcx.recursion_limit().value_within_limit(num_steps) {
let normalized_ty = ecx.next_ty_infer();
let normalizes_to_goal =
goal.with(tcx, ty::NormalizesTo { alias, term: normalized_ty.into() });
ecx.add_goal(GoalSource::Misc, normalizes_to_goal);
if let Err(NoSolution) = ecx.try_evaluate_added_goals() {
debug!("self type normalization failed");
return vec![];
}
let normalized_ty = ecx.resolve_vars_if_possible(normalized_ty);
debug!(?normalized_ty, "self type normalized");
// NOTE: Alternatively we could call `evaluate_goal` here and only
// have a `Normalized` candidate. This doesn't work as long as we
// use `CandidateSource` in winnowing.
let goal = goal.with(tcx, goal.predicate.with_self_ty(tcx, normalized_ty));
ecx.assemble_candidates_via_self_ty(goal, num_steps + 1)
} else {
match ecx.evaluate_added_goals_and_make_canonical_response(Certainty::OVERFLOW) {
Ok(result) => vec![Candidate {
source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
result,
}],
Err(NoSolution) => vec![],
}
}
}));
self.assemble_param_env_candidates(goal, &mut candidates);

self.assemble_coherence_unknowable_candidates(goal, &mut candidates);

candidates
}

#[instrument(level = "debug", skip_all)]
Expand Down Expand Up @@ -500,24 +409,6 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
}
}

#[instrument(level = "debug", skip_all)]
fn assemble_unsize_to_dyn_candidate<G: GoalKind<'tcx>>(
&mut self,
goal: Goal<'tcx, G>,
candidates: &mut Vec<Candidate<'tcx>>,
) {
let tcx = self.tcx();
if tcx.lang_items().unsize_trait() == Some(goal.predicate.trait_def_id(tcx)) {
match G::consider_unsize_to_dyn_candidate(self, goal) {
Ok(result) => candidates.push(Candidate {
source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
result,
}),
Err(NoSolution) => (),
}
}
}

#[instrument(level = "debug", skip_all)]
fn assemble_blanket_impl_candidates<G: GoalKind<'tcx>>(
&mut self,
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_trait_selection/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

/// Normalize a type when it is structually matched on.
///
/// For self types this is generally already handled through
/// `assemble_candidates_after_normalizing_self_ty`, so anything happening
/// in [`EvalCtxt::assemble_candidates_via_self_ty`] does not have to normalize
/// the self type. It is required when structurally matching on any other
/// arguments of a trait goal, e.g. when assembling builtin unsize candidates.
/// In nearly all cases this function must be used before matching on a type.
/// Not doing so is likely to be incomplete and therefore unsound during
/// coherence.
#[instrument(level = "debug", skip(self), ret)]
fn try_normalize_ty(
&mut self,
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,13 +603,6 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> {
)
}

fn consider_unsize_to_dyn_candidate(
_ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx> {
bug!("`Unsize` does not have an associated type: {:?}", goal)
}

fn consider_structural_builtin_unsize_candidates(
_ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
Expand Down
90 changes: 41 additions & 49 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,53 +490,6 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
ecx.evaluate_added_goals_and_make_canonical_response(certainty)
}

fn consider_unsize_to_dyn_candidate(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx> {
ecx.probe(|_| ProbeKind::UnsizeAssembly).enter(|ecx| {
let a_ty = goal.predicate.self_ty();
// We need to normalize the b_ty since it's destructured as a `dyn Trait`.
let Some(b_ty) =
ecx.try_normalize_ty(goal.param_env, goal.predicate.trait_ref.args.type_at(1))
else {
return ecx.evaluate_added_goals_and_make_canonical_response(Certainty::OVERFLOW);
};

let ty::Dynamic(b_data, b_region, ty::Dyn) = *b_ty.kind() else {
return Err(NoSolution);
};

let tcx = ecx.tcx();

// Can only unsize to an object-safe trait.
if b_data.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
return Err(NoSolution);
}

// Check that the type implements all of the predicates of the trait object.
// (i.e. the principal, all of the associated types match, and any auto traits)
ecx.add_goals(
GoalSource::ImplWhereBound,
b_data.iter().map(|pred| goal.with(tcx, pred.with_self_ty(tcx, a_ty))),
);

// The type must be `Sized` to be unsized.
if let Some(sized_def_id) = tcx.lang_items().sized_trait() {
ecx.add_goal(
GoalSource::ImplWhereBound,
goal.with(tcx, ty::TraitRef::new(tcx, sized_def_id, [a_ty])),
);
} else {
return Err(NoSolution);
}

// The type must outlive the lifetime of the `dyn` we're unsizing into.
ecx.add_goal(GoalSource::Misc, goal.with(tcx, ty::OutlivesPredicate(a_ty, b_region)));
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
})
}

/// ```ignore (builtin impl example)
/// trait Trait {
/// fn foo(&self);
Expand Down Expand Up @@ -588,8 +541,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
goal, a_data, a_region, b_data, b_region,
),

// `T` -> `dyn Trait` unsizing is handled separately in `consider_unsize_to_dyn_candidate`
(_, &ty::Dynamic(..)) => vec![],
// `T` -> `dyn Trait` unsizing.
(_, &ty::Dynamic(b_region, b_data, ty::Dyn)) => result_to_single(
ecx.consider_builtin_unsize_to_dyn_candidate(goal, b_region, b_data),
BuiltinImplSource::Misc,
),

// `[T; N]` -> `[T]` unsizing
(&ty::Array(a_elem_ty, ..), &ty::Slice(b_elem_ty)) => result_to_single(
Expand Down Expand Up @@ -691,6 +647,42 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
responses
}

fn consider_builtin_unsize_to_dyn_candidate(
&mut self,
goal: Goal<'tcx, (Ty<'tcx>, Ty<'tcx>)>,
b_data: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
b_region: ty::Region<'tcx>,
) -> QueryResult<'tcx> {
let tcx = self.tcx();
let Goal { predicate: (a_ty, _), .. } = goal;

// Can only unsize to an object-safe trait.
if b_data.principal_def_id().is_some_and(|def_id| !tcx.check_is_object_safe(def_id)) {
return Err(NoSolution);
}

// Check that the type implements all of the predicates of the trait object.
// (i.e. the principal, all of the associated types match, and any auto traits)
self.add_goals(
GoalSource::ImplWhereBound,
b_data.iter().map(|pred| goal.with(tcx, pred.with_self_ty(tcx, a_ty))),
);

// The type must be `Sized` to be unsized.
if let Some(sized_def_id) = tcx.lang_items().sized_trait() {
self.add_goal(
GoalSource::ImplWhereBound,
goal.with(tcx, ty::TraitRef::new(tcx, sized_def_id, [a_ty])),
);
} else {
return Err(NoSolution);
}

// The type must outlive the lifetime of the `dyn` we're unsizing into.
self.add_goal(GoalSource::Misc, goal.with(tcx, ty::OutlivesPredicate(a_ty, b_region)));
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>)>,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/next-solver/alias-bound-unsound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ trait Foo {

impl Foo for () {
type Item = String where String: Copy;
//~^ ERROR overflow evaluating the requirement `<() as Foo>::Item: Copy`
//~^ ERROR overflow evaluating the requirement `String: Copy`
}

fn main() {
Expand Down
14 changes: 8 additions & 6 deletions tests/ui/traits/next-solver/alias-bound-unsound.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
error[E0275]: overflow evaluating the requirement `<() as Foo>::Item: Copy`
--> $DIR/alias-bound-unsound.rs:18:17
error[E0275]: overflow evaluating the requirement `String: Copy`
--> $DIR/alias-bound-unsound.rs:18:38
|
LL | type Item = String where String: Copy;
| ^^^^^^
| ^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`alias_bound_unsound`)
note: required by a bound in `Foo::Item`
--> $DIR/alias-bound-unsound.rs:8:16
note: the requirement `String: Copy` appears on the `impl`'s associated type `Item` but not on the corresponding trait's associated type
--> $DIR/alias-bound-unsound.rs:8:10
|
LL | trait Foo {
| --- in this trait
LL | type Item: Copy
| ^^^^ required by this bound in `Foo::Item`
| ^^^^ this trait's associated type doesn't have the requirement `String: Copy`

error[E0275]: overflow evaluating the requirement `String <: <() as Foo>::Item`
--> $DIR/alias-bound-unsound.rs:24:31
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// check-pass
// compile-flags: -Znext-solver

trait Reader: Default {
fn read_u8_array<A>(&self) -> Result<A, ()> {
todo!()
}

fn read_u8(&self) -> Result<u8, ()> {
let a: [u8; 1] = self.read_u8_array::<_>()?;
// This results in a nested `<Result<?0, ()> as Try>::Residual: Sized` goal.
// The self type normalizes to `?0`. We previously did not force that to be
// ambiguous but instead incompletely applied the `Self: Sized` candidate
// from the `ParamEnv`, resulting in a type error.
Ok(a[0])
}
}

fn main() {}
Loading

0 comments on commit c401f09

Please sign in to comment.