Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

instantiate higher ranked goals outside of candidate selection #119820

Merged
merged 2 commits into from
Apr 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 62 additions & 14 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,20 @@ use rustc_middle::ty::print::with_no_trimmed_paths;
mod candidate_assembly;
mod confirmation;

/// Whether to consider the binder of higher ranked goals for the `leak_check` when
/// evaluating higher-ranked goals. See #119820 for more info.
///
/// While this is a bit hacky, it is necessary to match the behavior of the new solver:
/// We eagerly instantiate binders in the new solver, outside of candidate selection, so
/// the leak check inside of candidates does not consider any bound vars from the higher
/// ranked goal. However, we do exit the binder once we're completely finished with a goal,
/// so the leak-check can be used in evaluate by causing nested higher-ranked goals to fail.
#[derive(Debug, Copy, Clone)]
enum LeakCheckHigherRankedGoal {
No,
Yes,
}

#[derive(Clone, Debug, Eq, PartialEq, Hash)]
pub enum IntercrateAmbiguityCause<'tcx> {
DownstreamCrate { trait_ref: ty::TraitRef<'tcx>, self_ty: Option<Ty<'tcx>> },
Expand Down Expand Up @@ -384,7 +398,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let mut no_candidates_apply = true;

for c in candidate_set.vec.iter() {
if self.evaluate_candidate(stack, c)?.may_apply() {
if self
.evaluate_candidate(stack, c, LeakCheckHigherRankedGoal::No)?
.may_apply()
{
no_candidates_apply = false;
break;
}
Expand Down Expand Up @@ -455,7 +472,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// is needed for specialization. Propagate overflow if it occurs.
let mut candidates = candidates
.into_iter()
.map(|c| match self.evaluate_candidate(stack, &c) {
.map(|c| match self.evaluate_candidate(stack, &c, LeakCheckHigherRankedGoal::No) {
Ok(eval) if eval.may_apply() => {
Ok(Some(EvaluatedCandidate { candidate: c, evaluation: eval }))
}
Expand Down Expand Up @@ -545,7 +562,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation: &PredicateObligation<'tcx>,
) -> Result<EvaluationResult, OverflowError> {
debug_assert!(!self.infcx.next_trait_solver());
self.evaluation_probe(|this| {
self.evaluation_probe(|this, _outer_universe| {
let goal =
this.infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env));
let mut result = this.evaluate_predicate_recursively(
Expand All @@ -561,13 +578,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
})
}

/// Computes the evaluation result of `op`, discarding any constraints.
///
/// This also runs for leak check to allow higher ranked region errors to impact
/// selection. By default it checks for leaks from all universes created inside of
/// `op`, but this can be overwritten if necessary.
fn evaluation_probe(
&mut self,
op: impl FnOnce(&mut Self) -> Result<EvaluationResult, OverflowError>,
op: impl FnOnce(&mut Self, &mut ty::UniverseIndex) -> Result<EvaluationResult, OverflowError>,
) -> Result<EvaluationResult, OverflowError> {
self.infcx.probe(|snapshot| -> Result<EvaluationResult, OverflowError> {
let outer_universe = self.infcx.universe();
let result = op(self)?;
let mut outer_universe = self.infcx.universe();
let result = op(self, &mut outer_universe)?;

match self.infcx.leak_check(outer_universe, Some(snapshot)) {
Ok(()) => {}
Expand All @@ -586,9 +608,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
})
}

/// Evaluates the predicates in `predicates` recursively. Note that
/// this applies projections in the predicates, and therefore
/// Evaluates the predicates in `predicates` recursively. This may
/// guide inference. If this is not desired, run it inside of a
/// is run within an inference probe.
/// `probe`.
#[instrument(skip(self, stack), level = "debug")]
fn evaluate_predicates_recursively<'o, I>(
&mut self,
Expand Down Expand Up @@ -1194,7 +1217,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}

match self.candidate_from_obligation(stack) {
Ok(Some(c)) => self.evaluate_candidate(stack, &c),
Ok(Some(c)) => self.evaluate_candidate(stack, &c, LeakCheckHigherRankedGoal::Yes),
Ok(None) => Ok(EvaluatedToAmbig),
Err(Overflow(OverflowError::Canonical)) => Err(OverflowError::Canonical),
Err(..) => Ok(EvaluatedToErr),
Expand All @@ -1219,6 +1242,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
/// Further evaluates `candidate` to decide whether all type parameters match and whether nested
/// obligations are met. Returns whether `candidate` remains viable after this further
/// scrutiny.
///
/// Depending on the value of [LeakCheckHigherRankedGoal], we may ignore the binder of the goal
/// when eagerly detecting higher ranked region errors via the `leak_check`. See that enum for
/// more info.
#[instrument(
level = "debug",
skip(self, stack),
Expand All @@ -1229,10 +1256,25 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&mut self,
stack: &TraitObligationStack<'o, 'tcx>,
candidate: &SelectionCandidate<'tcx>,
leak_check_higher_ranked_goal: LeakCheckHigherRankedGoal,
) -> Result<EvaluationResult, OverflowError> {
let mut result = self.evaluation_probe(|this| {
let candidate = (*candidate).clone();
match this.confirm_candidate(stack.obligation, candidate) {
let mut result = self.evaluation_probe(|this, outer_universe| {
// We eagerly instantiate higher ranked goals to prevent universe errors
// from impacting candidate selection. This matches the behavior of the new
// solver. This slightly weakens type inference.
//
// In case there are no unresolved type or const variables this
// should still not be necessary to select a unique impl as any overlap
// relying on a universe error from higher ranked goals should have resulted
// in an overlap error in coherence.
let p = self.infcx.enter_forall_and_leak_universe(stack.obligation.predicate);
let obligation = stack.obligation.with(this.tcx(), ty::Binder::dummy(p));
match leak_check_higher_ranked_goal {
LeakCheckHigherRankedGoal::No => *outer_universe = self.infcx.universe(),
LeakCheckHigherRankedGoal::Yes => {}
}

match this.confirm_candidate(&obligation, candidate.clone()) {
Ok(selection) => {
debug!(?selection);
this.evaluate_predicates_recursively(
Expand Down Expand Up @@ -1657,8 +1699,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
stack: &TraitObligationStack<'o, 'tcx>,
where_clause_trait_ref: ty::PolyTraitRef<'tcx>,
) -> Result<EvaluationResult, OverflowError> {
self.evaluation_probe(|this| {
match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) {
self.evaluation_probe(|this, outer_universe| {
// Eagerly instantiate higher ranked goals.
//
// See the comment in `evaluate_candidate` to see why.
let p = self.infcx.enter_forall_and_leak_universe(stack.obligation.predicate);
let obligation = stack.obligation.with(this.tcx(), ty::Binder::dummy(p));
*outer_universe = self.infcx.universe();
match this.match_where_clause_trait_ref(&obligation, where_clause_trait_ref) {
Ok(obligations) => this.evaluate_predicates_recursively(stack.list(), obligations),
Err(()) => Ok(EvaluatedToErr),
}
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/issues.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,6 @@
"ui/generics/issue-98432.rs",
"ui/higher-ranked/trait-bounds/issue-100689.rs",
"ui/higher-ranked/trait-bounds/issue-102899.rs",
"ui/higher-ranked/trait-bounds/issue-30786.rs",
"ui/higher-ranked/trait-bounds/issue-36139-normalize-closure-sig.rs",
"ui/higher-ranked/trait-bounds/issue-39292.rs",
"ui/higher-ranked/trait-bounds/issue-42114.rs",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// cc #119820

trait Trait {}

impl<T: Trait> Trait for &T {}
impl Trait for u32 {}

fn hr_bound<T>()
where
for<'a> &'a T: Trait,
{
}

fn foo<T>()
where
T: Trait,
for<'a> &'a &'a T: Trait,
{
// We get a universe error when using the `param_env` candidate
// but are able to successfully use the impl candidate. Without
// the leak check both candidates may apply and we prefer the
// `param_env` candidate in winnowing.
hr_bound::<&T>();
//~^ ERROR the parameter type `T` may not live long enough
//~| ERROR implementation of `Trait` is not general enough
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0310]: the parameter type `T` may not live long enough
--> $DIR/candidate-from-env-universe-err-1.rs:23:5
|
LL | hr_bound::<&T>();
| ^^^^^^^^^^^^^^
| |
| the parameter type `T` must be valid for the static lifetime...
| ...so that the type `T` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | T: Trait + 'static,
| +++++++++

error: implementation of `Trait` is not general enough
--> $DIR/candidate-from-env-universe-err-1.rs:23:5
|
LL | hr_bound::<&T>();
| ^^^^^^^^^^^^^^ implementation of `Trait` is not general enough
|
= note: `Trait` would have to be implemented for the type `&'0 &T`, for any lifetime `'0`...
= note: ...but `Trait` is actually implemented for the type `&'1 &'1 T`, for some specific lifetime `'1`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0310`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error: lifetime may not live long enough
--> $DIR/candidate-from-env-universe-err-2.rs:14:5
|
LL | fn not_hr<'a, T: for<'b> Trait<'a, 'b> + OtherTrait<'static>>() {
| -- lifetime `'a` defined here
LL | impl_hr::<T>();
| ^^^^^^^^^^^^ requires that `'a` must outlive `'static`
|
note: due to current limitations in the borrow checker, this implies a `'static` lifetime
--> $DIR/candidate-from-env-universe-err-2.rs:11:19
|
LL | fn impl_hr<'b, T: for<'a> Trait<'a, 'b>>() {}
| ^^^^^^^^^^^^^^^^^^^^^

error: implementation of `Trait` is not general enough
--> $DIR/candidate-from-env-universe-err-2.rs:14:5
|
LL | impl_hr::<T>();
| ^^^^^^^^^^^^ implementation of `Trait` is not general enough
|
= note: `T` must implement `Trait<'0, '_>`, for any lifetime `'0`...
= note: ...but it actually implements `Trait<'1, '_>`, for some specific lifetime `'1`

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error[E0277]: the trait bound `for<'a> T: Trait<'a, '_>` is not satisfied
--> $DIR/candidate-from-env-universe-err-2.rs:14:5
|
LL | impl_hr::<T>();
| ^^^^^^^^^^^^^^ the trait `for<'a> Trait<'a, '_>` is not implemented for `T`
|
note: required by a bound in `impl_hr`
--> $DIR/candidate-from-env-universe-err-2.rs:11:19
|
LL | fn impl_hr<'b, T: for<'a> Trait<'a, 'b>>() {}
| ^^^^^^^^^^^^^^^^^^^^^ required by this bound in `impl_hr`
help: consider further restricting this bound
|
LL | fn not_hr<'a, T: for<'b> Trait<'a, 'b> + OtherTrait<'static> + for<'a> Trait<'a, '_>>() {
| +++++++++++++++++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: lifetime may not live long enough
--> $DIR/candidate-from-env-universe-err-2.rs:14:5
|
LL | fn not_hr<'a, T: for<'b> Trait<'a, 'b> + OtherTrait<'static>>() {
| -- lifetime `'a` defined here
LL | impl_hr::<T>();
| ^^^^^^^^^^^^ requires that `'a` must outlive `'static`
|
note: due to current limitations in the borrow checker, this implies a `'static` lifetime
--> $DIR/candidate-from-env-universe-err-2.rs:11:19
|
LL | fn impl_hr<'b, T: for<'a> Trait<'a, 'b>>() {}
| ^^^^^^^^^^^^^^^^^^^^^

error[E0308]: mismatched types
--> $DIR/candidate-from-env-universe-err-2.rs:14:5
|
LL | impl_hr::<T>();
| ^^^^^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a> Trait<'a, '_>`
found trait `for<'b> Trait<'_, 'b>`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//@ revisions: current next
//@[next] compile-flags: -Znext-solver

// cc #119820

trait Trait<'a, 'b> {}

trait OtherTrait<'b> {}
impl<'a, 'b, T: OtherTrait<'b>> Trait<'a, 'b> for T {}

fn impl_hr<'b, T: for<'a> Trait<'a, 'b>>() {}

fn not_hr<'a, T: for<'b> Trait<'a, 'b> + OtherTrait<'static>>() {
impl_hr::<T>();
//[next]~^ ERROR the trait bound `for<'a> T: Trait<'a, '_>` is not satisfied
//[current]~^^ERROR lifetime may not live long enough
//[current]~| ERROR implementation of `Trait` is not general enough
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error: implementation of `Trait` is not general enough
--> $DIR/candidate-from-env-universe-err-project.rs:28:5
|
LL | trait_bound::<T>();
| ^^^^^^^^^^^^^^^^^^ implementation of `Trait` is not general enough
|
= note: `T` must implement `Trait<'0>`, for any lifetime `'0`...
= note: ...but it actually implements `Trait<'static>`

error: implementation of `Trait` is not general enough
--> $DIR/candidate-from-env-universe-err-project.rs:39:5
|
LL | projection_bound::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Trait` is not general enough
|
= note: `T` must implement `Trait<'0>`, for any lifetime `'0`...
= note: ...but it actually implements `Trait<'static>`

error[E0308]: mismatched types
--> $DIR/candidate-from-env-universe-err-project.rs:39:5
|
LL | projection_bound::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected associated type `<T as Trait<'static>>::Assoc`
found associated type `<T as Trait<'a>>::Assoc`
note: the lifetime requirement is introduced here
--> $DIR/candidate-from-env-universe-err-project.rs:18:42
|
LL | fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
| ^^^^^^^^^^^^^

error[E0308]: mismatched types
--> $DIR/candidate-from-env-universe-err-project.rs:55:30
|
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected associated type `<T as Trait<'static>>::Assoc`
found associated type `<T as Trait<'a>>::Assoc`

error[E0308]: mismatched types
--> $DIR/candidate-from-env-universe-err-project.rs:55:30
|
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected associated type `<T as Trait<'static>>::Assoc`
found associated type `<T as Trait<'a>>::Assoc`
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0308`.
Loading
Loading