Skip to content

Commit

Permalink
Unrolled build for rust-lang#124871
Browse files Browse the repository at this point in the history 
Rollup merge of rust-lang#124871 - compiler-errors:overflowo, r=lcnr

Don't ICE because recomputing overflow goals during find_best_leaf_obligation causes inference side-effects

See the comments for more info. Reprocessing overflowed obligations may cause *other* goals to go from ambig -> pass/fail, causing an ICE. This suppresses that error, but makes all the overflow obligations messages back to their root obl. That kinda sucks, but 🤷

Fixes rust-lang#124834
Fixes rust-lang#124845

r? lcnr
  • Loading branch information
@rust-timer
rust-timer committed May 16, 2024
2 parents 4a78c00 + d3e510e commit f93e7ed
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 220 deletions.
105 changes: 64 additions & 41 deletions compiler/rustc_trait_selection/src/solve/fulfill.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,16 +244,23 @@ fn fulfillment_error_for_no_solution<'tcx>(

fn fulfillment_error_for_stalled<'tcx>(
infcx: &InferCtxt<'tcx>,
obligation: PredicateObligation<'tcx>,
root_obligation: PredicateObligation<'tcx>,
) -> FulfillmentError<'tcx> {
let code = infcx.probe(|_| {
match infcx.evaluate_root_goal(obligation.clone().into(), GenerateProofTree::Never).0 {
let (code, refine_obligation) = infcx.probe(|_| {
match infcx.evaluate_root_goal(root_obligation.clone().into(), GenerateProofTree::Never).0 {
Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => {
FulfillmentErrorCode::Ambiguity { overflow: None }
}
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => {
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) }
(FulfillmentErrorCode::Ambiguity { overflow: None }, true)
}
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => (
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) },
// Don't look into overflows because we treat overflows weirdly anyways.
// In `instantiate_response_discarding_overflow` we set `has_changed = false`,
// recomputing the goal again during `find_best_leaf_obligation` may apply
// inference guidance that makes other goals go from ambig -> pass, for example.
//
// FIXME: We should probably just look into overflows here.
false,
),
Ok((_, Certainty::Yes)) => {
bug!("did not expect successful goal when collecting ambiguity errors")
}
Expand All @@ -264,9 +271,13 @@ fn fulfillment_error_for_stalled<'tcx>(
});

FulfillmentError {
obligation: find_best_leaf_obligation(infcx, &obligation, true),
obligation: if refine_obligation {
find_best_leaf_obligation(infcx, &root_obligation, true)
} else {
root_obligation.clone()
},
code,
root_obligation: obligation,
root_obligation,
}
}

Expand Down Expand Up @@ -302,41 +313,50 @@ impl<'tcx> BestObligation<'tcx> {
res
}

/// Filter out the candidates that aren't either error or ambiguous (depending
/// on what we are looking for), and also throw out candidates that have no
/// failing WC (or higher-ranked obligations, for which there should only be
/// one candidate anyways -- but I digress). This most likely means that the
/// goal just didn't unify at all, e.g. a param candidate with an alias in it.
/// Filter out the candidates that aren't interesting to visit for the
/// purposes of reporting errors. For ambiguities, we only consider
/// candidates that may hold. For errors, we only consider candidates that
/// *don't* hold and which have impl-where clauses that also don't hold.
fn non_trivial_candidates<'a>(
&self,
goal: &'a InspectGoal<'a, 'tcx>,
) -> Vec<InspectCandidate<'a, 'tcx>> {
let mut candidates = goal
.candidates()
.into_iter()
.filter(|candidate| match self.consider_ambiguities {
true => matches!(candidate.result(), Ok(Certainty::Maybe(_))),
false => matches!(candidate.result(), Err(_)),
})
.collect::<Vec<_>>();

// If we have >1 candidate, one may still be due to "boring" reasons, like
// an alias-relate that failed to hold when deeply evaluated. We really
// don't care about reasons like this.
if candidates.len() > 1 {
candidates.retain(|candidate| {
goal.infcx().probe(|_| {
candidate.instantiate_nested_goals(self.span()).iter().any(|nested_goal| {
matches!(
nested_goal.source(),
GoalSource::ImplWhereBound | GoalSource::InstantiateHigherRanked
) && match self.consider_ambiguities {
true => matches!(nested_goal.result(), Ok(Certainty::Maybe(_))),
false => matches!(nested_goal.result(), Err(_)),
}
})
})
});
let mut candidates = goal.candidates();
match self.consider_ambiguities {
true => {
// If we have an ambiguous obligation, we must consider *all* candidates
// that hold, or else we may guide inference causing other goals to go
// from ambig -> pass/fail.
candidates.retain(|candidate| candidate.result().is_ok());
}
false => {
// If we have >1 candidate, one may still be due to "boring" reasons, like
// an alias-relate that failed to hold when deeply evaluated. We really
// don't care about reasons like this.
if candidates.len() > 1 {
candidates.retain(|candidate| {
goal.infcx().probe(|_| {
candidate.instantiate_nested_goals(self.span()).iter().any(
|nested_goal| {
matches!(
nested_goal.source(),
GoalSource::ImplWhereBound
| GoalSource::InstantiateHigherRanked
) && match self.consider_ambiguities {
true => {
matches!(
nested_goal.result(),
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
)
}
false => matches!(nested_goal.result(), Err(_)),
}
},
)
})
});
}
}
}

candidates
Expand Down Expand Up @@ -401,7 +421,10 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {

// Skip nested goals that aren't the *reason* for our goal's failure.
match self.consider_ambiguities {
true if matches!(nested_goal.result(), Ok(Certainty::Maybe(_))) => {}
true if matches!(
nested_goal.result(),
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
) => {}
false if matches!(nested_goal.result(), Err(_)) => {}
_ => continue,
}
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error[E0119]: conflicting implementations of trait `Trait` for type `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>>`
error[E0119]: conflicting implementations of trait `Trait` for type `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>`
--> $DIR/coherence-fulfill-overflow.rs:12:1
|
LL | impl<T: ?Sized + TwoW> Trait for W<T> {}
| ------------------------------------- first implementation here
LL | impl<T: ?Sized + TwoW> Trait for T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>>`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>`
|
= note: overflow evaluating the requirement `W<W<W<W<_>>>>: TwoW`
= note: overflow evaluating the requirement `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>: TwoW`
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "20"]` attribute to your crate (`coherence_fulfill_overflow`)

error: aborting due to 1 previous error
Expand Down
9 changes: 1 addition & 8 deletions tests/ui/traits/next-solver/cycles/coinduction/fixpoint-exponential-growth.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
error[E0275]: overflow evaluating the requirement `_: Sized`
error[E0275]: overflow evaluating the requirement `W<_>: Trait`
--> $DIR/fixpoint-exponential-growth.rs:33:13
|
LL | impls::<W<_>>();
| ^^^^
|
note: required for `W<(W<_>, W<_>)>` to implement `Trait`
--> $DIR/fixpoint-exponential-growth.rs:23:12
|
LL | impl<T, U> Trait for W<(W<T>, W<U>)>
| - ^^^^^ ^^^^^^^^^^^^^^^
| |
| unsatisfied trait bound introduced here
note: required by a bound in `impls`
--> $DIR/fixpoint-exponential-growth.rs:30:13
|
Expand Down
36 changes: 2 additions & 34 deletions tests/ui/traits/next-solver/cycles/double-cycle-inductive-coinductive.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,53 +1,21 @@
error[E0275]: overflow evaluating the requirement `(): Inductive`
error[E0275]: overflow evaluating the requirement `(): Trait`
--> $DIR/double-cycle-inductive-coinductive.rs:32:19
|
LL | impls_trait::<()>();
| ^^
|
note: required for `()` to implement `Trait`
--> $DIR/double-cycle-inductive-coinductive.rs:9:34
|
LL | impl<T: Inductive + Coinductive> Trait for T {}
| --------- ^^^^^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `Inductive`
--> $DIR/double-cycle-inductive-coinductive.rs:12:16
|
LL | impl<T: Trait> Inductive for T {}
| ----- ^^^^^^^^^ ^
| |
| unsatisfied trait bound introduced here
= note: 7 redundant requirements hidden
= note: required for `()` to implement `Trait`
note: required by a bound in `impls_trait`
--> $DIR/double-cycle-inductive-coinductive.rs:17:19
|
LL | fn impls_trait<T: Trait>() {}
| ^^^^^ required by this bound in `impls_trait`

error[E0275]: overflow evaluating the requirement `(): CoinductiveRev`
error[E0275]: overflow evaluating the requirement `(): TraitRev`
--> $DIR/double-cycle-inductive-coinductive.rs:35:23
|
LL | impls_trait_rev::<()>();
| ^^
|
note: required for `()` to implement `TraitRev`
--> $DIR/double-cycle-inductive-coinductive.rs:21:40
|
LL | impl<T: CoinductiveRev + InductiveRev> TraitRev for T {}
| -------------- ^^^^^^^^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `CoinductiveRev`
--> $DIR/double-cycle-inductive-coinductive.rs:27:19
|
LL | impl<T: TraitRev> CoinductiveRev for T {}
| -------- ^^^^^^^^^^^^^^ ^
| |
| unsatisfied trait bound introduced here
= note: 7 redundant requirements hidden
= note: required for `()` to implement `TraitRev`
note: required by a bound in `impls_trait_rev`
--> $DIR/double-cycle-inductive-coinductive.rs:29:23
|
Expand Down
12 changes: 1 addition & 11 deletions tests/ui/traits/next-solver/cycles/inductive-fixpoint-hang.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,9 @@
error[E0275]: overflow evaluating the requirement `W<W<_>>: Trait`
error[E0275]: overflow evaluating the requirement `W<_>: Trait`
--> $DIR/inductive-fixpoint-hang.rs:31:19
|
LL | impls_trait::<W<_>>();
| ^^^^
|
note: required for `W<W<W<_>>>` to implement `Trait`
--> $DIR/inductive-fixpoint-hang.rs:22:17
|
LL | impl<T: ?Sized> Trait for W<W<T>>
| ^^^^^ ^^^^^^^
LL | where
LL | W<T>: Trait,
| ----- unsatisfied trait bound introduced here
= note: 8 redundant requirements hidden
= note: required for `W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>` to implement `Trait`
note: required by a bound in `impls_trait`
--> $DIR/inductive-fixpoint-hang.rs:28:19
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/next-solver/cycles/inductive-not-on-stack.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn impls_ar<T: AR>() {}

fn main() {
impls_a::<()>();
//~^ ERROR overflow evaluating the requirement `(): B`
//~^ ERROR overflow evaluating the requirement `(): A`

impls_ar::<()>();
//~^ ERROR overflow evaluating the requirement `(): AR`
Expand Down
41 changes: 1 addition & 40 deletions tests/ui/traits/next-solver/cycles/inductive-not-on-stack.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
error[E0275]: overflow evaluating the requirement `(): B`
error[E0275]: overflow evaluating the requirement `(): A`
--> $DIR/inductive-not-on-stack.rs:41:15
|
LL | impls_a::<()>();
| ^^
|
note: required for `()` to implement `A`
--> $DIR/inductive-not-on-stack.rs:21:16
|
LL | impl<T: B + C> A for T {}
| - ^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `B`
--> $DIR/inductive-not-on-stack.rs:22:12
|
LL | impl<T: A> B for T {}
| - ^ ^
| |
| unsatisfied trait bound introduced here
= note: 7 redundant requirements hidden
= note: required for `()` to implement `A`
note: required by a bound in `impls_a`
--> $DIR/inductive-not-on-stack.rs:25:15
|
Expand All @@ -32,29 +16,6 @@ error[E0275]: overflow evaluating the requirement `(): AR`
LL | impls_ar::<()>();
| ^^
|
note: required for `()` to implement `BR`
--> $DIR/inductive-not-on-stack.rs:35:13
|
LL | impl<T: AR> BR for T {}
| -- ^^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `CR`
--> $DIR/inductive-not-on-stack.rs:36:13
|
LL | impl<T: BR> CR for T {}
| -- ^^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `AR`
--> $DIR/inductive-not-on-stack.rs:34:18
|
LL | impl<T: CR + BR> AR for T {}
| -- ^^ ^
| |
| unsatisfied trait bound introduced here
= note: 6 redundant requirements hidden
= note: required for `()` to implement `AR`
note: required by a bound in `impls_ar`
--> $DIR/inductive-not-on-stack.rs:38:16
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/next-solver/cycles/mixed-cycles-1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,5 @@ fn impls_a<T: A>() {}

fn main() {
impls_a::<()>();
//~^ ERROR overflow evaluating the requirement `(): CInd`
//~^ ERROR overflow evaluating the requirement `(): A`
}
39 changes: 1 addition & 38 deletions tests/ui/traits/next-solver/cycles/mixed-cycles-1.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,46 +1,9 @@
error[E0275]: overflow evaluating the requirement `(): CInd`
error[E0275]: overflow evaluating the requirement `(): A`
--> $DIR/mixed-cycles-1.rs:37:15
|
LL | impls_a::<()>();
| ^^
|
note: required for `()` to implement `B`
--> $DIR/mixed-cycles-1.rs:31:28
|
LL | impl<T: ?Sized + CInd + C> B for T {}
| ---- ^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `C`
--> $DIR/mixed-cycles-1.rs:32:25
|
LL | impl<T: ?Sized + B + A> C for T {}
| - ^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `CInd`
--> $DIR/mixed-cycles-1.rs:28:21
|
LL | impl<T: ?Sized + C> CInd for T {}
| - ^^^^ ^
| |
| unsatisfied trait bound introduced here
= note: 4 redundant requirements hidden
= note: required for `()` to implement `B`
note: required for `()` to implement `BInd`
--> $DIR/mixed-cycles-1.rs:23:21
|
LL | impl<T: ?Sized + B> BInd for T {}
| - ^^^^ ^
| |
| unsatisfied trait bound introduced here
note: required for `()` to implement `A`
--> $DIR/mixed-cycles-1.rs:30:28
|
LL | impl<T: ?Sized + BInd + C> A for T {}
| ---- ^ ^
| |
| unsatisfied trait bound introduced here
note: required by a bound in `impls_a`
--> $DIR/mixed-cycles-1.rs:34:15
|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/traits/next-solver/cycles/mixed-cycles-2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@ fn impls_a<T: A>() {}

fn main() {
impls_a::<()>();
//~^ ERROR overflow evaluating the requirement `(): BInd`
//~^ ERROR overflow evaluating the requirement `(): A`
}

0 comments on commit f93e7ed

Please sign in to comment.