Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

normalize in trait_ref_is_knowable in new solver #114457

Merged
merged 2 commits into from
Aug 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 35 additions & 19 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

self.assemble_param_env_candidates(goal, &mut candidates);

self.assemble_coherence_unknowable_candidates(goal, &mut candidates);

candidates
}

Expand Down Expand Up @@ -363,10 +365,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

self.assemble_object_bound_candidates(goal, &mut candidates);

self.assemble_coherence_unknowable_candidates(goal, &mut candidates);

self.assemble_candidates_after_normalizing_self_ty(goal, &mut candidates, num_steps);

candidates
}

Expand Down Expand Up @@ -877,26 +876,43 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
goal: Goal<'tcx, G>,
candidates: &mut Vec<Candidate<'tcx>>,
) {
let tcx = self.tcx();
match self.solver_mode() {
SolverMode::Normal => return,
SolverMode::Coherence => {
let trait_ref = goal.predicate.trait_ref(self.tcx());
match coherence::trait_ref_is_knowable(self.tcx(), trait_ref) {
Ok(()) => {}
Err(_) => match self
.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
{
Ok(result) => candidates.push(Candidate {
source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
result,
}),
// FIXME: This will be reachable at some point if we're in
// `assemble_candidates_after_normalizing_self_ty` and we get a
// universe error. We'll deal with it at this point.
Err(NoSolution) => bug!("coherence candidate resulted in NoSolution"),
},
SolverMode::Coherence => {}
};

let result = self.probe_candidate("coherence unknowable").enter(|ecx| {
let trait_ref = goal.predicate.trait_ref(tcx);

#[derive(Debug)]
enum FailureKind {
Overflow,
NoSolution(NoSolution),
}
Comment on lines +888 to +892
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think lazy_normalize_ty should just return this FailureKind?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I think it's valuable to guarantee that trait_ref_is_knowable always returns the outer Ok when used outside of the new solver

let lazily_normalize_ty = |ty| match ecx.try_normalize_ty(goal.param_env, ty) {
Ok(Some(ty)) => Ok(ty),
Ok(None) => Err(FailureKind::Overflow),
Err(e) => Err(FailureKind::NoSolution(e)),
};

match coherence::trait_ref_is_knowable(tcx, trait_ref, lazily_normalize_ty) {
Err(FailureKind::Overflow) => {
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::OVERFLOW)
}
Err(FailureKind::NoSolution(NoSolution)) | Ok(Ok(())) => Err(NoSolution),
Ok(Err(_)) => {
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS)
}
}
});

match result {
Ok(result) => candidates.push(Candidate {
source: CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
result,
}),
Err(NoSolution) => {}
}
}

Expand Down
82 changes: 49 additions & 33 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,44 +388,60 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
&& is_normalizes_to_hack == IsNormalizesToHack::No
&& !self.search_graph.in_cycle()
{
debug!("rerunning goal to check result is stable");
self.search_graph.reset_encountered_overflow(encountered_overflow);
let (_orig_values, canonical_goal) = self.canonicalize_goal(goal);
let Ok(new_canonical_response) = EvalCtxt::evaluate_canonical_goal(
self.tcx(),
self.search_graph,
canonical_goal,
// FIXME(-Ztrait-solver=next): we do not track what happens in `evaluate_canonical_goal`
&mut ProofTreeBuilder::new_noop(),
) else {
bug!(
"goal went from {certainty:?} to error: re-canonicalized goal={canonical_goal:#?} \
first_response={canonical_response:#?},
second response was error"
);
};
// We only check for modulo regions as we convert all regions in
// the input to new existentials, even if they're expected to be
// `'static` or a placeholder region.
if !new_canonical_response.value.var_values.is_identity_modulo_regions() {
bug!(
"unstable result: re-canonicalized goal={canonical_goal:#?} \
first_response={canonical_response:#?} \
second_response={new_canonical_response:#?}"
);
}
if certainty != new_canonical_response.value.certainty {
bug!(
"unstable certainty: {certainty:#?} re-canonicalized goal={canonical_goal:#?} \
first_response={canonical_response:#?} \
second_response={new_canonical_response:#?}"
);
}
// The nested evaluation has to happen with the original state
// of `encountered_overflow`.
let from_original_evaluation =
self.search_graph.reset_encountered_overflow(encountered_overflow);
self.check_evaluate_goal_stable_result(goal, canonical_goal, canonical_response);
// In case the evaluation was unstable, we manually make sure that this
// debug check does not influence the result of the parent goal.
self.search_graph.reset_encountered_overflow(from_original_evaluation);
}

Ok((has_changed, certainty, nested_goals))
}

fn check_evaluate_goal_stable_result(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should even when debug assertions are enabled -- otherwise, there's UI test instability between rustc w/ debug_assertions and not.

&mut self,
goal: Goal<'tcx, ty::Predicate<'tcx>>,
original_input: CanonicalInput<'tcx>,
original_result: CanonicalResponse<'tcx>,
) {
let (_orig_values, canonical_goal) = self.canonicalize_goal(goal);
let result = EvalCtxt::evaluate_canonical_goal(
self.tcx(),
self.search_graph,
canonical_goal,
// FIXME(-Ztrait-solver=next): we do not track what happens in `evaluate_canonical_goal`
&mut ProofTreeBuilder::new_noop(),
);

macro_rules! fail {
($msg:expr) => {{
let msg = $msg;
warn!(
"unstable result: {msg}\n\
original goal: {original_input:?},\n\
original result: {original_result:?}\n\
re-canonicalized goal: {canonical_goal:?}\n\
second response: {result:?}"
);
return;
}};
}

let Ok(new_canonical_response) = result else { fail!("second response was error") };
// We only check for modulo regions as we convert all regions in
// the input to new existentials, even if they're expected to be
// `'static` or a placeholder region.
if !new_canonical_response.value.var_values.is_identity_modulo_regions() {
fail!("additional constraints from second response")
}
if original_result.value.certainty != new_canonical_response.value.certainty {
fail!("unstable certainty")
}
}

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
31 changes: 31 additions & 0 deletions compiler/rustc_trait_selection/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,37 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

Ok(self.make_ambiguous_response_no_constraints(maybe_cause))
}

/// Normalize a type when it is structually matched on.
///
/// For self types this is generally already handled through
/// `assemble_candidates_after_normalizing_self_ty`, so anything happening
/// in [`EvalCtxt::assemble_candidates_via_self_ty`] does not have to normalize
/// the self type. It is required when structurally matching on any other
/// arguments of a trait goal, e.g. when assembling builtin unsize candidates.
fn try_normalize_ty(
&mut self,
param_env: ty::ParamEnv<'tcx>,
mut ty: Ty<'tcx>,
) -> Result<Option<Ty<'tcx>>, NoSolution> {
for _ in 0..self.local_overflow_limit() {
let ty::Alias(_, projection_ty) = *ty.kind() else {
return Ok(Some(ty));
};

let normalized_ty = self.next_ty_infer();
let normalizes_to_goal = Goal::new(
self.tcx(),
param_env,
ty::ProjectionPredicate { projection_ty, term: normalized_ty.into() },
);
self.add_goal(normalizes_to_goal);
self.try_evaluate_added_goals()?;
ty = self.resolve_vars_if_possible(normalized_ty);
}

Ok(None)
}
}

fn response_no_constraints_raw<'tcx>(
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_trait_selection/src/solve/search_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,13 @@ impl<'tcx> SearchGraph<'tcx> {
/// Resets `encountered_overflow` of the current goal.
///
/// This should only be used for the check in `evaluate_goal`.
pub(super) fn reset_encountered_overflow(&mut self, encountered_overflow: bool) {
if encountered_overflow {
self.stack.raw.last_mut().unwrap().encountered_overflow = true;
pub(super) fn reset_encountered_overflow(&mut self, encountered_overflow: bool) -> bool {
if let Some(last) = self.stack.raw.last_mut() {
let prev = last.encountered_overflow;
last.encountered_overflow = encountered_overflow;
prev
} else {
false
}
}

Expand Down
39 changes: 1 addition & 38 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
// We need to normalize the b_ty since it's matched structurally
// in the other functions below.
let b_ty = match ecx
.normalize_non_self_ty(goal.predicate.trait_ref.args.type_at(1), goal.param_env)
.try_normalize_ty(goal.param_env, goal.predicate.trait_ref.args.type_at(1))
{
Ok(Some(b_ty)) => b_ty,
Ok(None) => return vec![misc_candidate(ecx, Certainty::OVERFLOW)],
Expand Down Expand Up @@ -927,41 +927,4 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
let candidates = self.assemble_and_evaluate_candidates(goal);
self.merge_candidates(candidates)
}

/// Normalize a non-self type when it is structually matched on when solving
/// a built-in goal.
///
/// This is handled already through `assemble_candidates_after_normalizing_self_ty`
/// for the self type, but for other goals, additional normalization of other
/// arguments may be needed to completely implement the semantics of the trait.
///
/// This is required when structurally matching on any trait argument that is
/// not the self type.
fn normalize_non_self_ty(
&mut self,
mut ty: Ty<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> Result<Option<Ty<'tcx>>, NoSolution> {
if !matches!(ty.kind(), ty::Alias(..)) {
return Ok(Some(ty));
}

for _ in 0..self.local_overflow_limit() {
let ty::Alias(_, projection_ty) = *ty.kind() else {
return Ok(Some(ty));
};

let normalized_ty = self.next_ty_infer();
let normalizes_to_goal = Goal::new(
self.tcx(),
param_env,
ty::ProjectionPredicate { projection_ty, term: normalized_ty.into() },
);
self.add_goal(normalizes_to_goal);
self.try_evaluate_added_goals()?;
ty = self.resolve_vars_if_possible(normalized_ty);
}

Ok(None)
}
}
Loading