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

our folding strategy in check_type_bound is insufficient #116

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants