Skip to content

Commit

Permalink
Unrolled build for rust-lang#127568
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#127568 - lcnr:undo-leakcheck, r=oli-obk

instantiate higher ranked goals in candidate selection again

This reverts rust-lang#119820 as that PR has a significant impact and breaks code which *feels like it should work*. The impact ended up being larger than we expected during the FCP and we've ended up with some ideas for how we can work around this issue in the next solver. This has been discussed in the previous high bandwidth t-types meeting: https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2024-07-09.20high.20bandwidth.20meeting.

We'll therefore keep this inconsistency between the two solvers for now and will have to deal with it before stabilizating the use of the new solver outside of coherence: rust-lang/trait-system-refactor-initiative#120.

fixes rust-lang#125194 after a beta-backport.

The pattern which is more widely used than expected and feels like it should work, especially without deep knowledge of the type system is
```rust
trait Trait<'a> {}
impl<'a, T> Trait<'a> for T {}

fn trait_bound<T: for<'a> Trait<'a>>() {}

// A function with a where-bound which is more restrictive than the impl.
fn function1<T: Trait<'static>>() {
    // stable: ok
    // with rust-lang#119820: error as we prefer the where-bound over the impl
    // with this PR: back to ok
    trait_bound::<T>();
}

```

r? `@rust-lang/types`
  • Loading branch information
rust-timer authored Jul 10, 2024
2 parents b215beb + f77394f commit f30522e
Show file tree
Hide file tree
Showing 21 changed files with 121 additions and 299 deletions.
66 changes: 12 additions & 54 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,6 @@ mod _match;
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 @@ -402,10 +388,7 @@ 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, LeakCheckHigherRankedGoal::No)?
.may_apply()
{
if self.evaluate_candidate(stack, c)?.may_apply() {
no_candidates_apply = false;
break;
}
Expand Down Expand Up @@ -476,7 +459,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, LeakCheckHigherRankedGoal::No) {
.map(|c| match self.evaluate_candidate(stack, &c) {
Ok(eval) if eval.may_apply() => {
Ok(Some(EvaluatedCandidate { candidate: c, evaluation: eval }))
}
Expand Down Expand Up @@ -566,7 +549,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation: &PredicateObligation<'tcx>,
) -> Result<EvaluationResult, OverflowError> {
debug_assert!(!self.infcx.next_trait_solver());
self.evaluation_probe(|this, _outer_universe| {
self.evaluation_probe(|this| {
let goal =
this.infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env));
let mut result = this.evaluate_predicate_recursively(
Expand All @@ -589,11 +572,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
/// `op`, but this can be overwritten if necessary.
fn evaluation_probe(
&mut self,
op: impl FnOnce(&mut Self, &mut ty::UniverseIndex) -> Result<EvaluationResult, OverflowError>,
op: impl FnOnce(&mut Self) -> Result<EvaluationResult, OverflowError>,
) -> Result<EvaluationResult, OverflowError> {
self.infcx.probe(|snapshot| -> Result<EvaluationResult, OverflowError> {
let mut outer_universe = self.infcx.universe();
let result = op(self, &mut outer_universe)?;
let outer_universe = self.infcx.universe();
let result = op(self)?;

match self.infcx.leak_check(outer_universe, Some(snapshot)) {
Ok(()) => {}
Expand Down Expand Up @@ -1254,7 +1237,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}

match self.candidate_from_obligation(stack) {
Ok(Some(c)) => self.evaluate_candidate(stack, &c, LeakCheckHigherRankedGoal::Yes),
Ok(Some(c)) => self.evaluate_candidate(stack, &c),
Ok(None) => Ok(EvaluatedToAmbig),
Err(Overflow(OverflowError::Canonical)) => Err(OverflowError::Canonical),
Err(..) => Ok(EvaluatedToErr),
Expand All @@ -1279,10 +1262,6 @@ 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 @@ -1293,25 +1272,9 @@ 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, 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()) {
let mut result = self.evaluation_probe(|this| {
match this.confirm_candidate(stack.obligation, candidate.clone()) {
Ok(selection) => {
debug!(?selection);
this.evaluate_predicates_recursively(
Expand Down Expand Up @@ -1731,19 +1694,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
})
.map_err(|_| ())
}

fn where_clause_may_apply<'o>(
&mut self,
stack: &TraitObligationStack<'o, 'tcx>,
where_clause_trait_ref: ty::PolyTraitRef<'tcx>,
) -> Result<EvaluationResult, OverflowError> {
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) {
self.evaluation_probe(|this| {
match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) {
Ok(obligations) => this.evaluate_predicates_recursively(stack.list(), obligations),
Err(()) => Ok(EvaluatedToErr),
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0277]: the trait bound `for<'a> &'a &T: Trait` is not satisfied
--> $DIR/candidate-from-env-universe-err-1.rs:27:16
|
LL | hr_bound::<&T>();
| ^^ the trait `for<'a> Trait` is not implemented for `&'a &T`
|
note: required by a bound in `hr_bound`
--> $DIR/candidate-from-env-universe-err-1.rs:14:20
|
LL | fn hr_bound<T>()
| -------- required by a bound in this function
LL | where
LL | for<'a> &'a T: Trait,
| ^^^^^ required by this bound in `hr_bound`
help: consider removing the leading `&`-reference
|
LL - hr_bound::<&T>();
LL + hr_bound::<T>();
|

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
@@ -1,3 +1,7 @@
//@ revisions: old next
//@[next] compile-flags: -Znext-solver
//@[old] check-pass

// cc #119820

trait Trait {}
Expand All @@ -21,8 +25,7 @@ where
// 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
//[next]~^ ERROR the trait bound `for<'a> &'a &T: Trait` is not satisfied
}

fn main() {}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error[E0277]: the trait bound `for<'a> T: Trait<'a, '_>` is not satisfied
--> $DIR/candidate-from-env-universe-err-2.rs:14:5
--> $DIR/candidate-from-env-universe-err-2.rs:15: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
--> $DIR/candidate-from-env-universe-err-2.rs:12:19
|
LL | fn impl_hr<'b, T: for<'a> Trait<'a, 'b>>() {}
| ^^^^^^^^^^^^^^^^^^^^^ required by this bound in `impl_hr`
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@ revisions: current next
//@[next] compile-flags: -Znext-solver
//@[current] check-pass

// cc #119820

Expand All @@ -13,8 +14,6 @@ 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
@@ -1,23 +1,5 @@
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
--> $DIR/candidate-from-env-universe-err-project.rs:38:5
|
LL | projection_bound::<T>();
| ^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
Expand All @@ -31,7 +13,7 @@ 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
--> $DIR/candidate-from-env-universe-err-project.rs:53:30
|
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
Expand All @@ -40,7 +22,7 @@ LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
found associated type `<T as Trait<'a>>::Assoc`

error[E0308]: mismatched types
--> $DIR/candidate-from-env-universe-err-project.rs:55:30
--> $DIR/candidate-from-env-universe-err-project.rs:53:30
|
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
Expand All @@ -49,6 +31,6 @@ LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::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
error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ LL | fn function1<T: Trait<'static> + for<'a> Trait<'a>>() {
| +++++++++++++++++++

error[E0277]: the trait bound `for<'a> T: Trait<'a>` is not satisfied
--> $DIR/candidate-from-env-universe-err-project.rs:39:24
--> $DIR/candidate-from-env-universe-err-project.rs:38:24
|
LL | projection_bound::<T>();
| ^ the trait `for<'a> Trait<'a>` is not implemented for `T`
Expand All @@ -31,7 +31,7 @@ LL | fn function2<T: Trait<'static, Assoc = usize> + for<'a> Trait<'a>>() {
| +++++++++++++++++++

error[E0271]: type mismatch resolving `<T as Trait<'a>>::Assoc == usize`
--> $DIR/candidate-from-env-universe-err-project.rs:39:24
--> $DIR/candidate-from-env-universe-err-project.rs:38:24
|
LL | projection_bound::<T>();
| ^ type mismatch resolving `<T as Trait<'a>>::Assoc == usize`
Expand All @@ -48,13 +48,13 @@ LL | fn projection_bound<T: for<'a> Trait<'a, Assoc = usize>>() {}
| ^^^^^^^^^^^^^ required by this bound in `projection_bound`

error: higher-ranked subtype error
--> $DIR/candidate-from-env-universe-err-project.rs:55:30
--> $DIR/candidate-from-env-universe-err-project.rs:53:30
|
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: higher-ranked subtype error
--> $DIR/candidate-from-env-universe-err-project.rs:55:30
--> $DIR/candidate-from-env-universe-err-project.rs:53:30
|
LL | let _higher_ranked_norm: for<'a> fn(<T as Trait<'a>>::Assoc) = |_| ();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
Loading

0 comments on commit f30522e

Please sign in to comment.