Skip to content

Commit

Permalink
Auto merge of rust-lang#136824 - lcnr:yeet, r=<try>
Browse files Browse the repository at this point in the history
solver cycles are coinductive once they have one coinductive step

Implements the new cycle semantics in the new solver, dealing with the fallout from rust-lang/trait-system-refactor-initiative#10.

I am currently also changing inductive cycles back to an error instead of ambiguity outside of coherence to deal with rust-lang/trait-system-refactor-initiative#114. This should allow nalgebra to compile without affecting anything on stable. Whether a cycle results in ambiguity or success should not matter for coherence, as it only checks for errors.

The first commit has been extensively fuzzed via https://github.com/lcnr/search_graph_fuzz.

TODO:
- [ ] fix issues from https://hackmd.io/JsblAvk4R5y30niSNQVYeA
- [ ] add ui tests
- [ ] explain this approach and why we believe it to be correct

r? `@compiler-errors` `@nikomatsakis`
  • Loading branch information
bors committed Feb 13, 2025
2 parents cfe9ffc + 68f2802 commit d5c3c24
Show file tree
Hide file tree
Showing 51 changed files with 911 additions and 835 deletions.
51 changes: 23 additions & 28 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use derive_where::derive_where;
use rustc_type_ir::fold::TypeFoldable;
use rustc_type_ir::inherent::*;
use rustc_type_ir::lang_items::TraitSolverLangItem;
use rustc_type_ir::solve::inspect;
use rustc_type_ir::visit::TypeVisitableExt as _;
use rustc_type_ir::{self as ty, Interner, TypingMode, Upcast as _, elaborate};
use tracing::{debug, instrument};
Expand Down Expand Up @@ -292,25 +291,6 @@ where
let Ok(normalized_self_ty) =
self.structurally_normalize_ty(goal.param_env, goal.predicate.self_ty())
else {
// FIXME: We register a fake candidate when normalization fails so that
// we can point at the reason for *why*. I'm tempted to say that this
// is the wrong way to do this, though.
let result =
self.probe(|&result| inspect::ProbeKind::RigidAlias { result }).enter(|this| {
let normalized_ty = this.next_ty_infer();
let alias_relate_goal = Goal::new(
this.cx(),
goal.param_env,
ty::PredicateKind::AliasRelate(
goal.predicate.self_ty().into(),
normalized_ty.into(),
ty::AliasRelationDirection::Equate,
),
);
this.add_goal(GoalSource::AliasWellFormed, alias_relate_goal);
this.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
});
assert_eq!(result, Err(NoSolution));
return vec![];
};

Expand Down Expand Up @@ -789,11 +769,12 @@ where
/// treat the alias as rigid.
///
/// See trait-system-refactor-initiative#124 for more details.
#[instrument(level = "debug", skip(self), ret)]
#[instrument(level = "debug", skip(self, inject_normalize_to_rigid_candidate), ret)]
pub(super) fn merge_candidates(
&mut self,
proven_via: Option<TraitGoalProvenVia>,
candidates: Vec<Candidate<I>>,
inject_normalize_to_rigid_candidate: impl FnOnce(&mut EvalCtxt<'_, D>) -> QueryResult<I>,
) -> QueryResult<I> {
let Some(proven_via) = proven_via else {
// We don't care about overflow. If proving the trait goal overflowed, then
Expand All @@ -810,13 +791,27 @@ where
// FIXME(const_trait_impl): should this behavior also be used by
// constness checking. Doing so is *at least theoretically* breaking,
// see github.com/rust-lang/rust/issues/133044#issuecomment-2500709754
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => candidates
.iter()
.filter(|c| {
matches!(c.source, CandidateSource::AliasBound | CandidateSource::ParamEnv(_))
})
.map(|c| c.result)
.collect(),
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
let mut candidates_from_env: Vec<_> = candidates
.iter()
.filter(|c| {
matches!(
c.source,
CandidateSource::AliasBound | CandidateSource::ParamEnv(_)
)
})
.map(|c| c.result)
.collect();

// If the trait goal has been proven by using the environment, we want to treat
// aliases as rigid if there are no applicable projection bounds in the environment.
if candidates_from_env.is_empty() {
if let Ok(response) = inject_normalize_to_rigid_candidate(self) {
candidates_from_env.push(response);
}
}
candidates_from_env
}
TraitGoalProvenVia::Misc => candidates.iter().map(|c| c.result).collect(),
};

Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_next_trait_solver/src/solve/effect_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,27 @@ where
let impl_trait_ref = impl_trait_ref.instantiate(cx, impl_args);

ecx.eq(goal.param_env, goal.predicate.trait_ref, impl_trait_ref)?;
// Resolve inference variables here to improve the debug output :)
let impl_trait_ref = ecx.resolve_vars_if_possible(impl_trait_ref);

let where_clause_bounds = cx
.predicates_of(impl_def_id)
.iter_instantiated(cx, impl_args)
.map(|pred| goal.with(cx, pred));
ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds);

// When using an impl, we have to check that its super trait bounds are actually satisfied.
// This is necessary as otherwise the impl `impl<T: Magic> Magic for T` would allow us to
// incorrectly assume all super traits of `Magic`.
for clause in elaborate::elaborate(
cx,
cx.explicit_super_predicates_of(impl_trait_ref.def_id)
.iter_instantiated(cx, impl_trait_ref.args)
.map(|(pred, _)| pred),
) {
ecx.add_goal(GoalSource::Misc, goal.with(cx, clause));
}

// For this impl to be `const`, we need to check its `~const` bounds too.
let const_conditions = cx
.const_conditions(impl_def_id)
Expand Down Expand Up @@ -398,6 +413,6 @@ where
goal.with(ecx.cx(), goal.predicate.trait_ref);
ecx.compute_trait_goal(trait_goal)
})?;
self.merge_candidates(proven_via, candidates)
self.merge_candidates(proven_via, candidates, |_ecx| Err(NoSolution))
}
}
27 changes: 24 additions & 3 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_type_ir::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
use rustc_type_ir::inherent::*;
use rustc_type_ir::relate::Relate;
use rustc_type_ir::relate::solver_relating::RelateExt;
use rustc_type_ir::search_graph::PathKind;
use rustc_type_ir::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor};
use rustc_type_ir::{self as ty, CanonicalVarValues, InferCtxtLike, Interner, TypingMode};
use rustc_type_ir_macros::{Lift_Generic, TypeFoldable_Generic, TypeVisitable_Generic};
Expand Down Expand Up @@ -59,6 +60,12 @@ where
/// when then adds these to its own context. The caller is always an `AliasRelate`
/// goal so this never leaks out of the solver.
is_normalizes_to_goal: bool,

/// Whether the current `probe` should be treated as a coinductive step when encountering
/// trait solver cycles. This changes the step kind of all nested goals computed in that
/// probe to be coinductive.
is_coinductive_probe: bool,

pub(super) var_values: CanonicalVarValues<I>,

predefined_opaques_in_body: I::PredefinedOpaques,
Expand Down Expand Up @@ -230,6 +237,17 @@ where
self.is_normalizes_to_goal = true;
}

pub(super) fn step_kind_for_source(&self, source: GoalSource) -> PathKind {
if self.is_coinductive_probe {
return PathKind::Coinductive;
}

match source {
GoalSource::ImplWhereBound => PathKind::Coinductive,
_ => PathKind::Inductive,
}
}

/// Creates a root evaluation context and search graph. This should only be
/// used from outside of any evaluation, and other methods should be preferred
/// over using this manually (such as [`SolverDelegateEvalExt::evaluate_root_goal`]).
Expand Down Expand Up @@ -257,6 +275,7 @@ where
variables: Default::default(),
var_values: CanonicalVarValues::dummy(),
is_normalizes_to_goal: false,
is_coinductive_probe: false,
origin_span,
tainted: Ok(()),
};
Expand Down Expand Up @@ -295,6 +314,7 @@ where
variables: canonical_input.canonical.variables,
var_values,
is_normalizes_to_goal: false,
is_coinductive_probe: false,
predefined_opaques_in_body: input.predefined_opaques_in_body,
max_input_universe: canonical_input.canonical.max_universe,
search_graph,
Expand Down Expand Up @@ -340,6 +360,7 @@ where
cx: I,
search_graph: &'a mut SearchGraph<D>,
canonical_input: CanonicalInput<I>,
step_kind_from_parent: PathKind,
goal_evaluation: &mut ProofTreeBuilder<D>,
) -> QueryResult<I> {
let mut canonical_goal_evaluation =
Expand All @@ -352,6 +373,7 @@ where
search_graph.with_new_goal(
cx,
canonical_input,
step_kind_from_parent,
&mut canonical_goal_evaluation,
|search_graph, canonical_goal_evaluation| {
EvalCtxt::enter_canonical(
Expand Down Expand Up @@ -395,12 +417,10 @@ where
/// `NormalizesTo` is only used by `AliasRelate`, all other callsites
/// should use [`EvalCtxt::evaluate_goal`] which discards that empty
/// storage.
// FIXME(-Znext-solver=coinduction): `_source` is currently unused but will
// be necessary once we implement the new coinduction approach.
pub(super) fn evaluate_goal_raw(
&mut self,
goal_evaluation_kind: GoalEvaluationKind,
_source: GoalSource,
source: GoalSource,
goal: Goal<I, I::Predicate>,
) -> Result<(NestedNormalizationGoals<I>, HasChanged, Certainty), NoSolution> {
let (orig_values, canonical_goal) = self.canonicalize_goal(goal);
Expand All @@ -410,6 +430,7 @@ where
self.cx(),
self.search_graph,
canonical_goal,
self.step_kind_for_source(source),
&mut goal_evaluation,
);
let response = match canonical_response {
Expand Down
36 changes: 36 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/eval_ctxt/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,42 @@ where
variables: outer_ecx.variables,
var_values: outer_ecx.var_values,
is_normalizes_to_goal: outer_ecx.is_normalizes_to_goal,
is_coinductive_probe: outer_ecx.is_coinductive_probe,
predefined_opaques_in_body: outer_ecx.predefined_opaques_in_body,
max_input_universe,
search_graph: outer_ecx.search_graph,
nested_goals: outer_ecx.nested_goals.clone(),
origin_span: outer_ecx.origin_span,
tainted: outer_ecx.tainted,
inspect: outer_ecx.inspect.take_and_enter_probe(),
};
let r = nested_ecx.delegate.probe(|| {
let r = f(&mut nested_ecx);
nested_ecx.inspect.probe_final_state(delegate, max_input_universe);
r
});
if !nested_ecx.inspect.is_noop() {
let probe_kind = probe_kind(&r);
nested_ecx.inspect.probe_kind(probe_kind);
outer_ecx.inspect = nested_ecx.inspect.finish_probe();
}
r
}

pub(in crate::solve) fn enter_coinductively(
self,
f: impl FnOnce(&mut EvalCtxt<'_, D>) -> T,
) -> T {
let ProbeCtxt { ecx: outer_ecx, probe_kind, _result } = self;

let delegate = outer_ecx.delegate;
let max_input_universe = outer_ecx.max_input_universe;
let mut nested_ecx = EvalCtxt {
delegate,
variables: outer_ecx.variables,
var_values: outer_ecx.var_values,
is_normalizes_to_goal: outer_ecx.is_normalizes_to_goal,
is_coinductive_probe: true,
predefined_opaques_in_body: outer_ecx.predefined_opaques_in_body,
max_input_universe,
search_graph: outer_ecx.search_graph,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ where
//
// FIXME(-Znext-solver=coinductive): I think this should be split
// and we tag the impl bounds with `GoalSource::ImplWhereBound`?
// Right not this includes both the impl and the assoc item where bounds,
// Right now this includes both the impl and the assoc item where bounds,
// and I don't think the assoc item where-bounds are allowed to be coinductive.
self.add_goals(
GoalSource::Misc,
Expand Down
Loading

0 comments on commit d5c3c24

Please sign in to comment.