Skip to content

Commit 987c3e4

Browse files
compiler-errorscompiler-errors
committed
Don't ICE because recomputing overflow goals during find_best_leaf_obligation causes inference side-effects
1 parent 14081a2 commit 987c3e4

18 files changed

+144
-225
lines changed

‎compiler/rustc_trait_selection/src/solve/fulfill.rs‎

Lines changed: 63 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -247,16 +247,24 @@ fn fulfillment_error_for_no_solution<'tcx>(
247247

248248
fn fulfillment_error_for_stalled<'tcx>(
249249
infcx: &InferCtxt<'tcx>,
250-
obligation: PredicateObligation<'tcx>,
250+
root_obligation: PredicateObligation<'tcx>,
251251
) -> FulfillmentError<'tcx> {
252-
let code = infcx.probe(|_| {
253-
match infcx.evaluate_root_goal(obligation.clone().into(), GenerateProofTree::Never).0 {
254-
Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => {
255-
FulfillmentErrorCode::Ambiguity { overflow: None }
256-
}
257-
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => {
258-
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) }
259-
}
252+
let (code, obligation) = infcx.probe(|_| {
253+
match infcx.evaluate_root_goal(root_obligation.clone().into(), GenerateProofTree::Never).0 {
254+
Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => (
255+
FulfillmentErrorCode::Ambiguity { overflow: None },
256+
find_best_leaf_obligation(infcx, &root_obligation, true),
257+
),
258+
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => (
259+
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) },
260+
// Don't look into overflows because we treat overflows weirdly anyways.
261+
// In `instantiate_response_discarding_overflow` we set `has_changed = false`,
262+
// recomputing the goal again during `find_best_leaf_obligation` may apply
263+
// inference guidance that makes other goals go from ambig -> pass, for example.
264+
//
265+
// FIXME: We should probably just look into overflows here.
266+
root_obligation.clone(),
267+
),
260268
Ok((_, Certainty::Yes)) => {
261269
bug!("did not expect successful goal when collecting ambiguity errors")
262270
}
@@ -266,11 +274,7 @@ fn fulfillment_error_for_stalled<'tcx>(
266274
}
267275
});
268276

269-
FulfillmentError {
270-
obligation: find_best_leaf_obligation(infcx, &obligation, true),
271-
code,
272-
root_obligation: obligation,
273-
}
277+
FulfillmentError { obligation, code, root_obligation }
274278
}
275279

276280
fn find_best_leaf_obligation<'tcx>(
@@ -305,41 +309,51 @@ impl<'tcx> BestObligation<'tcx> {
305309
res
306310
}
307311

308-
/// Filter out the candidates that aren't either error or ambiguous (depending
309-
/// on what we are looking for), and also throw out candidates that have no
310-
/// failing WC (or higher-ranked obligations, for which there should only be
311-
/// one candidate anyways -- but I digress). This most likely means that the
312-
/// goal just didn't unify at all, e.g. a param candidate with an alias in it.
312+
/// Filter out the candidates that aren't interesting to visit for the
313+
/// purposes of reporting errors. For ambiguities, we only consider
314+
/// candidates that may hold. For errors, we only consider candidates that
315+
/// *don't* hold and which have impl-where clauses that also don't hold.
313316
fn non_trivial_candidates<'a>(
314317
&self,
315318
goal: &'a InspectGoal<'a, 'tcx>,
316319
) -> Vec<InspectCandidate<'a, 'tcx>> {
317-
let mut candidates = goal
318-
.candidates()
319-
.into_iter()
320-
.filter(|candidate| match self.consider_ambiguities {
321-
true => matches!(candidate.result(), Ok(Certainty::Maybe(_))),
322-
false => matches!(candidate.result(), Err(_)),
323-
})
324-
.collect::<Vec<_>>();
325-
326-
// If we have >1 candidate, one may still be due to "boring" reasons, like
327-
// an alias-relate that failed to hold when deeply evaluated. We really
328-
// don't care about reasons like this.
329-
if candidates.len() > 1 {
330-
candidates.retain(|candidate| {
331-
goal.infcx().probe(|_| {
332-
candidate.instantiate_nested_goals(self.span()).iter().any(|nested_goal| {
333-
matches!(
334-
nested_goal.source(),
335-
GoalSource::ImplWhereBound | GoalSource::InstantiateHigherRanked
336-
) && match self.consider_ambiguities {
337-
true => matches!(nested_goal.result(), Ok(Certainty::Maybe(_))),
338-
false => matches!(nested_goal.result(), Err(_)),
339-
}
340-
})
341-
})
342-
});
320+
let mut candidates = goal.candidates();
321+
match self.consider_ambiguities {
322+
true => {
323+
// If we have an ambiguous obligation, we must consider *all* candidates
324+
// that hold, or else we may guide inference causing other goals to go
325+
// from ambig -> pass/fail.
326+
candidates.retain(|candidate| candidate.result().is_ok());
327+
}
328+
false => {
329+
candidates.retain(|candidate| candidate.result().is_err());
330+
// If we have >1 candidate, one may still be due to "boring" reasons, like
331+
// an alias-relate that failed to hold when deeply evaluated. We really
332+
// don't care about reasons like this.
333+
if candidates.len() > 1 {
334+
candidates.retain(|candidate| {
335+
goal.infcx().probe(|_| {
336+
candidate.instantiate_nested_goals(self.span()).iter().any(
337+
|nested_goal| {
338+
matches!(
339+
nested_goal.source(),
340+
GoalSource::ImplWhereBound
341+
| GoalSource::InstantiateHigherRanked
342+
) && match self.consider_ambiguities {
343+
true => {
344+
matches!(
345+
nested_goal.result(),
346+
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
347+
)
348+
}
349+
false => matches!(nested_goal.result(), Err(_)),
350+
}
351+
},
352+
)
353+
})
354+
});
355+
}
356+
}
343357
}
344358

345359
candidates
@@ -404,7 +418,10 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
404418

405419
// Skip nested goals that aren't the *reason* for our goal's failure.
406420
match self.consider_ambiguities {
407-
true if matches!(nested_goal.result(), Ok(Certainty::Maybe(_))) => {}
421+
true if matches!(
422+
nested_goal.result(),
423+
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
424+
) => {}
408425
false if matches!(nested_goal.result(), Err(_)) => {}
409426
_ => continue,
410427
}

‎tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
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<_>>>>>>>>>>>>>>>>>>>>>>`
1+
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<_>>>>>>>>>>>>>>>>>>>>>`
22
--> $DIR/coherence-fulfill-overflow.rs:12:1
33
|
44
LL | impl<T: ?Sized + TwoW> Trait for W<T> {}
55
| ------------------------------------- first implementation here
66
LL | impl<T: ?Sized + TwoW> Trait for T {}
7-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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<_>>>>>>>>>>>>>>>>>>>>>>`
7+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>>>>>>>>>>>`
88
|
9-
= note: overflow evaluating the requirement `W<W<W<W<_>>>>: TwoW`
9+
= 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`
1010
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "20"]` attribute to your crate (`coherence_fulfill_overflow`)
1111

1212
error: aborting due to 1 previous error

‎tests/ui/traits/next-solver/cycles/coinduction/fixpoint-exponential-growth.stderr‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,9 @@
1-
error[E0275]: overflow evaluating the requirement `_: Sized`
1+
error[E0275]: overflow evaluating the requirement `W<_>: Trait`
22
--> $DIR/fixpoint-exponential-growth.rs:33:13
33
|
44
LL | impls::<W<_>>();
55
| ^^^^
66
|
7-
note: required for `W<(W<_>, W<_>)>` to implement `Trait`
8-
--> $DIR/fixpoint-exponential-growth.rs:23:12
9-
|
10-
LL | impl<T, U> Trait for W<(W<T>, W<U>)>
11-
| - ^^^^^ ^^^^^^^^^^^^^^^
12-
| |
13-
| unsatisfied trait bound introduced here
147
note: required by a bound in `impls`
158
--> $DIR/fixpoint-exponential-growth.rs:30:13
169
|

‎tests/ui/traits/next-solver/cycles/double-cycle-inductive-coinductive.stderr‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,21 @@
1-
error[E0275]: overflow evaluating the requirement `(): Inductive`
1+
error[E0275]: overflow evaluating the requirement `(): Trait`
22
--> $DIR/double-cycle-inductive-coinductive.rs:32:19
33
|
44
LL | impls_trait::<()>();
55
| ^^
66
|
7-
note: required for `()` to implement `Trait`
8-
--> $DIR/double-cycle-inductive-coinductive.rs:9:34
9-
|
10-
LL | impl<T: Inductive + Coinductive> Trait for T {}
11-
| --------- ^^^^^ ^
12-
| |
13-
| unsatisfied trait bound introduced here
14-
note: required for `()` to implement `Inductive`
15-
--> $DIR/double-cycle-inductive-coinductive.rs:12:16
16-
|
17-
LL | impl<T: Trait> Inductive for T {}
18-
| ----- ^^^^^^^^^ ^
19-
| |
20-
| unsatisfied trait bound introduced here
21-
= note: 7 redundant requirements hidden
22-
= note: required for `()` to implement `Trait`
237
note: required by a bound in `impls_trait`
248
--> $DIR/double-cycle-inductive-coinductive.rs:17:19
259
|
2610
LL | fn impls_trait<T: Trait>() {}
2711
| ^^^^^ required by this bound in `impls_trait`
2812

29-
error[E0275]: overflow evaluating the requirement `(): CoinductiveRev`
13+
error[E0275]: overflow evaluating the requirement `(): TraitRev`
3014
--> $DIR/double-cycle-inductive-coinductive.rs:35:23
3115
|
3216
LL | impls_trait_rev::<()>();
3317
| ^^
3418
|
35-
note: required for `()` to implement `TraitRev`
36-
--> $DIR/double-cycle-inductive-coinductive.rs:21:40
37-
|
38-
LL | impl<T: CoinductiveRev + InductiveRev> TraitRev for T {}
39-
| -------------- ^^^^^^^^ ^
40-
| |
41-
| unsatisfied trait bound introduced here
42-
note: required for `()` to implement `CoinductiveRev`
43-
--> $DIR/double-cycle-inductive-coinductive.rs:27:19
44-
|
45-
LL | impl<T: TraitRev> CoinductiveRev for T {}
46-
| -------- ^^^^^^^^^^^^^^ ^
47-
| |
48-
| unsatisfied trait bound introduced here
49-
= note: 7 redundant requirements hidden
50-
= note: required for `()` to implement `TraitRev`
5119
note: required by a bound in `impls_trait_rev`
5220
--> $DIR/double-cycle-inductive-coinductive.rs:29:23
5321
|

‎tests/ui/traits/next-solver/cycles/inductive-fixpoint-hang.stderr‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,9 @@
1-
error[E0275]: overflow evaluating the requirement `W<W<_>>: Trait`
1+
error[E0275]: overflow evaluating the requirement `W<_>: Trait`
22
--> $DIR/inductive-fixpoint-hang.rs:31:19
33
|
44
LL | impls_trait::<W<_>>();
55
| ^^^^
66
|
7-
note: required for `W<W<W<_>>>` to implement `Trait`
8-
--> $DIR/inductive-fixpoint-hang.rs:22:17
9-
|
10-
LL | impl<T: ?Sized> Trait for W<W<T>>
11-
| ^^^^^ ^^^^^^^
12-
LL | where
13-
LL | W<T>: Trait,
14-
| ----- unsatisfied trait bound introduced here
15-
= note: 8 redundant requirements hidden
16-
= note: required for `W<W<W<W<W<W<W<W<W<W<W<_>>>>>>>>>>>` to implement `Trait`
177
note: required by a bound in `impls_trait`
188
--> $DIR/inductive-fixpoint-hang.rs:28:19
199
|

‎tests/ui/traits/next-solver/cycles/inductive-not-on-stack.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ fn impls_ar<T: AR>() {}
3939

4040
fn main() {
4141
impls_a::<()>();
42-
//~^ ERROR overflow evaluating the requirement `(): B`
42+
//~^ ERROR overflow evaluating the requirement `(): A`
4343

4444
impls_ar::<()>();
4545
//~^ ERROR overflow evaluating the requirement `(): AR`

‎tests/ui/traits/next-solver/cycles/inductive-not-on-stack.stderr‎

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,9 @@
1-
error[E0275]: overflow evaluating the requirement `(): B`
1+
error[E0275]: overflow evaluating the requirement `(): A`
22
--> $DIR/inductive-not-on-stack.rs:41:15
33
|
44
LL | impls_a::<()>();
55
| ^^
66
|
7-
note: required for `()` to implement `A`
8-
--> $DIR/inductive-not-on-stack.rs:21:16
9-
|
10-
LL | impl<T: B + C> A for T {}
11-
| - ^ ^
12-
| |
13-
| unsatisfied trait bound introduced here
14-
note: required for `()` to implement `B`
15-
--> $DIR/inductive-not-on-stack.rs:22:12
16-
|
17-
LL | impl<T: A> B for T {}
18-
| - ^ ^
19-
| |
20-
| unsatisfied trait bound introduced here
21-
= note: 7 redundant requirements hidden
22-
= note: required for `()` to implement `A`
237
note: required by a bound in `impls_a`
248
--> $DIR/inductive-not-on-stack.rs:25:15
259
|
@@ -32,29 +16,6 @@ error[E0275]: overflow evaluating the requirement `(): AR`
3216
LL | impls_ar::<()>();
3317
| ^^
3418
|
35-
note: required for `()` to implement `BR`
36-
--> $DIR/inductive-not-on-stack.rs:35:13
37-
|
38-
LL | impl<T: AR> BR for T {}
39-
| -- ^^ ^
40-
| |
41-
| unsatisfied trait bound introduced here
42-
note: required for `()` to implement `CR`
43-
--> $DIR/inductive-not-on-stack.rs:36:13
44-
|
45-
LL | impl<T: BR> CR for T {}
46-
| -- ^^ ^
47-
| |
48-
| unsatisfied trait bound introduced here
49-
note: required for `()` to implement `AR`
50-
--> $DIR/inductive-not-on-stack.rs:34:18
51-
|
52-
LL | impl<T: CR + BR> AR for T {}
53-
| -- ^^ ^
54-
| |
55-
| unsatisfied trait bound introduced here
56-
= note: 6 redundant requirements hidden
57-
= note: required for `()` to implement `AR`
5819
note: required by a bound in `impls_ar`
5920
--> $DIR/inductive-not-on-stack.rs:38:16
6021
|

‎tests/ui/traits/next-solver/cycles/mixed-cycles-1.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,5 @@ fn impls_a<T: A>() {}
3535

3636
fn main() {
3737
impls_a::<()>();
38-
//~^ ERROR overflow evaluating the requirement `(): CInd`
38+
//~^ ERROR overflow evaluating the requirement `(): A`
3939
}

‎tests/ui/traits/next-solver/cycles/mixed-cycles-1.stderr‎

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,9 @@
1-
error[E0275]: overflow evaluating the requirement `(): CInd`
1+
error[E0275]: overflow evaluating the requirement `(): A`
22
--> $DIR/mixed-cycles-1.rs:37:15
33
|
44
LL | impls_a::<()>();
55
| ^^
66
|
7-
note: required for `()` to implement `B`
8-
--> $DIR/mixed-cycles-1.rs:31:28
9-
|
10-
LL | impl<T: ?Sized + CInd + C> B for T {}
11-
| ---- ^ ^
12-
| |
13-
| unsatisfied trait bound introduced here
14-
note: required for `()` to implement `C`
15-
--> $DIR/mixed-cycles-1.rs:32:25
16-
|
17-
LL | impl<T: ?Sized + B + A> C for T {}
18-
| - ^ ^
19-
| |
20-
| unsatisfied trait bound introduced here
21-
note: required for `()` to implement `CInd`
22-
--> $DIR/mixed-cycles-1.rs:28:21
23-
|
24-
LL | impl<T: ?Sized + C> CInd for T {}
25-
| - ^^^^ ^
26-
| |
27-
| unsatisfied trait bound introduced here
28-
= note: 4 redundant requirements hidden
29-
= note: required for `()` to implement `B`
30-
note: required for `()` to implement `BInd`
31-
--> $DIR/mixed-cycles-1.rs:23:21
32-
|
33-
LL | impl<T: ?Sized + B> BInd for T {}
34-
| - ^^^^ ^
35-
| |
36-
| unsatisfied trait bound introduced here
37-
note: required for `()` to implement `A`
38-
--> $DIR/mixed-cycles-1.rs:30:28
39-
|
40-
LL | impl<T: ?Sized + BInd + C> A for T {}
41-
| ---- ^ ^
42-
| |
43-
| unsatisfied trait bound introduced here
447
note: required by a bound in `impls_a`
458
--> $DIR/mixed-cycles-1.rs:34:15
469
|

‎tests/ui/traits/next-solver/cycles/mixed-cycles-2.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,5 @@ fn impls_a<T: A>() {}
2828

2929
fn main() {
3030
impls_a::<()>();
31-
//~^ ERROR overflow evaluating the requirement `(): BInd`
31+
//~^ ERROR overflow evaluating the requirement `(): A`
3232
}

0 commit comments

Comments
 (0)