From 4a38442c900bd859ecece400454d5abc56647b64 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 18 Dec 2023 08:31:00 +0100 Subject: [PATCH] dont discard overflow from normalizes-to goals --- compiler/rustc_middle/src/traits/solve.rs | 3 ++ .../src/solve/eval_ctxt/canonical.rs | 14 ------ .../src/solve/eval_ctxt/mod.rs | 43 +++++++++++++++---- .../src/solve/search_graph.rs | 10 +++++ .../traits/issue-90662-projection-caching.rs | 10 ++++- ...cursion-limit-normalizes-to-constraints.rs | 25 +++++++++++ 6 files changed, 82 insertions(+), 23 deletions(-) create mode 100644 tests/ui/traits/next-solver/overflow/recursion-limit-normalizes-to-constraints.rs diff --git a/compiler/rustc_middle/src/traits/solve.rs b/compiler/rustc_middle/src/traits/solve.rs index dd5b57eed46..048df367bd6 100644 --- a/compiler/rustc_middle/src/traits/solve.rs +++ b/compiler/rustc_middle/src/traits/solve.rs @@ -248,6 +248,9 @@ pub enum GoalSource { /// This also impacts whether we erase constraints on overflow. /// Erasing constraints is generally very useful for perf and also /// results in better error messages by avoiding spurious errors. + /// We do not erase overflow constraints in `normalizes-to` goals unless + /// they are from an impl where-clause. This is necessary due to + /// backwards compatability, cc trait-system-refactor-initiatitive#70. ImplWhereBound, } diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs index 7457ba837f5..ecdae2521b9 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs @@ -94,20 +94,6 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { ); let certainty = certainty.unify_with(goals_certainty); - if let Certainty::OVERFLOW = 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 - // query response. - // - // This may prevent us from potentially useful inference, e.g. - // 2 candidates, one ambiguous and one overflow, which both - // have the same inference constraints. - // - // Changing this to retain some constraints in the future - // won't be a breaking change, so this is good enough for now. - return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Overflow)); - } let var_values = self.var_values; let external_constraints = self.compute_external_query_constraints()?; diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs index 7f5e5654657..ee0fc8139a2 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs @@ -157,7 +157,7 @@ impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> { Option>, ) { EvalCtxt::enter_root(self, generate_proof_tree, |ecx| { - ecx.evaluate_goal(GoalEvaluationKind::Root, goal) + ecx.evaluate_goal(GoalEvaluationKind::Root, GoalSource::Misc, goal) }) } } @@ -335,6 +335,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { fn evaluate_goal( &mut self, goal_evaluation_kind: GoalEvaluationKind, + source: GoalSource, goal: Goal<'tcx, ty::Predicate<'tcx>>, ) -> Result<(bool, Certainty, Vec>>), NoSolution> { let (orig_values, canonical_goal) = self.canonicalize_goal(goal); @@ -354,13 +355,13 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { Ok(response) => response, }; - let has_changed = !canonical_response.value.var_values.is_identity_modulo_regions() - || !canonical_response.value.external_constraints.opaque_types.is_empty(); - let (certainty, nested_goals) = match self.instantiate_and_apply_query_response( - goal.param_env, - orig_values, - canonical_response, - ) { + let (certainty, has_changed, nested_goals) = match self + .instantiate_response_discarding_overflow( + goal.param_env, + source, + orig_values, + canonical_response, + ) { Err(e) => { self.inspect.goal_evaluation(goal_evaluation); return Err(e); @@ -387,6 +388,30 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { Ok((has_changed, certainty, nested_goals)) } + fn instantiate_response_discarding_overflow( + &mut self, + param_env: ty::ParamEnv<'tcx>, + source: GoalSource, + original_values: Vec>, + response: CanonicalResponse<'tcx>, + ) -> Result<(Certainty, bool, Vec>>), NoSolution> { + let keep_overflow_constraints = || { + self.search_graph.current_goal_is_normalizes_to() + && source != GoalSource::ImplWhereBound + }; + + if response.value.certainty == Certainty::OVERFLOW && !keep_overflow_constraints() { + Ok((Certainty::OVERFLOW, false, Vec::new())) + } else { + let has_changed = !response.value.var_values.is_identity_modulo_regions() + || !response.value.external_constraints.opaque_types.is_empty(); + + let (certainty, nested_goals) = + self.instantiate_and_apply_query_response(param_env, original_values, response)?; + Ok((certainty, has_changed, nested_goals)) + } + } + fn compute_goal(&mut self, goal: Goal<'tcx, ty::Predicate<'tcx>>) -> QueryResult<'tcx> { let Goal { param_env, predicate } = goal; let kind = predicate.kind(); @@ -509,6 +534,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { let (_, certainty, instantiate_goals) = self.evaluate_goal( GoalEvaluationKind::Nested { is_normalizes_to_hack: IsNormalizesToHack::Yes }, + GoalSource::Misc, unconstrained_goal, )?; self.nested_goals.goals.extend(with_misc_source(instantiate_goals)); @@ -544,6 +570,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { for (source, goal) in goals.goals.drain(..) { let (has_changed, certainty, instantiate_goals) = self.evaluate_goal( GoalEvaluationKind::Nested { is_normalizes_to_hack: IsNormalizesToHack::No }, + source, goal, )?; self.nested_goals.goals.extend(with_misc_source(instantiate_goals)); diff --git a/compiler/rustc_trait_selection/src/solve/search_graph.rs b/compiler/rustc_trait_selection/src/solve/search_graph.rs index 2a08b80e02a..2a161c2d956 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph.rs @@ -8,6 +8,7 @@ use rustc_index::IndexVec; use rustc_middle::dep_graph::dep_kinds; use rustc_middle::traits::solve::CacheData; use rustc_middle::traits::solve::{CanonicalInput, Certainty, EvaluationCache, QueryResult}; +use rustc_middle::ty; use rustc_middle::ty::TyCtxt; use rustc_session::Limit; use std::collections::hash_map::Entry; @@ -111,6 +112,15 @@ impl<'tcx> SearchGraph<'tcx> { self.stack.is_empty() } + pub(super) fn current_goal_is_normalizes_to(&self) -> bool { + self.stack.raw.last().map_or(false, |e| { + matches!( + e.input.value.goal.predicate.kind().skip_binder(), + ty::PredicateKind::NormalizesTo(..) + ) + }) + } + /// Returns the remaining depth allowed for nested goals. /// /// This is generally simply one less than the current depth. diff --git a/tests/ui/traits/issue-90662-projection-caching.rs b/tests/ui/traits/issue-90662-projection-caching.rs index 879f30071bf..e08ab53fbb0 100644 --- a/tests/ui/traits/issue-90662-projection-caching.rs +++ b/tests/ui/traits/issue-90662-projection-caching.rs @@ -1,7 +1,15 @@ +// revisions: old next +//[next] compile-flags: -Znext-solver=coherence // check-pass // Regression test for issue #90662 -// Tests that projection caching does not cause a spurious error +// Tests that projection caching does not cause a spurious error. +// Coherence relies on the following overflowing goal to still constrain +// `?0` to `dyn Service`. +// +// Projection(>::Interface. ?0) +// +// cc https://github.com/rust-lang/trait-system-refactor-initiative/issues/70. trait HasProvider {} trait Provider { diff --git a/tests/ui/traits/next-solver/overflow/recursion-limit-normalizes-to-constraints.rs b/tests/ui/traits/next-solver/overflow/recursion-limit-normalizes-to-constraints.rs new file mode 100644 index 00000000000..03ef93dc233 --- /dev/null +++ b/tests/ui/traits/next-solver/overflow/recursion-limit-normalizes-to-constraints.rs @@ -0,0 +1,25 @@ +// compile-flags: -Znext-solver=coherence +// check-pass + +// A regression test for trait-system-refactor-initiative#70. + +trait Trait { + type Assoc; +} + +struct W(*mut T); +impl Trait for W> +where + W: Trait, +{ + type Assoc = (); +} + +trait NoOverlap {} +impl> NoOverlap for T {} +// `Projection( as Trait>::Assoc, u32)` should result in error even +// though applying the impl results in overflow. This is necessary to match +// the behavior of the old solver. +impl NoOverlap for W {} + +fn main() {}