Skip to content

Commit d5c3c24

Browse files
committed
Auto merge of rust-lang#136824 - lcnr:yeet, r=<try>
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`
2 parents cfe9ffc + 68f2802 commit d5c3c24

File tree

51 files changed

+911
-835
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+911
-835
lines changed

compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

+23-28
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use derive_where::derive_where;
66
use rustc_type_ir::fold::TypeFoldable;
77
use rustc_type_ir::inherent::*;
88
use rustc_type_ir::lang_items::TraitSolverLangItem;
9-
use rustc_type_ir::solve::inspect;
109
use rustc_type_ir::visit::TypeVisitableExt as _;
1110
use rustc_type_ir::{self as ty, Interner, TypingMode, Upcast as _, elaborate};
1211
use tracing::{debug, instrument};
@@ -292,25 +291,6 @@ where
292291
let Ok(normalized_self_ty) =
293292
self.structurally_normalize_ty(goal.param_env, goal.predicate.self_ty())
294293
else {
295-
// FIXME: We register a fake candidate when normalization fails so that
296-
// we can point at the reason for *why*. I'm tempted to say that this
297-
// is the wrong way to do this, though.
298-
let result =
299-
self.probe(|&result| inspect::ProbeKind::RigidAlias { result }).enter(|this| {
300-
let normalized_ty = this.next_ty_infer();
301-
let alias_relate_goal = Goal::new(
302-
this.cx(),
303-
goal.param_env,
304-
ty::PredicateKind::AliasRelate(
305-
goal.predicate.self_ty().into(),
306-
normalized_ty.into(),
307-
ty::AliasRelationDirection::Equate,
308-
),
309-
);
310-
this.add_goal(GoalSource::AliasWellFormed, alias_relate_goal);
311-
this.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
312-
});
313-
assert_eq!(result, Err(NoSolution));
314294
return vec![];
315295
};
316296

@@ -789,11 +769,12 @@ where
789769
/// treat the alias as rigid.
790770
///
791771
/// See trait-system-refactor-initiative#124 for more details.
792-
#[instrument(level = "debug", skip(self), ret)]
772+
#[instrument(level = "debug", skip(self, inject_normalize_to_rigid_candidate), ret)]
793773
pub(super) fn merge_candidates(
794774
&mut self,
795775
proven_via: Option<TraitGoalProvenVia>,
796776
candidates: Vec<Candidate<I>>,
777+
inject_normalize_to_rigid_candidate: impl FnOnce(&mut EvalCtxt<'_, D>) -> QueryResult<I>,
797778
) -> QueryResult<I> {
798779
let Some(proven_via) = proven_via else {
799780
// We don't care about overflow. If proving the trait goal overflowed, then
@@ -810,13 +791,27 @@ where
810791
// FIXME(const_trait_impl): should this behavior also be used by
811792
// constness checking. Doing so is *at least theoretically* breaking,
812793
// see github.com/rust-lang/rust/issues/133044#issuecomment-2500709754
813-
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => candidates
814-
.iter()
815-
.filter(|c| {
816-
matches!(c.source, CandidateSource::AliasBound | CandidateSource::ParamEnv(_))
817-
})
818-
.map(|c| c.result)
819-
.collect(),
794+
TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
795+
let mut candidates_from_env: Vec<_> = candidates
796+
.iter()
797+
.filter(|c| {
798+
matches!(
799+
c.source,
800+
CandidateSource::AliasBound | CandidateSource::ParamEnv(_)
801+
)
802+
})
803+
.map(|c| c.result)
804+
.collect();
805+
806+
// If the trait goal has been proven by using the environment, we want to treat
807+
// aliases as rigid if there are no applicable projection bounds in the environment.
808+
if candidates_from_env.is_empty() {
809+
if let Ok(response) = inject_normalize_to_rigid_candidate(self) {
810+
candidates_from_env.push(response);
811+
}
812+
}
813+
candidates_from_env
814+
}
820815
TraitGoalProvenVia::Misc => candidates.iter().map(|c| c.result).collect(),
821816
};
822817

compiler/rustc_next_trait_solver/src/solve/effect_goals.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -154,12 +154,27 @@ where
154154
let impl_trait_ref = impl_trait_ref.instantiate(cx, impl_args);
155155

156156
ecx.eq(goal.param_env, goal.predicate.trait_ref, impl_trait_ref)?;
157+
// Resolve inference variables here to improve the debug output :)
158+
let impl_trait_ref = ecx.resolve_vars_if_possible(impl_trait_ref);
159+
157160
let where_clause_bounds = cx
158161
.predicates_of(impl_def_id)
159162
.iter_instantiated(cx, impl_args)
160163
.map(|pred| goal.with(cx, pred));
161164
ecx.add_goals(GoalSource::ImplWhereBound, where_clause_bounds);
162165

166+
// When using an impl, we have to check that its super trait bounds are actually satisfied.
167+
// This is necessary as otherwise the impl `impl<T: Magic> Magic for T` would allow us to
168+
// incorrectly assume all super traits of `Magic`.
169+
for clause in elaborate::elaborate(
170+
cx,
171+
cx.explicit_super_predicates_of(impl_trait_ref.def_id)
172+
.iter_instantiated(cx, impl_trait_ref.args)
173+
.map(|(pred, _)| pred),
174+
) {
175+
ecx.add_goal(GoalSource::Misc, goal.with(cx, clause));
176+
}
177+
163178
// For this impl to be `const`, we need to check its `~const` bounds too.
164179
let const_conditions = cx
165180
.const_conditions(impl_def_id)
@@ -398,6 +413,6 @@ where
398413
goal.with(ecx.cx(), goal.predicate.trait_ref);
399414
ecx.compute_trait_goal(trait_goal)
400415
})?;
401-
self.merge_candidates(proven_via, candidates)
416+
self.merge_candidates(proven_via, candidates, |_ecx| Err(NoSolution))
402417
}
403418
}

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

+24-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use rustc_type_ir::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
99
use rustc_type_ir::inherent::*;
1010
use rustc_type_ir::relate::Relate;
1111
use rustc_type_ir::relate::solver_relating::RelateExt;
12+
use rustc_type_ir::search_graph::PathKind;
1213
use rustc_type_ir::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor};
1314
use rustc_type_ir::{self as ty, CanonicalVarValues, InferCtxtLike, Interner, TypingMode};
1415
use rustc_type_ir_macros::{Lift_Generic, TypeFoldable_Generic, TypeVisitable_Generic};
@@ -59,6 +60,12 @@ where
5960
/// when then adds these to its own context. The caller is always an `AliasRelate`
6061
/// goal so this never leaks out of the solver.
6162
is_normalizes_to_goal: bool,
63+
64+
/// Whether the current `probe` should be treated as a coinductive step when encountering
65+
/// trait solver cycles. This changes the step kind of all nested goals computed in that
66+
/// probe to be coinductive.
67+
is_coinductive_probe: bool,
68+
6269
pub(super) var_values: CanonicalVarValues<I>,
6370

6471
predefined_opaques_in_body: I::PredefinedOpaques,
@@ -230,6 +237,17 @@ where
230237
self.is_normalizes_to_goal = true;
231238
}
232239

240+
pub(super) fn step_kind_for_source(&self, source: GoalSource) -> PathKind {
241+
if self.is_coinductive_probe {
242+
return PathKind::Coinductive;
243+
}
244+
245+
match source {
246+
GoalSource::ImplWhereBound => PathKind::Coinductive,
247+
_ => PathKind::Inductive,
248+
}
249+
}
250+
233251
/// Creates a root evaluation context and search graph. This should only be
234252
/// used from outside of any evaluation, and other methods should be preferred
235253
/// over using this manually (such as [`SolverDelegateEvalExt::evaluate_root_goal`]).
@@ -257,6 +275,7 @@ where
257275
variables: Default::default(),
258276
var_values: CanonicalVarValues::dummy(),
259277
is_normalizes_to_goal: false,
278+
is_coinductive_probe: false,
260279
origin_span,
261280
tainted: Ok(()),
262281
};
@@ -295,6 +314,7 @@ where
295314
variables: canonical_input.canonical.variables,
296315
var_values,
297316
is_normalizes_to_goal: false,
317+
is_coinductive_probe: false,
298318
predefined_opaques_in_body: input.predefined_opaques_in_body,
299319
max_input_universe: canonical_input.canonical.max_universe,
300320
search_graph,
@@ -340,6 +360,7 @@ where
340360
cx: I,
341361
search_graph: &'a mut SearchGraph<D>,
342362
canonical_input: CanonicalInput<I>,
363+
step_kind_from_parent: PathKind,
343364
goal_evaluation: &mut ProofTreeBuilder<D>,
344365
) -> QueryResult<I> {
345366
let mut canonical_goal_evaluation =
@@ -352,6 +373,7 @@ where
352373
search_graph.with_new_goal(
353374
cx,
354375
canonical_input,
376+
step_kind_from_parent,
355377
&mut canonical_goal_evaluation,
356378
|search_graph, canonical_goal_evaluation| {
357379
EvalCtxt::enter_canonical(
@@ -395,12 +417,10 @@ where
395417
/// `NormalizesTo` is only used by `AliasRelate`, all other callsites
396418
/// should use [`EvalCtxt::evaluate_goal`] which discards that empty
397419
/// storage.
398-
// FIXME(-Znext-solver=coinduction): `_source` is currently unused but will
399-
// be necessary once we implement the new coinduction approach.
400420
pub(super) fn evaluate_goal_raw(
401421
&mut self,
402422
goal_evaluation_kind: GoalEvaluationKind,
403-
_source: GoalSource,
423+
source: GoalSource,
404424
goal: Goal<I, I::Predicate>,
405425
) -> Result<(NestedNormalizationGoals<I>, HasChanged, Certainty), NoSolution> {
406426
let (orig_values, canonical_goal) = self.canonicalize_goal(goal);
@@ -410,6 +430,7 @@ where
410430
self.cx(),
411431
self.search_graph,
412432
canonical_goal,
433+
self.step_kind_for_source(source),
413434
&mut goal_evaluation,
414435
);
415436
let response = match canonical_response {

compiler/rustc_next_trait_solver/src/solve/eval_ctxt/probe.rs

+36
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,42 @@ where
3535
variables: outer_ecx.variables,
3636
var_values: outer_ecx.var_values,
3737
is_normalizes_to_goal: outer_ecx.is_normalizes_to_goal,
38+
is_coinductive_probe: outer_ecx.is_coinductive_probe,
39+
predefined_opaques_in_body: outer_ecx.predefined_opaques_in_body,
40+
max_input_universe,
41+
search_graph: outer_ecx.search_graph,
42+
nested_goals: outer_ecx.nested_goals.clone(),
43+
origin_span: outer_ecx.origin_span,
44+
tainted: outer_ecx.tainted,
45+
inspect: outer_ecx.inspect.take_and_enter_probe(),
46+
};
47+
let r = nested_ecx.delegate.probe(|| {
48+
let r = f(&mut nested_ecx);
49+
nested_ecx.inspect.probe_final_state(delegate, max_input_universe);
50+
r
51+
});
52+
if !nested_ecx.inspect.is_noop() {
53+
let probe_kind = probe_kind(&r);
54+
nested_ecx.inspect.probe_kind(probe_kind);
55+
outer_ecx.inspect = nested_ecx.inspect.finish_probe();
56+
}
57+
r
58+
}
59+
60+
pub(in crate::solve) fn enter_coinductively(
61+
self,
62+
f: impl FnOnce(&mut EvalCtxt<'_, D>) -> T,
63+
) -> T {
64+
let ProbeCtxt { ecx: outer_ecx, probe_kind, _result } = self;
65+
66+
let delegate = outer_ecx.delegate;
67+
let max_input_universe = outer_ecx.max_input_universe;
68+
let mut nested_ecx = EvalCtxt {
69+
delegate,
70+
variables: outer_ecx.variables,
71+
var_values: outer_ecx.var_values,
72+
is_normalizes_to_goal: outer_ecx.is_normalizes_to_goal,
73+
is_coinductive_probe: true,
3874
predefined_opaques_in_body: outer_ecx.predefined_opaques_in_body,
3975
max_input_universe,
4076
search_graph: outer_ecx.search_graph,

compiler/rustc_next_trait_solver/src/solve/normalizes_to/inherent.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ where
3939
//
4040
// FIXME(-Znext-solver=coinductive): I think this should be split
4141
// and we tag the impl bounds with `GoalSource::ImplWhereBound`?
42-
// Right not this includes both the impl and the assoc item where bounds,
42+
// Right now this includes both the impl and the assoc item where bounds,
4343
// and I don't think the assoc item where-bounds are allowed to be coinductive.
4444
self.add_goals(
4545
GoalSource::Misc,

0 commit comments

Comments
 (0)