Skip to content

Commit d3e510e

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

18 files changed

+145
-220
lines changed

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

+64-41
Original file line numberDiff line numberDiff line change
@@ -244,16 +244,23 @@ fn fulfillment_error_for_no_solution<'tcx>(
244244

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

266273
FulfillmentError {
267-
obligation: find_best_leaf_obligation(infcx, &obligation, true),
274+
obligation: if refine_obligation {
275+
find_best_leaf_obligation(infcx, &root_obligation, true)
276+
} else {
277+
root_obligation.clone()
278+
},
268279
code,
269-
root_obligation: obligation,
280+
root_obligation,
270281
}
271282
}
272283

@@ -302,41 +313,50 @@ impl<'tcx> BestObligation<'tcx> {
302313
res
303314
}
304315

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

342362
candidates
@@ -401,7 +421,10 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
401421

402422
// Skip nested goals that aren't the *reason* for our goal's failure.
403423
match self.consider_ambiguities {
404-
true if matches!(nested_goal.result(), Ok(Certainty::Maybe(_))) => {}
424+
true if matches!(
425+
nested_goal.result(),
426+
Ok(Certainty::Maybe(MaybeCause::Ambiguity))
427+
) => {}
405428
false if matches!(nested_goal.result(), Err(_)) => {}
406429
_ => continue,
407430
}

‎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)