Skip to content

small refactor to new projection code #107348

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 1, 2023
Merged

small refactor to new projection code #107348

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c3fe58
small refactor to new projection code
lcnr Jan 27, 2023
85e6f38
assert that solver results are stable
lcnr Jan 27, 2023
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
77 changes: 40 additions & 37 deletions compiler/rustc_trait_selection/src/solve/project_goals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,28 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
&& goal.param_env.visit_with(&mut visitor).is_continue()
}

/// After normalizing the projection to `normalized_alias` with the given
/// `normalization_certainty`, constrain the inference variable `term` to it
/// and return a query response.
fn eq_term_and_make_canonical_response(
&mut self,
goal: Goal<'tcx, ProjectionPredicate<'tcx>>,
normalization_certainty: Certainty,
normalized_alias: impl Into<ty::Term<'tcx>>,
) -> QueryResult<'tcx> {
// The term of our goal should be fully unconstrained, so this should never fail.
//
// It can however be ambiguous when the `normalized_alias` contains a projection.
let nested_goals = self
.infcx
.eq(goal.param_env, goal.predicate.term, normalized_alias.into())
.expect("failed to unify with unconstrained term");
let rhs_certainty =
self.evaluate_all(nested_goals).expect("failed to unify with unconstrained term");

self.make_canonical_response(normalization_certainty.unify_and(rhs_certainty))
}

fn merge_project_candidates(
&mut self,
mut candidates: Vec<Candidate<'tcx>>,
Expand Down Expand Up @@ -218,7 +240,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
.map(|pred| goal.with(tcx, pred));

nested_goals.extend(where_clause_bounds);
let trait_ref_certainty = ecx.evaluate_all(nested_goals)?;
let match_impl_certainty = ecx.evaluate_all(nested_goals)?;

// In case the associated item is hidden due to specialization, we have to
// return ambiguity this would otherwise be incomplete, resulting in
Expand All @@ -230,7 +252,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
goal.predicate.def_id(),
impl_def_id
)? else {
return ecx.make_canonical_response(trait_ref_certainty.unify_and(Certainty::AMBIGUOUS));
return ecx.make_canonical_response(match_impl_certainty.unify_and(Certainty::AMBIGUOUS));
};

if !assoc_def.item.defaultness(tcx).has_value() {
Expand Down Expand Up @@ -277,17 +299,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
ty.map_bound(|ty| ty.into())
};

// The term of our goal should be fully unconstrained, so this should never fail.
//
// It can however be ambiguous when the resolved type is a projection.
let nested_goals = ecx
.infcx
.eq(goal.param_env, goal.predicate.term, term.subst(tcx, substs))
.expect("failed to unify with unconstrained term");
let rhs_certainty =
ecx.evaluate_all(nested_goals).expect("failed to unify with unconstrained term");

ecx.make_canonical_response(trait_ref_certainty.unify_and(rhs_certainty))
ecx.eq_term_and_make_canonical_response(goal, match_impl_certainty, term.subst(tcx, substs))
})
}

Expand All @@ -307,18 +319,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
)?;
let subst_certainty = ecx.evaluate_all(nested_goals)?;

// The term of our goal should be fully unconstrained, so this should never fail.
//
// It can however be ambiguous when the resolved type is a projection.
let nested_goals = ecx
.infcx
.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)
.expect("failed to unify with unconstrained term");
let rhs_certainty = ecx
.evaluate_all(nested_goals)
.expect("failed to unify with unconstrained term");

ecx.make_canonical_response(subst_certainty.unify_and(rhs_certainty))
ecx.eq_term_and_make_canonical_response(
goal,
subst_certainty,
assumption_projection_pred.term,
)
})
} else {
Err(NoSolution)
Expand Down Expand Up @@ -434,14 +439,12 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
[ty::GenericArg::from(goal.predicate.self_ty())],
));

let mut nested_goals = ecx.infcx.eq(
goal.param_env,
goal.predicate.term.ty().unwrap(),
let is_sized_certainty = ecx.evaluate_goal(goal.with(tcx, sized_predicate))?.1;
return ecx.eq_term_and_make_canonical_response(
goal,
is_sized_certainty,
tcx.types.unit,
)?;
nested_goals.push(goal.with(tcx, sized_predicate));

return ecx.evaluate_all_and_make_canonical_response(nested_goals);
);
}

ty::Adt(def, substs) if def.is_struct() => {
Expand All @@ -453,7 +456,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
tcx,
ty::Binder::dummy(goal.predicate.with_self_ty(tcx, self_ty)),
);
return ecx.evaluate_all_and_make_canonical_response(vec![new_goal]);
let (_, certainty) = ecx.evaluate_goal(new_goal)?;
Copy link
Member

@compiler-errors compiler-errors Jan 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why do we throw away changed here, but evaluate_all runs in a loop until changed is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because evaluate_all has multiple goals which can influence each other.

added a second commit which checks that the result of evaluate_goal is stable and we don't get any additional information by rerunning a goal after applying its substitutions.

Will have to change the is_identity check to ignore regions once they are implemented but that doesn't yet matter.

return ecx.make_canonical_response(certainty);
}
}
}
Expand All @@ -466,7 +470,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
tcx,
ty::Binder::dummy(goal.predicate.with_self_ty(tcx, self_ty)),
);
return ecx.evaluate_all_and_make_canonical_response(vec![new_goal]);
let (_, certainty) = ecx.evaluate_goal(new_goal)?;
return ecx.make_canonical_response(certainty);
}
},

Expand All @@ -479,9 +484,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
),
};

let nested_goals =
ecx.infcx.eq(goal.param_env, goal.predicate.term.ty().unwrap(), metadata_ty)?;
ecx.evaluate_all_and_make_canonical_response(nested_goals)
ecx.eq_term_and_make_canonical_response(goal, Certainty::Yes, metadata_ty)
})
}

Expand Down