From 96536dddaa69977763ce1dca79d8ab504761d27e Mon Sep 17 00:00:00 2001 From: Adwin White Date: Mon, 24 Aug 2026 16:46:03 +0800 Subject: [PATCH] eagerly report overflow errors --- .../src/solve/fulfill.rs | 99 +++++-------------- .../src/solve/fulfill/derive_errors.rs | 11 --- .../coherence-fulfill-overflow.stderr | 3 + .../dont-drop-obligations-on-overflow-1.rs | 53 ++++++++++ ...dont-drop-obligations-on-overflow-1.stderr | 11 +++ .../dont-drop-obligations-on-overflow-2.rs | 27 +++++ ...dont-drop-obligations-on-overflow-2.stderr | 19 ++++ 7 files changed, 137 insertions(+), 86 deletions(-) create mode 100644 tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.rs create mode 100644 tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.stderr create mode 100644 tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.rs create mode 100644 tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.stderr diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index 318897c36b010..7d3c0a4a6c1f1 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -17,6 +17,7 @@ use tracing::instrument; use self::derive_errors::*; use super::Certainty; use super::delegate::SolverDelegate; +use crate::error_reporting::InferCtxtErrorExt; use crate::traits::{FulfillmentError, FulfillmentErrorCode, ScrubbedTraitError}; mod derive_errors; @@ -53,12 +54,6 @@ pub struct FulfillmentCtxt<'tcx, E: 'tcx> { #[derive(Default, Debug)] struct ObligationStorage<'tcx> { - /// Obligations which resulted in an overflow in fulfillment itself. - /// - /// We cannot eagerly return these as error so we instead store them here - /// to avoid recomputing them each time `try_evaluate_obligations` is called. - /// This also allows us to return the correct `FulfillmentError` for them. - overflowed: Vec>, pending: PendingObligations<'tcx>, } @@ -72,24 +67,18 @@ impl<'tcx> ObligationStorage<'tcx> { } fn has_pending_obligations(&self) -> bool { - !self.pending.is_empty() || !self.overflowed.is_empty() + !self.pending.is_empty() } fn clone_pending(&self) -> PredicateObligations<'tcx> { - let mut obligations: PredicateObligations<'tcx> = - self.pending.iter().map(|(o, _)| o.clone()).collect(); - obligations.extend(self.overflowed.iter().cloned()); - obligations + self.pending.iter().map(|(o, _)| o.clone()).collect() } fn clone_pending_filtered(&self, f: F) -> PredicateObligations<'tcx> where F: FnMut(&&(PredicateObligation<'tcx>, Option>>)) -> bool, { - let mut obligations: PredicateObligations<'tcx> = - self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect(); - obligations.extend(self.overflowed.iter().cloned()); - obligations + self.pending.iter().filter(f).map(|(o, _)| o.clone()).collect() } fn drain_pending( @@ -101,29 +90,6 @@ impl<'tcx> ObligationStorage<'tcx> { self.pending = pending; unstalled } - - fn on_fulfillment_overflow(&mut self, infcx: &InferCtxt<'tcx>) { - infcx.probe(|_| { - // IMPORTANT: we must not use solve any inference variables in the obligations - // as this is all happening inside of a probe. We use a probe to make sure - // we get all obligations involved in the overflow. We pretty much check: if - // we were to do another step of `try_evaluate_obligations`, which goals would - // change. - self.overflowed.extend( - self.pending - .extract_if(.., |(o, stalled_on)| { - let goal = o.as_goal(); - let result = <&SolverDelegate<'tcx>>::from(infcx).evaluate_root_goal( - goal, - o.cause.span, - stalled_on.take(), - ); - matches!(result, Ok(GoalEvaluation { has_changed: HasChanged::Yes, .. })) - }) - .map(|(o, _)| o), - ); - }) - } } impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> { @@ -186,7 +152,7 @@ where #[inline] fn collect_remaining_errors(&mut self, infcx: &InferCtxt<'tcx>) -> TraitErrors { - if self.obligations.pending.is_empty() && self.obligations.overflowed.is_empty() { + if self.obligations.pending.is_empty() { // Typically in more than 99.9% of cases this condition is true, therefore we outline // the other case. TraitErrors::NoErrors @@ -202,13 +168,8 @@ where let delegate = <&SolverDelegate<'tcx>>::from(infcx); loop { let mut any_changed = false; - let mut overflowed = false; self.obligations.pending.retain_mut(|(obligation, opt_stalled_on)| { - if overflowed { - return false; - } - // Common case: still stalled; keep the obligation. This path is extremely hot in // some cases; there can be thousands of pending obligations. if let Some(stalled_on) = opt_stalled_on @@ -239,21 +200,26 @@ where // constrained by evaluating the goal. obligation.predicate = goal.predicate; if has_changed == HasChanged::Yes { - // We increment the recursion depth here to track the number of times - // this goal has resulted in inference progress. This doesn't precisely - // model the way that we track recursion depth in the old solver due - // to the fact that we only process root obligations, but it is a good - // approximation and should only result in fulfillment overflow in - // pathological cases. - obligation.recursion_depth += 1; - if !infcx.tcx.recursion_limit().value_within_limit(obligation.recursion_depth) { - // At this point we want to stop evaluating goals. We can't break out of - // `retain_mut`, so instead we set this flag which causes all other - // elements to be skipped. - overflowed = true; - return false; + // We limit the total count of inference progress to avoid hang so we don't + // try to recover from this. + // It's more complicated to collect all overflows thus we stopped doing that. + // Eager aborting is also what the old solver does. + // + // Note: it's incredibly rare to actually encounter fulfillment overflow + // as a single obligation would have to result in different inference progress + // a `recursion_depth` number of times. This mostly happens in bugs or with + // `Subtype` obligations because we no longer use the `sub_unification_table` + // in generalization. + infcx.err_ctxt().report_overflow_obligation(obligation, true); } else { + // We increment the recursion depth here to track the number of times + // this goal has resulted in inference progress. This doesn't precisely + // model the way that we track recursion depth in the old solver due + // to the fact that we only process root obligations, but it is a good + // approximation and should only result in fulfillment overflow in + // pathological cases. + obligation.recursion_depth += 1; any_changed = true; } } @@ -288,11 +254,6 @@ where } } }); - if overflowed { - self.obligations.on_fulfillment_overflow(infcx); - // Only return true errors that we have accumulated while processing. - return errors; - } if !any_changed { break; @@ -409,12 +370,6 @@ where .filter_map(|(obligation, _)| { try_ambiguity_error_for_stalled(infcx, obligation).map(NextSolverError::Ambiguity) }) - .chain( - cx.obligations - .overflowed - .drain(..) - .map(|obligation| NextSolverError::Overflow(obligation)), - ) .map(|e| E::from_solver_error(infcx, e)) .collect() } @@ -432,7 +387,6 @@ pub struct NextSolverAmbiguityError<'tcx> { pub enum NextSolverError<'tcx> { TrueError(PredicateObligation<'tcx>), Ambiguity(NextSolverAmbiguityError<'tcx>), - Overflow(PredicateObligation<'tcx>), } impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for FulfillmentError<'tcx> { @@ -444,9 +398,6 @@ impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for FulfillmentError<'tc NextSolverError::Ambiguity(ambiguity) => { fulfillment_error_for_stalled(infcx, ambiguity) } - NextSolverError::Overflow(obligation) => { - fulfillment_error_for_overflow(infcx, obligation) - } } } } @@ -455,9 +406,7 @@ impl<'tcx> FromSolverError<'tcx, NextSolverError<'tcx>> for ScrubbedTraitError<' fn from_solver_error(_infcx: &InferCtxt<'tcx>, error: NextSolverError<'tcx>) -> Self { match error { NextSolverError::TrueError(_) => ScrubbedTraitError::TrueError, - NextSolverError::Ambiguity(_) | NextSolverError::Overflow(_) => { - ScrubbedTraitError::Ambiguity - } + NextSolverError::Ambiguity(_) => ScrubbedTraitError::Ambiguity, } } } diff --git a/compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs b/compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs index 1c7d5b742e1ac..c44809e217791 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs @@ -161,17 +161,6 @@ pub(super) fn try_ambiguity_error_for_stalled<'tcx>( Some(NextSolverAmbiguityError { root_obligation, code, refine_obligation }) } -pub(super) fn fulfillment_error_for_overflow<'tcx>( - infcx: &InferCtxt<'tcx>, - root_obligation: PredicateObligation<'tcx>, -) -> FulfillmentError<'tcx> { - FulfillmentError { - obligation: find_best_leaf_obligation(infcx, &root_obligation, true), - code: FulfillmentErrorCode::Ambiguity { overflow: Some(true) }, - root_obligation, - } -} - #[instrument(level = "debug", skip(infcx), ret)] fn find_best_leaf_obligation<'tcx>( infcx: &InferCtxt<'tcx>, diff --git a/tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr b/tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr index 1827533a84d90..a93fbc2404dad 100644 --- a/tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr +++ b/tests/ui/traits/next-solver/coherence/coherence-fulfill-overflow.stderr @@ -5,6 +5,9 @@ LL | impl Trait for W {} | ---------------------------- first implementation here LL | impl Trait for T {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `W>>>>>>>>>>>>>>>>>>>>>>` + | + = note: overflow evaluating the requirement `W>>>>>>>>>>>>>>>>>>>>>>: TwoW` + = help: consider increasing the recursion limit by adding a `#![recursion_limit = "20"]` attribute to your crate (`coherence_fulfill_overflow`) error: aborting due to 1 previous error diff --git a/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.rs b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.rs new file mode 100644 index 0000000000000..e8bf7851f4f2e --- /dev/null +++ b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.rs @@ -0,0 +1,53 @@ +//@ compile-flags: -Znext-solver + +// Regression test for trait-system-refactor-initiative#294 +// We used to drop all subsequent obligations when one obligation overflows +// in fulfillment. It means we don't really prove all obligations even if +// fulfillment returns no error. +// +// We now eagerly abort on the first overflowed obligation. + +#![feature(impl_trait_in_assoc_type)] +#![forbid(unsafe_code)] + +trait Amb<'z> {} +trait Sub<'c, 'd>: Amb<'c> + Amb<'d> {} +impl<'z> Amb<'z> for i32 {} +impl<'c, 'd> Sub<'c, 'd> for i32 {} + +trait Call<'a> { + type Output; + fn call() -> Self::Output; +} + +trait Leak { + fn leak(self) -> &'static u8; +} +impl<'z, G: Call<'static, Output = R>, R: Amb<'z>> Leak for &'static u8 { + fn leak(self) -> &'static u8 { + self + } +} + +#[expect(dead_code)] +struct Foo<'c, 'd>(&'c (), &'d ()); + +impl<'a, 'c, 'd> Call<'a> for Foo<'c, 'd> +where + i32: Sub<'c, 'd>, +{ + type Output = impl Sized + use<>; + fn call() -> Self::Output { + let r = { + let local = 42_u8; + <&u8 as Leak>>::leak(&local) + //~^ ERROR: overflow evaluating the requirement `&u8: Leak>` + }; + println!("{r}"); // use-after-free + 1_i32 + } +} + +fn main() { + Foo::call(); +} diff --git a/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.stderr b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.stderr new file mode 100644 index 0000000000000..b7f1881605f86 --- /dev/null +++ b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-1.stderr @@ -0,0 +1,11 @@ +error[E0275]: overflow evaluating the requirement `&u8: Leak>` + --> $DIR/dont-drop-obligations-on-overflow-1.rs:43:13 + | +LL | <&u8 as Leak>>::leak(&local) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`dont_drop_obligations_on_overflow_1`) + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0275`. diff --git a/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.rs b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.rs new file mode 100644 index 0000000000000..63be17cc9a7e3 --- /dev/null +++ b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.rs @@ -0,0 +1,27 @@ +//@ compile-flags: -Znext-solver + +// Regression test for trait-system-refactor-initiative#242 +// We used to drop all subsequent obligations when one obligation overflows +// in fulfillment. It means we don't really prove all obligations even if +// fulfillment returns no error. +// +// We now eagerly abort on the first overflowed obligation. +// +// FIXME: this probably should compile and we shall fix duplicate +// uses of opaques. + +#![feature(type_alias_impl_trait)] +type Tait<'a> = impl Sized; + +fn prove() +where + for<'a> Tait<'a>: Sized, +{} + +#[define_opaque(Tait)] +fn foo<'a>() -> &'a Tait<'a> { + prove(); + //~^ ERROR: overflow evaluating the requirement `for<'a> Tait<'a>: Sized` + &() +} +fn main() {} diff --git a/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.stderr b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.stderr new file mode 100644 index 0000000000000..c8d3b877657f3 --- /dev/null +++ b/tests/ui/traits/next-solver/overflow/dont-drop-obligations-on-overflow-2.stderr @@ -0,0 +1,19 @@ +error[E0275]: overflow evaluating the requirement `for<'a> Tait<'a>: Sized` + --> $DIR/dont-drop-obligations-on-overflow-2.rs:23:5 + | +LL | prove(); + | ^^^^^^^ + | + = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`dont_drop_obligations_on_overflow_2`) +note: required by a bound in `prove` + --> $DIR/dont-drop-obligations-on-overflow-2.rs:18:23 + | +LL | fn prove() + | ----- required by a bound in this function +LL | where +LL | for<'a> Tait<'a>: Sized, + | ^^^^^ required by this bound in `prove` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0275`.