Rollup merge of #140711 - compiler-errors:combine-maybes, r=lcnr

Do not discard constraints on overflow if there was candidate ambiguity

Fixes https://github.com/rust-lang/trait-system-refactor-initiative/issues/201.

There's a pretty chunky justification in the test.

r? lcnr
This commit is contained in:
Matthias Krüger 2025-05-08 08:14:18 +02:00 committed by GitHub
commit 74b79aee60
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 130 additions and 24 deletions

View file

@ -132,12 +132,14 @@ where
(Certainty::Yes, NestedNormalizationGoals(goals))
}
_ => {
let certainty = shallow_certainty.unify_with(goals_certainty);
let certainty = shallow_certainty.and(goals_certainty);
(certainty, NestedNormalizationGoals::empty())
}
};
if let Certainty::Maybe(cause @ MaybeCause::Overflow { .. }) = certainty {
if let Certainty::Maybe(cause @ MaybeCause::Overflow { keep_constraints: false, .. }) =
certainty
{
// If we have overflow, it's probable that we're substituting a type
// into itself infinitely and any partial substitutions in the query
// response are probably not useful anyways, so just return an empty
@ -193,6 +195,7 @@ where
debug!(?num_non_region_vars, "too many inference variables -> overflow");
return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow {
suggest_increasing_limit: true,
keep_constraints: false,
}));
}
}

View file

@ -661,7 +661,7 @@ where
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.nested_goals.push((source, with_resolved_vars));
unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
}
}
} else {
@ -675,7 +675,7 @@ where
Certainty::Yes => {}
Certainty::Maybe(_) => {
self.nested_goals.push((source, goal));
unchanged_certainty = unchanged_certainty.map(|c| c.unify_with(certainty));
unchanged_certainty = unchanged_certainty.map(|c| c.and(certainty));
}
}
}

View file

@ -253,16 +253,18 @@ where
}
fn bail_with_ambiguity(&mut self, responses: &[CanonicalResponse<I>]) -> CanonicalResponse<I> {
debug_assert!(!responses.is_empty());
if let Certainty::Maybe(maybe_cause) =
responses.iter().fold(Certainty::AMBIGUOUS, |certainty, response| {
certainty.unify_with(response.value.certainty)
})
{
self.make_ambiguous_response_no_constraints(maybe_cause)
} else {
panic!("expected flounder response to be ambiguous")
}
debug_assert!(responses.len() > 1);
let maybe_cause = responses.iter().fold(MaybeCause::Ambiguity, |maybe_cause, response| {
// Pull down the certainty of `Certainty::Yes` to ambiguity when combining
// these responses, b/c we're combining more than one response and this we
// don't know which one applies.
let candidate = match response.value.certainty {
Certainty::Yes => MaybeCause::Ambiguity,
Certainty::Maybe(candidate) => candidate,
};
maybe_cause.or(candidate)
});
self.make_ambiguous_response_no_constraints(maybe_cause)
}
/// If we fail to merge responses we flounder and return overflow or ambiguity.

View file

@ -99,7 +99,13 @@ pub(super) fn fulfillment_error_for_stalled<'tcx>(
Ok((_, Certainty::Maybe(MaybeCause::Ambiguity))) => {
(FulfillmentErrorCode::Ambiguity { overflow: None }, true)
}
Ok((_, Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit }))) => (
Ok((
_,
Certainty::Maybe(MaybeCause::Overflow {
suggest_increasing_limit,
keep_constraints: _,
}),
)) => (
FulfillmentErrorCode::Ambiguity { overflow: Some(suggest_increasing_limit) },
// Don't look into overflows because we treat overflows weirdly anyways.
// We discard the inference constraints from overflowing goals, so

View file

@ -382,7 +382,7 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {
if let Some(term_hack) = normalizes_to_term_hack {
infcx
.probe(|_| term_hack.constrain(infcx, DUMMY_SP, uncanonicalized_goal.param_env))
.map(|certainty| ok.value.certainty.unify_with(certainty))
.map(|certainty| ok.value.certainty.and(certainty))
} else {
Ok(ok.value.certainty)
}

View file

@ -273,17 +273,17 @@ impl Certainty {
/// however matter for diagnostics. If `T: Foo` resulted in overflow and `T: Bar`
/// in ambiguity without changing the inference state, we still want to tell the
/// user that `T: Baz` results in overflow.
pub fn unify_with(self, other: Certainty) -> Certainty {
pub fn and(self, other: Certainty) -> Certainty {
match (self, other) {
(Certainty::Yes, Certainty::Yes) => Certainty::Yes,
(Certainty::Yes, Certainty::Maybe(_)) => other,
(Certainty::Maybe(_), Certainty::Yes) => self,
(Certainty::Maybe(a), Certainty::Maybe(b)) => Certainty::Maybe(a.unify_with(b)),
(Certainty::Maybe(a), Certainty::Maybe(b)) => Certainty::Maybe(a.and(b)),
}
}
pub const fn overflow(suggest_increasing_limit: bool) -> Certainty {
Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit })
Certainty::Maybe(MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: false })
}
}
@ -296,19 +296,58 @@ pub enum MaybeCause {
/// or we hit a case where we just don't bother, e.g. `?x: Trait` goals.
Ambiguity,
/// We gave up due to an overflow, most often by hitting the recursion limit.
Overflow { suggest_increasing_limit: bool },
Overflow { suggest_increasing_limit: bool, keep_constraints: bool },
}
impl MaybeCause {
fn unify_with(self, other: MaybeCause) -> MaybeCause {
fn and(self, other: MaybeCause) -> MaybeCause {
match (self, other) {
(MaybeCause::Ambiguity, MaybeCause::Ambiguity) => MaybeCause::Ambiguity,
(MaybeCause::Ambiguity, MaybeCause::Overflow { .. }) => other,
(MaybeCause::Overflow { .. }, MaybeCause::Ambiguity) => self,
(
MaybeCause::Overflow { suggest_increasing_limit: a },
MaybeCause::Overflow { suggest_increasing_limit: b },
) => MaybeCause::Overflow { suggest_increasing_limit: a || b },
MaybeCause::Overflow {
suggest_increasing_limit: limit_a,
keep_constraints: keep_a,
},
MaybeCause::Overflow {
suggest_increasing_limit: limit_b,
keep_constraints: keep_b,
},
) => MaybeCause::Overflow {
suggest_increasing_limit: limit_a && limit_b,
keep_constraints: keep_a && keep_b,
},
}
}
pub fn or(self, other: MaybeCause) -> MaybeCause {
match (self, other) {
(MaybeCause::Ambiguity, MaybeCause::Ambiguity) => MaybeCause::Ambiguity,
// When combining ambiguity + overflow, we can keep constraints.
(
MaybeCause::Ambiguity,
MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: _ },
) => MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: true },
(
MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: _ },
MaybeCause::Ambiguity,
) => MaybeCause::Overflow { suggest_increasing_limit, keep_constraints: true },
(
MaybeCause::Overflow {
suggest_increasing_limit: limit_a,
keep_constraints: keep_a,
},
MaybeCause::Overflow {
suggest_increasing_limit: limit_b,
keep_constraints: keep_b,
},
) => MaybeCause::Overflow {
suggest_increasing_limit: limit_a || limit_b,
keep_constraints: keep_a || keep_b,
},
}
}
}

View file

@ -0,0 +1,56 @@
//@ check-pass
//@ compile-flags: -Znext-solver
// Regression test for <https://github.com/rust-lang/trait-system-refactor-initiative/issues/201>.
// See comment below on `fn main`.
trait Intersect<U> {
type Output;
}
impl<T, U> Intersect<Vec<U>> for T
where
T: Intersect<U>,
{
type Output = T;
}
impl Intersect<Cuboid> for Cuboid {
type Output = Cuboid;
}
fn intersect<T, U>(_: &T, _: &U) -> T::Output
where
T: Intersect<U>,
{
todo!()
}
struct Cuboid;
impl Cuboid {
fn method(&self) {}
}
fn main() {
let x = vec![];
// Here we end up trying to normalize `<Cuboid as Intersect<Vec<?0>>>::Output`
// for the return type of the function, to constrain `y`. The impl then requires
// `Cuboid: Intersect<?0>`, which has two candidates. One that constrains
// `?0 = Vec<?1>`, which itself has the same two candidates and ends up leading
// to a recursion depth overflow. In the second impl, we constrain `?0 = Cuboid`.
//
// Floundering leads to us combining the overflow candidate and yes candidate to
// overflow. Because the response was overflow, we used to bubble it up to the
// parent normalizes-to goal, which caused us to drop its constraint that would
// guide us to normalize the associated type mentioned before.
//
// After this PR, we implement a new floundering "algebra" such that `Overflow OR Maybe`
// returns anew `Overflow { keep_constraints: true }`, which means that we don't
// need to drop constraints in the parent normalizes-to goal. This allows us to
// normalize `y` to `Cuboid`, and allows us to call the method successfully. We
// then constrain the `?0` in `let x: Vec<Cuboid> = x` below, so that we don't have
// a left over ambiguous goal.
let y = intersect(&Cuboid, &x);
y.method();
let x: Vec<Cuboid> = x;
}