Skip to content

Assemble const bounds via normal item bounds in old solver too #143235

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

Merged
merged 1 commit into from
Jul 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 66 additions & 1 deletion compiler/rustc_trait_selection/src/traits/effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ pub fn evaluate_host_effect_obligation<'tcx>(
Err(EvaluationFailure::NoSolution) => {}
}

match evaluate_host_effect_from_conditionally_const_item_bounds(selcx, obligation) {
Ok(result) => return Ok(result),
Err(EvaluationFailure::Ambiguous) => return Err(EvaluationFailure::Ambiguous),
Err(EvaluationFailure::NoSolution) => {}
}

match evaluate_host_effect_from_item_bounds(selcx, obligation) {
Ok(result) => return Ok(result),
Err(EvaluationFailure::Ambiguous) => return Err(EvaluationFailure::Ambiguous),
Expand Down Expand Up @@ -153,7 +159,9 @@ fn evaluate_host_effect_from_bounds<'tcx>(
}
}

fn evaluate_host_effect_from_item_bounds<'tcx>(
/// Assembles constness bounds from `~const` item bounds on alias types, which only
/// hold if the `~const` where bounds also hold and the parent trait is `~const`.
fn evaluate_host_effect_from_conditionally_const_item_bounds<'tcx>(
selcx: &mut SelectionContext<'_, 'tcx>,
obligation: &HostEffectObligation<'tcx>,
) -> Result<ThinVec<PredicateObligation<'tcx>>, EvaluationFailure> {
Expand Down Expand Up @@ -232,6 +240,63 @@ fn evaluate_host_effect_from_item_bounds<'tcx>(
}
}

/// Assembles constness bounds "normal" item bounds on aliases, which may include
/// unconditionally `const` bounds that are *not* conditional and thus always hold.
fn evaluate_host_effect_from_item_bounds<'tcx>(
selcx: &mut SelectionContext<'_, 'tcx>,
obligation: &HostEffectObligation<'tcx>,
) -> Result<ThinVec<PredicateObligation<'tcx>>, EvaluationFailure> {
let infcx = selcx.infcx;
let tcx = infcx.tcx;
let drcx = DeepRejectCtxt::relate_rigid_rigid(selcx.tcx());
let mut candidate = None;

let mut consider_ty = obligation.predicate.self_ty();
while let ty::Alias(kind @ (ty::Projection | ty::Opaque), alias_ty) = *consider_ty.kind() {
for clause in tcx.item_bounds(alias_ty.def_id).iter_instantiated(tcx, alias_ty.args) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why can't this use the is_conditionally_const "fast path"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the constness of these item bounds is not dependent on the constness of the associated item. If I have:

trait Foo {
    type Assoc: const A + ~const B;
}

<T as Foo>::Assoc is always const A, but only const B if T: const Foo.

let bound_clause = clause.kind();
let ty::ClauseKind::HostEffect(data) = bound_clause.skip_binder() else {
continue;
};
let data = bound_clause.rebind(data);
if data.skip_binder().trait_ref.def_id != obligation.predicate.trait_ref.def_id {
continue;
}

if !drcx.args_may_unify(
obligation.predicate.trait_ref.args,
data.skip_binder().trait_ref.args,
) {
continue;
}

let is_match =
infcx.probe(|_| match_candidate(selcx, obligation, data, true, |_, _| {}).is_ok());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
infcx.probe(|_| match_candidate(selcx, obligation, data, true, |_, _| {}).is_ok());
infcx.probe(|_| match_candidate(selcx, obligation, data, false, |_, _| {}).is_ok());

In trying to use this patch locally with my experimental const sizedness changes, I find that I get errors in libcore when this does normalisation.

I don't have a minimal example unfortunately, but the issue was when:

  • obligation.predicate.trait_ref is <<char as str::pattern::Pattern>::Searcher<'_> as marker::MetaSized>
  • data.trait_ref was <<char as str::pattern::Pattern>::Searcher<'_> as marker::MetaSized>
    • ..then normalised to <str::pattern::CharSearcher<'_> as marker::MetaSized>
  • ..and then is_match ends up being false

I don't know that it's the correct solution but changing this to false allows the crate to compile successfully.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, that change is not right in general and would cause additional failures later on since predicates that we get from the item bounds need to be normalized after substitution.

I think you should identify where the error was being emitted, what phase/step of checking we're doing, the param-env, why the goal predicate is not being normalized, etc, since it doesn't have much to do with this PR I think.

Copy link
Member

Choose a reason for hiding this comment

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

Went back and looked into this further, much easier now I've got a sense of what to look for.

Unnormalised obligations were coming from the builtin impl for const sizedness that I was adding, in particular when the final field of a struct was a projection. On that branch, I've started normalising the obligations in evaluate_host_effect_from_builtin_impls, which is similar to what ends up happening for trait predicates in confirm_builtin_candidate after the built-in impl for sizedness there, so it seems like a reasonable thing to do.

I would submit a patch for that separately but not sure how to craft a test case for it with only the builtin impl for Destruct that is in-tree today, so will keep it in my branch for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidtwco: You can just put up a PR without a test. I can either come up with a test or I'll approve it without one.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I still need to work out some issues with the change and recursive structs, but will put a patch up after resolving that.

Copy link
Member

Choose a reason for hiding this comment

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


if is_match {
if candidate.is_some() {
return Err(EvaluationFailure::Ambiguous);
} else {
candidate = Some(data);
}
}
}

if kind != ty::Projection {
break;
}

consider_ty = alias_ty.self_ty();
}

if let Some(data) = candidate {
Ok(match_candidate(selcx, obligation, data, true, |_, _| {})
.expect("candidate matched before, so it should match again"))
} else {
Err(EvaluationFailure::NoSolution)
}
}

fn evaluate_host_effect_from_builtin_impls<'tcx>(
selcx: &mut SelectionContext<'_, 'tcx>,
obligation: &HostEffectObligation<'tcx>,
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/traits/const-traits/const-via-item-bound.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//@ check-pass
//@ revisions: current next
//@ ignore-compare-mode-next-solver (explicit revisions)
//@[next] compile-flags: -Znext-solver

#![feature(const_trait_impl)]

#[const_trait]
trait Bar {}

trait Baz: const Bar {}

trait Foo {
// Well-formedenss of `Baz` requires `<Self as Foo>::Bar: const Bar`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Well-formedenss of `Baz` requires `<Self as Foo>::Bar: const Bar`.
// Well-formedness of `Baz` requires `<Self as Foo>::Bar: const Bar`.
// Make sure we assemble a candidate for that via the item bounds.
type Bar: Baz;
}

fn main() {}
Loading