Skip to content

Commit

Permalink
dont discard overflow from normalizes-to goals
Browse files Browse the repository at this point in the history
  • Loading branch information
lcnr committed Dec 18, 2023
1 parent ca718ff commit 4a38442
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 23 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/traits/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
14 changes: 0 additions & 14 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
43 changes: 35 additions & 8 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> {
Option<inspect::GoalEvaluation<'tcx>>,
) {
EvalCtxt::enter_root(self, generate_proof_tree, |ecx| {
ecx.evaluate_goal(GoalEvaluationKind::Root, goal)
ecx.evaluate_goal(GoalEvaluationKind::Root, GoalSource::Misc, goal)
})
}
}
Expand Down Expand Up @@ -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<Goal<'tcx, ty::Predicate<'tcx>>>), NoSolution> {
let (orig_values, canonical_goal) = self.canonicalize_goal(goal);
Expand All @@ -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);
Expand All @@ -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<ty::GenericArg<'tcx>>,
response: CanonicalResponse<'tcx>,
) -> Result<(Certainty, bool, Vec<Goal<'tcx, ty::Predicate<'tcx>>>), 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();
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_trait_selection/src/solve/search_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 9 additions & 1 deletion tests/ui/traits/issue-90662-projection-caching.rs
Original file line number Diff line number Diff line change
@@ -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(<ServiceImpl as Provider<TestModule>>::Interface. ?0)
//
// cc https://github.com/rust-lang/trait-system-refactor-initiative/issues/70.

trait HasProvider<T: ?Sized> {}
trait Provider<M> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T: ?Sized>(*mut T);
impl<T: ?Sized> Trait for W<W<T>>
where
W<T>: Trait,
{
type Assoc = ();
}

trait NoOverlap {}
impl<T: Trait<Assoc = u32>> NoOverlap for T {}
// `Projection(<W<_> 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<T: ?Sized> NoOverlap for W<T> {}

fn main() {}

0 comments on commit 4a38442

Please sign in to comment.