Skip to content

[DRAFT] Make alias bounds sound in the new solver #110628

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

Closed
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
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,9 @@ impl<'tcx> Predicate<'tcx> {
#[instrument(level = "debug", skip(tcx), ret)]
pub fn is_coinductive(self, tcx: TyCtxt<'tcx>) -> bool {
match self.kind().skip_binder() {
// Always coinductive in the new solver.
_ if tcx.trait_solver_next() => true,
Comment on lines +525 to +526
Copy link
Member Author

Choose a reason for hiding this comment

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


ty::PredicateKind::Clause(ty::Clause::Trait(data)) => {
tcx.trait_is_coinductive(data.def_id())
}
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,9 @@ pub trait PrettyPrinter<'tcx>:
}
}
ty::Placeholder(placeholder) => match placeholder.bound.kind {
ty::BoundTyKind::Anon => p!(write("Placeholder({:?})", placeholder)),
ty::BoundTyKind::Anon => {
self.pretty_print_placeholder_var(placeholder.universe, placeholder.bound.var)?
}
ty::BoundTyKind::Param(_, name) => p!(write("{}", name)),
},
ty::Alias(ty::Opaque, ty::AliasTy { def_id, substs, .. }) => {
Expand Down Expand Up @@ -1172,6 +1174,18 @@ pub trait PrettyPrinter<'tcx>:
}
}

fn pretty_print_placeholder_var(
&mut self,
ui: ty::UniverseIndex,
var: ty::BoundVar,
) -> Result<(), Self::Error> {
if ui == ty::UniverseIndex::ROOT {
write!(self, "!{}", var.index())
} else {
write!(self, "!{}_{}", ui.index(), var.index())
}
}

fn ty_infer_name(&self, _: ty::TyVid) -> Option<Symbol> {
None
}
Expand Down
79 changes: 73 additions & 6 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::def_id::DefId;
use rustc_infer::traits::query::NoSolution;
use rustc_infer::traits::util::elaborate;
use rustc_infer::traits::Reveal;
use rustc_middle::traits::solve::{CanonicalResponse, Certainty, Goal, MaybeCause, QueryResult};
use rustc_middle::ty::fast_reject::TreatProjections;
use rustc_middle::ty::TypeFoldable;
Expand Down Expand Up @@ -87,7 +88,9 @@ pub(super) enum CandidateSource {
}

/// Methods used to assemble candidates for either trait or projection goals.
pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
pub(super) trait GoalKind<'tcx>:
TypeFoldable<TyCtxt<'tcx>> + Copy + Eq + std::fmt::Display
{
fn self_ty(self) -> Ty<'tcx>;

fn trait_ref(self, tcx: TyCtxt<'tcx>) -> ty::TraitRef<'tcx>;
Expand All @@ -106,10 +109,17 @@ pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
requirements: impl IntoIterator<Item = Goal<'tcx, ty::Predicate<'tcx>>>,
) -> QueryResult<'tcx>;

// Consider a clause specifically for a `dyn Trait` self type. This requires
fn consider_alias_bound_clause(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
assumption: ty::Predicate<'tcx>,
) -> QueryResult<'tcx>;

// FIXME: better wording here.
// Consider a clause that is spooky and we cannot trust. This requires
// additionally checking all of the supertraits and object bounds to hold,
// since they're not implied by the well-formedness of the object type.
fn consider_object_bound_candidate(
// since they're not implied by the well-formedness of anything necessarily.
fn consider_non_wf_assumption(
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a version of consider_implied_clause that registers all of the bounds relevant to a trait, namely its supertraits and item bounds. I assume we'll be using this in all of the built-in impls and for user impls too (or something like it).

Naming bikeshed I guess.

ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
assumption: ty::Predicate<'tcx>,
Expand Down Expand Up @@ -463,7 +473,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {

for assumption in self.tcx().item_bounds(alias_ty.def_id).subst(self.tcx(), alias_ty.substs)
{
match G::consider_implied_clause(self, goal, assumption, []) {
match G::consider_alias_bound_clause(self, goal, assumption) {
Ok(result) => {
candidates.push(Candidate { source: CandidateSource::AliasBound, result })
}
Expand Down Expand Up @@ -529,7 +539,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
continue;
}

match G::consider_object_bound_candidate(self, goal, assumption) {
match G::consider_non_wf_assumption(self, goal, assumption) {
Ok(result) => {
candidates.push(Candidate { source: CandidateSource::BuiltinImpl, result })
}
Expand Down Expand Up @@ -602,4 +612,61 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
}
self.flounder(&responses)
}

#[instrument(level = "debug", skip(self), ret)]
pub(super) fn evaluate_alias_bound_self_is_well_formed<G: GoalKind<'tcx>>(
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the logic we had discussed before, essentially that an alias bound only holds if the trait ref of the alias is satisfied via a param-env candidate.

&mut self,
goal: Goal<'tcx, G>,
) -> QueryResult<'tcx> {
match *goal.predicate.self_ty().kind() {
ty::Alias(ty::Projection, projection_ty) => {
let mut param_env_candidates = vec![];
let projection_trait_ref = projection_ty.trait_ref(self.tcx());

if projection_trait_ref.self_ty().is_ty_var() {
return self
.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS);
}

let trait_goal = goal.with(
self.tcx(),
ty::TraitPredicate {
trait_ref: projection_trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: ty::ImplPolarity::Positive,
},
);

// FIXME: We probably need some sort of recursion depth incr here.
// Can't come up with an example yet, though, and the worst case
// we can have is a compiler stack overflow...
self.assemble_param_env_candidates(trait_goal, &mut param_env_candidates);
self.assemble_alias_bound_candidates(trait_goal, &mut param_env_candidates);
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to consider nested alias bounds here I think - https://hackmd.io/frHMYxD5S4-rWlxkx_eWNw#3-Nested-alias-bound-candidates


// FIXME: Bad bad bad bad bad !!!!!
let lang_items = self.tcx().lang_items();
let trait_def_id = projection_trait_ref.def_id;
let funky_built_in_res = if lang_items.pointee_trait() == Some(trait_def_id) {
G::consider_builtin_pointee_candidate(self, goal)
} else if lang_items.discriminant_kind_trait() == Some(trait_def_id) {
G::consider_builtin_discriminant_kind_candidate(self, goal)
} else {
Err(NoSolution)
};
if let Ok(result) = funky_built_in_res {
param_env_candidates
.push(Candidate { source: CandidateSource::BuiltinImpl, result });
}
Comment on lines +646 to +659
Copy link
Member Author

Choose a reason for hiding this comment

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


self.merge_candidates(param_env_candidates)
}
ty::Alias(ty::Opaque, _opaque_ty) => match goal.param_env.reveal() {
Reveal::UserFacing => {
self.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
}
Reveal::All => return Err(NoSolution),
},
_ => bug!("we shouldn't have gotten here!"),
}
}
}
109 changes: 6 additions & 103 deletions compiler/rustc_trait_selection/src/solve/assembly/structural_traits.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{def_id::DefId, Movability, Mutability};
use rustc_hir::{Movability, Mutability};
use rustc_infer::traits::query::NoSolution;
use rustc_middle::ty::{
self, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitableExt,
};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};

use crate::solve::EvalCtxt;

Expand Down Expand Up @@ -301,51 +298,11 @@ pub(in crate::solve) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
}
}

/// Assemble a list of predicates that would be present on a theoretical
/// user impl for an object type. These predicates must be checked any time
/// we assemble a built-in object candidate for an object type, since they
/// are not implied by the well-formedness of the type.
///
/// For example, given the following traits:
///
/// ```rust,ignore (theoretical code)
/// trait Foo: Baz {
/// type Bar: Copy;
/// }
///
/// trait Baz {}
/// ```
///
/// For the dyn type `dyn Foo<Item = Ty>`, we can imagine there being a
/// pair of theoretical impls:
///
/// ```rust,ignore (theoretical code)
/// impl Foo for dyn Foo<Item = Ty>
/// where
/// Self: Baz,
/// <Self as Foo>::Bar: Copy,
/// {
/// type Bar = Ty;
/// }
///
/// impl Baz for dyn Foo<Item = Ty> {}
/// ```
///
/// However, in order to make such impls well-formed, we need to do an
/// additional step of eagerly folding the associated types in the where
/// clauses of the impl. In this example, that means replacing
/// `<Self as Foo>::Bar` with `Ty` in the first impl.
///
// FIXME: This is only necessary as `<Self as Trait>::Assoc: ItemBound`
// bounds in impls are trivially proven using the item bound candidates.
// This is unsound in general and once that is fixed, we don't need to
// normalize eagerly here. See https://github.com/lcnr/solver-woes/issues/9
// for more details.
pub(in crate::solve) fn predicates_for_object_candidate<'tcx>(
/// Assemble a list of predicates that need to hold for a trait implementation
/// to be WF.
pub(in crate::solve) fn requirements_for_trait_wf<'tcx>(
ecx: &EvalCtxt<'_, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
object_bound: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
) -> Vec<ty::Predicate<'tcx>> {
let tcx = ecx.tcx();
let mut requirements = vec![];
Expand All @@ -359,59 +316,5 @@ pub(in crate::solve) fn predicates_for_object_candidate<'tcx>(
requirements.extend(tcx.item_bounds(item.def_id).subst(tcx, trait_ref.substs));
}
}

let mut replace_projection_with = FxHashMap::default();
for bound in object_bound {
if let ty::ExistentialPredicate::Projection(proj) = bound.skip_binder() {
let proj = proj.with_self_ty(tcx, trait_ref.self_ty());
let old_ty = replace_projection_with.insert(proj.def_id(), bound.rebind(proj));
assert_eq!(
old_ty,
None,
"{} has two substitutions: {} and {}",
proj.projection_ty,
proj.term,
old_ty.unwrap()
);
}
}

requirements.fold_with(&mut ReplaceProjectionWith {
ecx,
param_env,
mapping: replace_projection_with,
})
}

struct ReplaceProjectionWith<'a, 'tcx> {
ecx: &'a EvalCtxt<'a, 'tcx>,
param_env: ty::ParamEnv<'tcx>,
mapping: FxHashMap<DefId, ty::PolyProjectionPredicate<'tcx>>,
}

impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceProjectionWith<'_, 'tcx> {
fn interner(&self) -> TyCtxt<'tcx> {
self.ecx.tcx()
}

fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
if let ty::Alias(ty::Projection, alias_ty) = *ty.kind()
&& let Some(replacement) = self.mapping.get(&alias_ty.def_id)
{
// We may have a case where our object type's projection bound is higher-ranked,
// but the where clauses we instantiated are not. We can solve this by instantiating
// the binder at the usage site.
let proj = self.ecx.instantiate_binder_with_infer(*replacement);
// FIXME: Technically this folder could be fallible?
let nested = self
.ecx
.eq_and_get_goals(self.param_env, alias_ty, proj.projection_ty)
.expect("expected to be able to unify goal projection with dyn's projection");
// FIXME: Technically we could register these too..
assert!(nested.is_empty(), "did not expect unification to have any nested goals");
proj.term.ty().unwrap()
} else {
ty.super_fold_with(self)
}
}
requirements
}
34 changes: 26 additions & 8 deletions compiler/rustc_trait_selection/src/solve/project_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,31 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
}
}

fn consider_object_bound_candidate(
fn consider_alias_bound_clause(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
assumption: ty::Predicate<'tcx>,
) -> QueryResult<'tcx> {
if let Some(poly_projection_pred) = assumption.to_opt_poly_projection_pred()
&& poly_projection_pred.projection_def_id() == goal.predicate.def_id()
{
ecx.probe(|ecx| {
let assumption_projection_pred =
ecx.instantiate_binder_with_infer(poly_projection_pred);
ecx.eq(
goal.param_env,
goal.predicate.projection_ty,
assumption_projection_pred.projection_ty,
)?;
ecx.eq(goal.param_env, goal.predicate.term, assumption_projection_pred.term)?;
ecx.evaluate_alias_bound_self_is_well_formed(goal)
})
} else {
Err(NoSolution)
}
}

fn consider_non_wf_assumption(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
assumption: ty::Predicate<'tcx>,
Expand All @@ -100,16 +124,10 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
goal.predicate.projection_ty,
assumption_projection_pred.projection_ty,
)?;

let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else {
bug!("expected object type in `consider_object_bound_candidate`");
};
ecx.add_goals(
structural_traits::predicates_for_object_candidate(
structural_traits::requirements_for_trait_wf(
&ecx,
goal.param_env,
goal.predicate.projection_ty.trait_ref(tcx),
bounds,
)
.into_iter()
.map(|pred| goal.with(tcx, pred)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ rustc_index::newtype_index! {
pub struct StackDepth {}
}

#[derive(Debug)]
struct StackElem<'tcx> {
goal: CanonicalGoal<'tcx>,
has_been_used: bool,
Expand Down
33 changes: 26 additions & 7 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,31 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
}
}

fn consider_object_bound_candidate(
fn consider_alias_bound_clause(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
assumption: ty::Predicate<'tcx>,
) -> QueryResult<'tcx> {
if let Some(poly_trait_pred) = assumption.to_opt_poly_trait_pred()
&& poly_trait_pred.def_id() == goal.predicate.def_id()
{
// FIXME: Constness and polarity
ecx.probe(|ecx| {
let assumption_trait_pred =
ecx.instantiate_binder_with_infer(poly_trait_pred);
ecx.eq(
goal.param_env,
goal.predicate.trait_ref,
assumption_trait_pred.trait_ref,
)?;
ecx.evaluate_alias_bound_self_is_well_formed(goal)
})
} else {
Err(NoSolution)
}
}

fn consider_non_wf_assumption(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
assumption: ty::Predicate<'tcx>,
Expand All @@ -123,15 +147,10 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
)?;

let tcx = ecx.tcx();
let ty::Dynamic(bounds, _, _) = *goal.predicate.self_ty().kind() else {
bug!("expected object type in `consider_object_bound_candidate`");
};
ecx.add_goals(
structural_traits::predicates_for_object_candidate(
structural_traits::requirements_for_trait_wf(
&ecx,
goal.param_env,
goal.predicate.trait_ref,
bounds,
)
.into_iter()
.map(|pred| goal.with(tcx, pred)),
Expand Down
Loading