Skip to content

Commit

Permalink
bye bye assemble_candidates_via_self_ty
Browse files Browse the repository at this point in the history
  • Loading branch information
lcnr committed Jan 29, 2024
1 parent 7e43442 commit bc7b899
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 133 deletions.
134 changes: 28 additions & 106 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,64 +271,32 @@ impl<'tcx> EvalCtxt<'_, '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 +306,15 @@ 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_unsize_to_dyn_candidate(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_blanket_impl_candidates(goal, &mut candidates);

self.assemble_param_env_candidates(goal, &mut candidates);

self.assemble_coherence_unknowable_candidates(goal, &mut candidates);

candidates
}

#[instrument(level = "debug", skip_all)]
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
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() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0283]: type annotations needed: cannot satisfy `Foo: Send`
--> $DIR/dont-type_of-tait-in-defining-scope.rs:15:18
|
LL | needs_send::<Foo>();
| ^^^
|
= note: cannot satisfy `Foo: Send`
note: required by a bound in `needs_send`
--> $DIR/dont-type_of-tait-in-defining-scope.rs:12:18
|
LL | fn needs_send<T: Send>() {}
| ^^^^ required by this bound in `needs_send`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0283`.
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error[E0283]: type annotations needed: cannot satisfy `Foo: Send`
--> $DIR/dont-type_of-tait-in-defining-scope.rs:16:18
--> $DIR/dont-type_of-tait-in-defining-scope.rs:15:18
|
LL | needs_send::<Foo>();
| ^^^
|
= note: cannot satisfy `Foo: Send`
note: required by a bound in `needs_send`
--> $DIR/dont-type_of-tait-in-defining-scope.rs:13:18
--> $DIR/dont-type_of-tait-in-defining-scope.rs:12:18
|
LL | fn needs_send<T: Send>() {}
| ^^^^ required by this bound in `needs_send`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// revisions: is_send not_send
// compile-flags: -Znext-solver
//[is_send] check-pass

#![feature(type_alias_impl_trait)]

Expand All @@ -14,7 +13,7 @@ fn needs_send<T: Send>() {}

fn test(_: Foo) {
needs_send::<Foo>();
//[not_send]~^ ERROR type annotations needed: cannot satisfy `Foo: Send`
//~^ ERROR type annotations needed: cannot satisfy `Foo: Send`
}

fn defines(_: Foo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ fn test<T: Foo1<Assoc1 = <T as Foo2>::Assoc2> + Foo2<Assoc2 = <T as Foo1>::Assoc
//~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
//~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
//~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
//~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1: Sized`
//~| ERROR overflow evaluating the requirement `<T as Foo1>::Assoc1: Bar`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@ note: required by a bound in `needs_bar`
LL | fn needs_bar<S: Bar>() {}
| ^^^ required by this bound in `needs_bar`

error[E0275]: overflow evaluating the requirement `<T as Foo1>::Assoc1: Sized`
--> $DIR/recursive-self-normalization-2.rs:15:17
|
LL | needs_bar::<T::Assoc1>();
| ^^^^^^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization_2`)
note: required by a bound in `needs_bar`
--> $DIR/recursive-self-normalization-2.rs:12:14
|
LL | fn needs_bar<S: Bar>() {}
| ^ required by this bound in `needs_bar`
help: consider relaxing the implicit `Sized` restriction
|
LL | fn needs_bar<S: Bar + ?Sized>() {}
| ++++++++

error[E0275]: overflow evaluating the requirement `<T as Foo1>::Assoc1 == _`
--> $DIR/recursive-self-normalization-2.rs:15:5
|
Expand All @@ -45,6 +62,6 @@ LL | needs_bar::<T::Assoc1>();
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization_2`)
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 5 previous errors
error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0275`.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ fn test<T: Foo<Assoc = <T as Foo>::Assoc>>() {
//~| ERROR overflow evaluating the requirement `<T as Foo>::Assoc == _`
//~| ERROR overflow evaluating the requirement `<T as Foo>::Assoc == _`
//~| ERROR overflow evaluating the requirement `<T as Foo>::Assoc == _`
//~| ERROR overflow evaluating the requirement `<T as Foo>::Assoc: Sized`
//~| ERROR overflow evaluating the requirement `<T as Foo>::Assoc: Bar`
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@ note: required by a bound in `needs_bar`
LL | fn needs_bar<S: Bar>() {}
| ^^^ required by this bound in `needs_bar`

error[E0275]: overflow evaluating the requirement `<T as Foo>::Assoc: Sized`
--> $DIR/recursive-self-normalization.rs:11:17
|
LL | needs_bar::<T::Assoc>();
| ^^^^^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization`)
note: required by a bound in `needs_bar`
--> $DIR/recursive-self-normalization.rs:8:14
|
LL | fn needs_bar<S: Bar>() {}
| ^ required by this bound in `needs_bar`
help: consider relaxing the implicit `Sized` restriction
|
LL | fn needs_bar<S: Bar + ?Sized>() {}
| ++++++++

error[E0275]: overflow evaluating the requirement `<T as Foo>::Assoc == _`
--> $DIR/recursive-self-normalization.rs:11:5
|
Expand All @@ -45,6 +62,6 @@ LL | needs_bar::<T::Assoc>();
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`recursive_self_normalization`)
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 5 previous errors
error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0275`.
22 changes: 13 additions & 9 deletions tests/ui/type-alias-impl-trait/issue-76202-trait-impl-for-tait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,14 @@
// revisions: current next
//[next] compile-flags: -Znext-solver
// check-pass

#![feature(type_alias_impl_trait)]

trait Dummy {}
impl Dummy for () {}

type F = impl Dummy;
fn f() -> F {}

trait Test {
fn test(self);
}

impl Test for F {

impl Test for define::F {
fn test(self) {}
}

Expand All @@ -27,7 +21,17 @@ impl Test for i32 {
fn test(self) {}
}

mod define {
use super::*;

pub trait Dummy {}
impl Dummy for () {}

pub type F = impl Dummy;
pub fn f() -> F {}
}

fn main() {
let x: F = f();
let x = define::f();
x.test();
}

0 comments on commit bc7b899

Please sign in to comment.