Skip to content

Commit d3f570b

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 d3f570b

18 files changed

+146
-220
lines changed

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

+65-41
Original file line numberDiff line numberDiff line change
@@ -247,16 +247,23 @@ 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 {
252+
let (code, refine_obligation) = infcx.probe(|_| {
253+
match infcx.evaluate_root_goal(root_obligation.clone().into(), GenerateProofTree::Never).0 {
254254
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) }
255+
(FulfillmentErrorCode::Ambiguity { overflow: None }, true)
259256
}
257+
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => (
258+
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) },
259+
// Don't look into overflows because we treat overflows weirdly anyways.
260+
// In `instantiate_response_discarding_overflow` we set `has_changed = false`,
261+
// recomputing the goal again during `find_best_leaf_obligation` may apply
262+
// inference guidance that makes other goals go from ambig -> pass, for example.
263+
//
264+
// FIXME: We should probably just look into overflows here.
265+
false,
266+
),
260267
Ok((_, Certainty::Yes)) => {
261268
bug!("did not expect successful goal when collecting ambiguity errors")
262269
}
@@ -267,9 +274,13 @@ fn fulfillment_error_for_stalled<'tcx>(
267274
});
268275

269276
FulfillmentError {
270-
obligation: find_best_leaf_obligation(infcx, &obligation, true),
277+
obligation: if refine_obligation {
278+
find_best_leaf_obligation(infcx, &root_obligation, true)
279+
} else {
280+
root_obligation.clone()
281+
},
271282
code,
272-
root_obligation: obligation,
283+
root_obligation,
273284
}
274285
}
275286

@@ -305,41 +316,51 @@ impl<'tcx> BestObligation<'tcx> {
305316
res
306317
}
307318

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.
319+
/// Filter out the candidates that aren't interesting to visit for the
320+
/// purposes of reporting errors. For ambiguities, we only consider
321+
/// candidates that may hold. For errors, we only consider candidates that
322+
/// *don't* hold and which have impl-where clauses that also don't hold.
313323
fn non_trivial_candidates<'a>(
314324
&self,
315325
goal: &'a InspectGoal<'a, 'tcx>,
316326
) -> 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-
});
327+
let mut candidates = goal.candidates();
328+
match self.consider_ambiguities {
329+
true => {
330+
// If we have an ambiguous obligation, we must consider *all* candidates
331+
// that hold, or else we may guide inference causing other goals to go
332+
// from ambig -> pass/fail.
333+
candidates.retain(|candidate| candidate.result().is_ok());
334+
}
335+
false => {
336+
candidates.retain(|candidate| candidate.result().is_err());
337+
// If we have >1 candidate, one may still be due to "boring" reasons, like
338+
// an alias-relate that failed to hold when deeply evaluated. We really
339+
// don't care about reasons like this.
340+
if candidates.len() > 1 {
341+
candidates.retain(|candidate| {
342+
goal.infcx().probe(|_| {
343+
candidate.instantiate_nested_goals(self.span()).iter().any(
344+
|nested_goal| {
345+
matches!(
346+
nested_goal.source(),
347+
GoalSource::ImplWhereBound
348+
| GoalSource::InstantiateHigherRanked
349+
) && match self.consider_ambiguities {
350+
true => {
351+
matches!(
352+
nested_goal.result(),
353+
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
354+
)
355+
}
356+
false => matches!(nested_goal.result(), Err(_)),
357+
}
358+
},
359+
)
360+
})
361+
});
362+
}
363+
}
343364
}
344365

345366
candidates
@@ -404,7 +425,10 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
404425

405426
// Skip nested goals that aren't the *reason* for our goal's failure.
406427
match self.consider_ambiguities {
407-
true if matches!(nested_goal.result(), Ok(Certainty::Maybe(_))) => {}
428+
true if matches!(
429+
nested_goal.result(),
430+
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
431+
) => {}
408432
false if matches!(nested_goal.result(), Err(_)) => {}
409433
_ => continue,
410434
}

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

+3-3
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

+1-8
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

+2-34
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

+1-11
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

+1-1
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

+1-40
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

+1-1
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

+1-38
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

+1-1
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)