From b64b25b99e738f4c68a83f987b203979eaad4fbf Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 21 Oct 2024 16:25:42 +0200 Subject: [PATCH 1/2] normalizes-to disable infer var check --- .../src/solve/eval_ctxt/canonical.rs | 23 +++++--- .../src/traits/coherence.rs | 2 + .../normalize-allow-too-many-vars.rs | 52 +++++++++++++++++++ .../overflow/coherence-alias-hang.rs | 11 ++-- 4 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 tests/ui/traits/next-solver/normalize/normalize-allow-too-many-vars.rs diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs index f49f3a1a3bf..bc691a8aa2b 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs @@ -16,7 +16,7 @@ use rustc_type_ir::inherent::*; use rustc_type_ir::relate::solver_relating::RelateExt; use rustc_type_ir::{self as ty, Canonical, CanonicalVarValues, InferCtxtLike, Interner}; -use tracing::{instrument, trace}; +use tracing::{debug, instrument, trace}; use crate::canonicalizer::{CanonicalizeMode, Canonicalizer}; use crate::delegate::SolverDelegate; @@ -165,12 +165,21 @@ pub(in crate::solve) fn evaluate_added_goals_and_make_canonical_response( // HACK: We bail with overflow if the response would have too many non-region // inference variables. This tends to only happen if we encounter a lot of // ambiguous alias types which get replaced with fresh inference variables - // during generalization. This prevents a hang in nalgebra. - let num_non_region_vars = canonical.variables.iter().filter(|c| !c.is_region()).count(); - if num_non_region_vars > self.cx().recursion_limit() { - return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow { - suggest_increasing_limit: true, - })); + // during generalization. This prevents hangs caused by an exponential blowup, + // see tests/ui/traits/next-solver/coherence-alias-hang.rs. + // + // We don't do so for `NormalizesTo` goals as we erased the expected term and + // bailing with overflow here would prevent us from detecting a type-mismatch, + // causing a coherence error in diesel, see #131969. We still bail with verflow + // when later returning from the parent AliasRelate goal. + if !self.is_normalizes_to_goal { + let num_non_region_vars = canonical.variables.iter().filter(|c| !c.is_region()).count(); + if num_non_region_vars > self.cx().recursion_limit() { + debug!(?num_non_region_vars, "too many inference variables -> overflow"); + return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow { + suggest_increasing_limit: true, + })); + } } Ok(canonical) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index f4a2483cebf..f8fb297e36c 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -298,6 +298,7 @@ fn equate_impl_headers<'tcx>( } /// The result of [fn impl_intersection_has_impossible_obligation]. +#[derive(Debug)] enum IntersectionHasImpossibleObligations<'tcx> { Yes, No { @@ -328,6 +329,7 @@ enum IntersectionHasImpossibleObligations<'tcx> { /// of the two impls above to be empty. /// /// Importantly, this works even if there isn't a `impl !Error for MyLocalType`. +#[instrument(level = "debug", skip(selcx), ret)] fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>( selcx: &mut SelectionContext<'cx, 'tcx>, obligations: &'a [PredicateObligation<'tcx>], diff --git a/tests/ui/traits/next-solver/normalize/normalize-allow-too-many-vars.rs b/tests/ui/traits/next-solver/normalize/normalize-allow-too-many-vars.rs new file mode 100644 index 00000000000..5284220ac38 --- /dev/null +++ b/tests/ui/traits/next-solver/normalize/normalize-allow-too-many-vars.rs @@ -0,0 +1,52 @@ +//@ check-pass + +// When canonicalizing a response in the trait solver, we bail with overflow +// if there are too many non-region inference variables. Doing so in normalizes-to +// goals ends up hiding inference constraints in cases which we want to support, +// see #131969. To prevent this issue we do not check for too many inference +// variables in normalizes-to goals. +#![recursion_limit = "8"] + +trait Bound {} +trait Trait { + type Assoc; +} + + +impl Trait for (T0, T1, T2, T3, T4, T5, T6, T7) +where + T0: Trait, + T1: Trait, + T2: Trait, + T3: Trait, + T4: Trait, + T5: Trait, + T6: Trait, + T7: Trait, + ( + T0::Assoc, + T1::Assoc, + T2::Assoc, + T3::Assoc, + T4::Assoc, + T5::Assoc, + T6::Assoc, + T7::Assoc, + ): Clone, +{ + type Assoc = ( + T0::Assoc, + T1::Assoc, + T2::Assoc, + T3::Assoc, + T4::Assoc, + T5::Assoc, + T6::Assoc, + T7::Assoc, + ); +} + +trait Overlap {} +impl> Overlap for T {} +impl Overlap for (T0, T1, T2, T3, T4, T5, T6, T7) {} +fn main() {} diff --git a/tests/ui/traits/next-solver/overflow/coherence-alias-hang.rs b/tests/ui/traits/next-solver/overflow/coherence-alias-hang.rs index 0d5f42231e4..f88f74680b9 100644 --- a/tests/ui/traits/next-solver/overflow/coherence-alias-hang.rs +++ b/tests/ui/traits/next-solver/overflow/coherence-alias-hang.rs @@ -1,8 +1,5 @@ //@ check-pass -//@ revisions: ai_current ai_next ia_current ia_next ii_current ii_next -//@[ai_next] compile-flags: -Znext-solver -//@[ia_next] compile-flags: -Znext-solver -//@[ii_next] compile-flags: -Znext-solver +//@ revisions: ai ia ii // Regression test for nalgebra hang . @@ -17,11 +14,11 @@ trait Trait { type Assoc: ?Sized; } impl Trait for W { - #[cfg(any(ai_current, ai_next))] + #[cfg(ai)] type Assoc = W>; - #[cfg(any(ia_current, ia_next))] + #[cfg(ia)] type Assoc = W, T::Assoc>; - #[cfg(any(ii_current, ii_next))] + #[cfg(ii)] type Assoc = W, Id>; } From 919b61a6f4aa2e0002c01037d923c68c57f47f33 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 21 Oct 2024 17:51:43 +0200 Subject: [PATCH 2/2] don't bail when encountering many placeholders --- .../src/solve/eval_ctxt/canonical.rs | 3 ++- .../canonical/do-not-bail-due-to-placeholders.rs | 16 ++++++++++++++++ .../recursion-limit-zero-issue-115351.rs | 6 +++--- .../recursion-limit-zero-issue-115351.stderr | 11 ----------- 4 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 tests/ui/traits/next-solver/canonical/do-not-bail-due-to-placeholders.rs delete mode 100644 tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.stderr diff --git a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs index bc691a8aa2b..71ce0cce772 100644 --- a/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_next_trait_solver/src/solve/eval_ctxt/canonical.rs @@ -173,7 +173,8 @@ pub(in crate::solve) fn evaluate_added_goals_and_make_canonical_response( // causing a coherence error in diesel, see #131969. We still bail with verflow // when later returning from the parent AliasRelate goal. if !self.is_normalizes_to_goal { - let num_non_region_vars = canonical.variables.iter().filter(|c| !c.is_region()).count(); + let num_non_region_vars = + canonical.variables.iter().filter(|c| !c.is_region() && c.is_existential()).count(); if num_non_region_vars > self.cx().recursion_limit() { debug!(?num_non_region_vars, "too many inference variables -> overflow"); return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow { diff --git a/tests/ui/traits/next-solver/canonical/do-not-bail-due-to-placeholders.rs b/tests/ui/traits/next-solver/canonical/do-not-bail-due-to-placeholders.rs new file mode 100644 index 00000000000..3fc11f27d1f --- /dev/null +++ b/tests/ui/traits/next-solver/canonical/do-not-bail-due-to-placeholders.rs @@ -0,0 +1,16 @@ +//@ compile-flags: -Znext-solver +//@ check-pass + +// When canonicalizing responses, we bail if there are too many inference variables. +// We previously also counted placeholders, which is incorrect. +#![recursion_limit = "8"] + +fn foo() {} + +fn bar() { + // The query response will contain 10 placeholders, which previously + // caused us to bail here. + foo::<(T0, T1, T2, T3, T4, T5, T6, T7, T8, T9)>(); +} + +fn main() {} diff --git a/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.rs b/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.rs index 86d428cc0f0..dd8ad3c2dfe 100644 --- a/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.rs +++ b/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.rs @@ -1,8 +1,8 @@ -//~ ERROR overflow evaluating the requirement `Self: Trait` -//~^ ERROR overflow evaluating the requirement `Self well-formed` -// This is a non-regression test for issue #115351, where a recursion limit of 0 caused an ICE. +//@ check-pass //@ compile-flags: -Znext-solver --crate-type=lib +// This is a non-regression test for issue #115351, where a recursion limit of 0 caused an ICE. + #![recursion_limit = "0"] trait Trait {} impl Trait for u32 {} diff --git a/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.stderr b/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.stderr deleted file mode 100644 index 2eed7e8f723..00000000000 --- a/tests/ui/traits/next-solver/overflow/recursion-limit-zero-issue-115351.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0275]: overflow evaluating the requirement `Self: Trait` - | - = help: consider increasing the recursion limit by adding a `#![recursion_limit = "2"]` attribute to your crate (`recursion_limit_zero_issue_115351`) - -error[E0275]: overflow evaluating the requirement `Self well-formed` - | - = help: consider increasing the recursion limit by adding a `#![recursion_limit = "2"]` attribute to your crate (`recursion_limit_zero_issue_115351`) - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0275`.