Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 74b79ae

Browse filesBrowse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #140711 - compiler-errors:combine-maybes, r=lcnr
Do not discard constraints on overflow if there was candidate ambiguity Fixes rust-lang/trait-system-refactor-initiative#201. There's a pretty chunky justification in the test. r? lcnr
2 parents aace488 + a910329 commit 74b79ae
Copy full SHA for 74b79ae

File tree

Expand file treeCollapse file tree

7 files changed

+130
-24
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+130
-24
lines changed

‎compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs

Copy file name to clipboardExpand all lines: compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs
+5-2Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,14 @@ where
132132
(Certainty::Yes, NestedNormalizationGoals(goals))
133133
}
134134
_ => {
135-
let certainty = shallow_certainty.unify_with(goals_certainty);
135+
let certainty = shallow_certainty.and(goals_certainty);
136136
(certainty, NestedNormalizationGoals::empty())
137137
}
138138
};
139139

140-
if let Certainty::Maybe(cause @ MaybeCause::Overflow { .. }) = certainty {
140+
if let Certainty::Maybe(cause @ MaybeCause::Overflow { keep_constraints: false, .. }) =
141+
certainty
142+
{
141143
// If we have overflow, it's probable that we're substituting a type
142144
// into itself infinitely and any partial substitutions in the query
143145
// response are probably not useful anyways, so just return an empty
@@ -193,6 +195,7 @@ where
193195
debug!(?num_non_region_vars, "too many inference variables -> overflow");
194196
return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow {
195197
suggest_increasing_limit: true,
198+
keep_constraints: false,
196199
}));
197200
}
198201
}

‎compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

Copy file name to clipboardExpand all lines: compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ where
661661
Certainty::Yes => {}
662662
Certainty::Maybe(_) => {
663663
self.nested_goals.push((source, with_resolved_vars));
664-
unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
664+
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
665665
}
666666
}
667667
} else {
@@ -675,7 +675,7 @@ where
675675
Certainty::Yes => {}
676676
Certainty::Maybe(_) => {
677677
self.nested_goals.push((source, goal));
678-
unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
678+
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
679679
}
680680
}
681681
}

‎compiler/rustc_next_trait_solver/src/solve/mod.rs

Copy file name to clipboardExpand all lines: compiler/rustc_next_trait_solver/src/solve/mod.rs
+12-10Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -253,16 +253,18 @@ where
253253
}
254254

255255
fn bail_with_ambiguity(&mut self, responses: &[CanonicalResponse<I>]) -> CanonicalResponse<I> {
256-
debug_assert!(!responses.is_empty());
257-
if let Certainty::Maybe(maybe_cause) =
258-
responses.iter().fold(Certainty::AMBIGUOUS, |certainty, response| {
259-
certainty.unify_with(response.value.certainty)
260-
})
261-
{
262-
self.make_ambiguous_response_no_constraints(maybe_cause)
263-
} else {
264-
panic!("expected flounder response to be ambiguous")
265-
}
256+
debug_assert!(responses.len() > 1);
257+
let maybe_cause = responses.iter().fold(MaybeCause::Ambiguity, |maybe_cause, response| {
258+
// Pull down the certainty of `Certainty::Yes` to ambiguity when combining
259+
// these responses, b/c we're combining more than one response and this we
260+
// don't know which one applies.
261+
let candidate = match response.value.certainty {
262+
Certainty::Yes => MaybeCause::Ambiguity,
263+
Certainty::Maybe(candidate) => candidate,
264+
};
265+
maybe_cause.or(candidate)
266+
});
267+
self.make_ambiguous_response_no_constraints(maybe_cause)
266268
}
267269

268270
/// If we fail to merge responses we flounder and return overflow or ambiguity.

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

Copy file name to clipboardExpand all lines: compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,13 @@ pub(super) fn fulfillment_error_for_stalled<'tcx>(
9999
Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => {
100100
(FulfillmentErrorCode::Ambiguity { overflow: None }, true)
101101
}
102-
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => (
102+
Ok((
103+
_,
104+
Certainty::Maybe(MaybeCause::Overflow {
105+
suggest_increasing_limit,
106+
keep_constraints: _,
107+
}),
108+
)) => (
103109
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) },
104110
// Don't look into overflows because we treat overflows weirdly anyways.
105111
// We discard the inference constraints from overflowing goals, so

‎compiler/rustc_trait_selection/src/solve/inspect/analyse.rs

Copy file name to clipboardExpand all lines: compiler/rustc_trait_selection/src/solve/inspect/analyse.rs
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
382382
if let Some(term_hack) = normalizes_to_term_hack {
383383
infcx
384384
.probe(|_| term_hack.constrain(infcx, DUMMY_SP, uncanonicalized_goal.param_env))
385-
.map(|certainty| ok.value.certainty.unify_with(certainty))
385+
.map(|certainty| ok.value.certainty.and(certainty))
386386
} else {
387387
Ok(ok.value.certainty)
388388
}

‎compiler/rustc_type_ir/src/solve/mod.rs

Copy file name to clipboardExpand all lines: compiler/rustc_type_ir/src/solve/mod.rs
+47-8Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -273,17 +273,17 @@ impl Certainty {
273273
/// however matter for diagnostics. If `T: Foo` resulted in overflow and `T: Bar`
274274
/// in ambiguity without changing the inference state, we still want to tell the
275275
/// user that `T: Baz` results in overflow.
276-
pub fn unify_with(self, other: Certainty) -> Certainty {
276+
pub fn and(self, other: Certainty) -> Certainty {
277277
match (self, other) {
278278
(Certainty::Yes, Certainty::Yes) => Certainty::Yes,
279279
(Certainty::Yes, Certainty::Maybe(_)) => other,
280280
(Certainty::Maybe(_), Certainty::Yes) => self,
281-
(Certainty::Maybe(a), Certainty::Maybe(b)) => Certainty::Maybe(a.unify_with(b)),
281+
(Certainty::Maybe(a), Certainty::Maybe(b)) => Certainty::Maybe(a.and(b)),
282282
}
283283
}
284284

285285
pub const fn overflow(suggest_increasing_limit: bool) -> Certainty {
286-
Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit })
286+
Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: false })
287287
}
288288
}
289289

@@ -296,19 +296,58 @@ pub enum MaybeCause {
296296
/// or we hit a case where we just don't bother, e.g. `?x: Trait` goals.
297297
Ambiguity,
298298
/// We gave up due to an overflow, most often by hitting the recursion limit.
299-
Overflow { suggest_increasing_limit: bool },
299+
Overflow { suggest_increasing_limit: bool, keep_constraints: bool },
300300
}
301301

302302
impl MaybeCause {
303-
fn unify_with(self, other: MaybeCause) -> MaybeCause {
303+
fn and(self, other: MaybeCause) -> MaybeCause {
304304
match (self, other) {
305305
(MaybeCause::Ambiguity, MaybeCause::Ambiguity) => MaybeCause::Ambiguity,
306306
(MaybeCause::Ambiguity, MaybeCause::Overflow { .. }) => other,
307307
(MaybeCause::Overflow { .. }, MaybeCause::Ambiguity) => self,
308308
(
309-
MaybeCause::Overflow { suggest_increasing_limit: a },
310-
MaybeCause::Overflow { suggest_increasing_limit: b },
311-
) => MaybeCause::Overflow { suggest_increasing_limit: a || b },
309+
MaybeCause::Overflow {
310+
suggest_increasing_limit: limit_a,
311+
keep_constraints: keep_a,
312+
},
313+
MaybeCause::Overflow {
314+
suggest_increasing_limit: limit_b,
315+
keep_constraints: keep_b,
316+
},
317+
) => MaybeCause::Overflow {
318+
suggest_increasing_limit: limit_a && limit_b,
319+
keep_constraints: keep_a && keep_b,
320+
},
321+
}
322+
}
323+
324+
pub fn or(self, other: MaybeCause) -> MaybeCause {
325+
match (self, other) {
326+
(MaybeCause::Ambiguity, MaybeCause::Ambiguity) => MaybeCause::Ambiguity,
327+
328+
// When combining ambiguity + overflow, we can keep constraints.
329+
(
330+
MaybeCause::Ambiguity,
331+
MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: _ },
332+
) => MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: true },
333+
(
334+
MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: _ },
335+
MaybeCause::Ambiguity,
336+
) => MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: true },
337+
338+
(
339+
MaybeCause::Overflow {
340+
suggest_increasing_limit: limit_a,
341+
keep_constraints: keep_a,
342+
},
343+
MaybeCause::Overflow {
344+
suggest_increasing_limit: limit_b,
345+
keep_constraints: keep_b,
346+
},
347+
) => MaybeCause::Overflow {
348+
suggest_increasing_limit: limit_a || limit_b,
349+
keep_constraints: keep_a || keep_b,
350+
},
312351
}
313352
}
314353
}

‎tests/ui/traits/next-solver/overflow-plus-ambiguity-normalizes-to-response.rs

Copy file name to clipboard
+56Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//@ check-pass
2+
//@ compile-flags: -Znext-solver
3+
4+
// Regression test for <https://github.com/rust-lang/trait-system-refactor-initiative/issues/201>.
5+
// See comment below on `fn main`.
6+
7+
trait Intersect<U> {
8+
type Output;
9+
}
10+
11+
impl<T, U> Intersect<Vec<U>> for T
12+
where
13+
T: Intersect<U>,
14+
{
15+
type Output = T;
16+
}
17+
18+
impl Intersect<Cuboid> for Cuboid {
19+
type Output = Cuboid;
20+
}
21+
22+
fn intersect<T, U>(_: &T, _: &U) -> T::Output
23+
where
24+
T: Intersect<U>,
25+
{
26+
todo!()
27+
}
28+
29+
struct Cuboid;
30+
impl Cuboid {
31+
fn method(&self) {}
32+
}
33+
34+
fn main() {
35+
let x = vec![];
36+
// Here we end up trying to normalize `<Cuboid as Intersect<Vec<?0>>>::Output`
37+
// for the return type of the function, to constrain `y`. The impl then requires
38+
// `Cuboid: Intersect<?0>`, which has two candidates. One that constrains
39+
// `?0 = Vec<?1>`, which itself has the same two candidates and ends up leading
40+
// to a recursion depth overflow. In the second impl, we constrain `?0 = Cuboid`.
41+
//
42+
// Floundering leads to us combining the overflow candidate and yes candidate to
43+
// overflow. Because the response was overflow, we used to bubble it up to the
44+
// parent normalizes-to goal, which caused us to drop its constraint that would
45+
// guide us to normalize the associated type mentioned before.
46+
//
47+
// After this PR, we implement a new floundering "algebra" such that `Overflow OR Maybe`
48+
// returns anew `Overflow { keep_constraints: true }`, which means that we don't
49+
// need to drop constraints in the parent normalizes-to goal. This allows us to
50+
// normalize `y` to `Cuboid`, and allows us to call the method successfully. We
51+
// then constrain the `?0` in `let x: Vec<Cuboid> = x` below, so that we don't have
52+
// a left over ambiguous goal.
53+
let y = intersect(&Cuboid, &x);
54+
y.method();
55+
let x: Vec<Cuboid> = x;
56+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.