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

alias-bounds consider non_self_assumptions for the alias self type #149

Closed
lcnr opened this issue Jan 21, 2025 · 2 comments · Fixed by rust-lang/rust#135902
Closed

alias-bounds consider non_self_assumptions for the alias self type #149

lcnr opened this issue Jan 21, 2025 · 2 comments · Fixed by rust-lang/rust#135902

Comments

@lcnr
Copy link
Contributor

lcnr commented Jan 21, 2025

affected test

  • tests/ui/associated-type-bounds/cant-see-copy-bound-from-child-rigid.rs
trait Id {
    type This: ?Sized;
}

trait Trait {
    type Assoc: Id<This: Copy>;
}

fn foo<T: Trait>(x: T::Assoc) -> (T::Assoc, T::Assoc)
where 
    T::Assoc: Id<This = T::Assoc>,
{
    (x, x)
}

This passes with the new solver, but not the old.

The reason is annoying subtle!

In the new solver computing alias bounds looks at all item bounds of the alias: https://github.com/rust-lang/rust/blob/cd805f09ffbfa3896c8f50a619de9b67e1d9f3c3/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs#L583-L593

In the old solver computing alias bounds only looks at item_super_predicates in the first step: https://github.com/rust-lang/rust/blob/cd805f09ffbfa3896c8f50a619de9b67e1d9f3c3/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1617-L1625

When proving T::Assoc: Copy only the new solver therefore considers the item bound <T::Assoc as Id>::This: Copy. It is then able to normalize <T::Assoc as Id>::This to T::Assoc, allowing us to prove the goal.

We're only able to do this, if there's a projection clause in the ParamEnv or in the item bounds of our associated type:

trait Id {
    type This: ?Sized;
}
impl<T: ?Sized> Id for T {
    type This = T;
}

trait Normalize: Id<This = Self> {}
trait Trait {
    type Assoc: Id<This: Copy> + Normalize;
}

fn foo<T: Trait>(x: T::Assoc) -> (T::Assoc, T::Assoc) {
    (x, x)
}

fn main() {}

As we can definitely use the item bound to prove T::Assoc: Id, we cannot use an impl to normalize the <T::Assoc as Id>::This, instead treating this associated type as rigid. The following does not compile:

trait Id {
    type This: ?Sized;
}
impl<T: ?Sized> Id for T {
    type This = T;
}

trait Trait {
    type Assoc: Id<This: Copy>;
}

fn foo<T: Trait>(x: T::Assoc) -> (T::Assoc, T::Assoc) {
    (x, x) //~ERROR use of moved value: `x`
}

fn main() {}
@lcnr
Copy link
Contributor Author

lcnr commented Jan 21, 2025

@compiler-errors u want to allow this? 🤔 it seems edge-case-y enough and better for perf to not allow it and to copy the old solver behavior here

@lcnr lcnr changed the title item_bounds considers non_self_assumptions for the alias self type alias-bounds consider non_self_assumptions for the alias self type Jan 21, 2025
@compiler-errors
Copy link
Member

Yeah, I'll roll this back by adding non_self_assumptions to the interner and using that instead.

@compiler-errors compiler-errors self-assigned this Jan 22, 2025
fmease added a commit to fmease/rust that referenced this issue Jan 29, 2025
…d-in-new-solver, r=lcnr

Do not consider child bound assumptions for rigid alias

r? lcnr

See first commit for the important details. For second commit, I also stacked a somewhat opinionated name change, though I can separate that if needed.

Fixes rust-lang/trait-system-refactor-initiative#149
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 29, 2025
Rollup merge of rust-lang#135902 - compiler-errors:item-non-self-bound-in-new-solver, r=lcnr

Do not consider child bound assumptions for rigid alias

r? lcnr

See first commit for the important details. For second commit, I also stacked a somewhat opinionated name change, though I can separate that if needed.

Fixes rust-lang/trait-system-refactor-initiative#149
@lcnr lcnr closed this as completed Jan 29, 2025
@lcnr lcnr moved this to done in -Znext-solver=globally Feb 6, 2025
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Feb 6, 2025
…solver, r=lcnr

Do not consider child bound assumptions for rigid alias

r? lcnr

See first commit for the important details. For second commit, I also stacked a somewhat opinionated name change, though I can separate that if needed.

Fixes rust-lang/trait-system-refactor-initiative#149
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

2 participants