Skip to content

Do not discard constraints on overflow if there was candidate ambiguity #140711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
}));
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,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 {
Expand All @@ -680,7 +680,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));
}
}
}
Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_next_trait_solver/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
55 changes: 47 additions & 8 deletions compiler/rustc_type_ir/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,17 +266,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 })
}
}

Expand All @@ -289,19 +289,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,
},
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Loading