Skip to content

our folding strategy in check_type_bound is insufficient #116

New issue

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

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

Already on GitHub? # to your account

Open
compiler-errors opened this issue May 30, 2024 · 2 comments
Open

our folding strategy in check_type_bound is insufficient #116

compiler-errors opened this issue May 30, 2024 · 2 comments

Comments

@compiler-errors
Copy link
Member

compiler-errors commented May 30, 2024

rust-lang/rust#125786 folds associated type bounds to replace associated types w/ their concrete types. However, it only does it for identity-substituted associated types, and only for the associated type that we're checking.

This is known to be insufficient, see rust-lang/rust#135246. Fixing this likely relies on proving item-bounds during normalization and also affects the old solver. We will probably only fix this issue after stabilization.


edit: old description

This seems to be sufficient, but I'm somewhat suspicious about things like:

type Assoc<T>: Bound<Self::Assoc<u32>>;
type Assoc: Bound<Self::Other>;
type Other: ...;

We should revisit this strategy and convince ourselves it's sound before stabilization, or else make it more general (which doesn't seem too difficult).

Previous unsoundnesses:

@lcnr
Copy link
Contributor

lcnr commented Jan 7, 2025

no :3 rust-lang/rust#135011 (comment)

@lcnr lcnr changed the title Is the folding strategy in check_type_bound sufficient? our folding strategy in check_type_bound is insufficient? Jan 8, 2025
@lcnr lcnr changed the title our folding strategy in check_type_bound is insufficient? our folding strategy in check_type_bound is insufficient Jan 21, 2025
@lcnr
Copy link
Contributor

lcnr commented Jan 21, 2025

structurally folding instead of using ordinary normalization also avoids proving the trait assoc-type where-clauses when checking that the impl definition is wf, causing tests/ui/generic-associated-types/issue-91883.rs to pass:

Zalathar added a commit to Zalathar/rust that referenced this issue Feb 18, 2025
…tem-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on rust-lang#136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2025
…tem-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on rust-lang#136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 18, 2025
…tem-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on rust-lang#136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2025
…tem-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on rust-lang#136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 18, 2025
…tem-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on rust-lang#136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 19, 2025
Rollup merge of rust-lang#137000 - compiler-errors:deeply-normalize-item-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on rust-lang#136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants