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

Correctly substitute GAT's type used in normalize_param_env in check_type_bounds #109470

Merged
merged 2 commits into from
Mar 28, 2023

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Mar 22, 2023

Given:

trait Foo {
    type Assoc<T>: PartialEq<Self::Assoc<i32>>;
}

impl Foo for () {
    type Assoc<T> = Wrapper<T>;
}

struct Wrapper<T>(T);

impl<T> PartialEq<Wrapper<i32>> for Wrapper<T> { }

We add an additional predicate in the normalize_param_env in check_type_bounds that is used to normalize the GAT's bounds to check them in the impl. Problematically, though, that predicate is constructed to be for<^0> <() as Foo>::Assoc<^0> => Wrapper<T>, instead of for<^0> <() as Foo>::Assoc<^0> => Wrapper<^0>.

That means Self::Assoc<i32> in the bounds that we're checking normalizes to Wrapper<T>, instead of Wrapper<i32>, and so the bound Self::Assoc<T>: PartialEq<Self::Assoc<i32>> normalizes to Wrapper<T>: PartialEq<Wrapper<T>>, which does not hold.

Fixes this by properly substituting the RHS of that normalizes predicate that we add to the normalize_param_env. That means the bound is properly normalized to Wrapper<T>: PartialEq<Wrapper<i32>>, which does hold.


The second commit in this PR just cleans up some substs stuff and some naming.

r? @jackh726 cc #87900

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 22, 2023
@BoxyUwU BoxyUwU added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Mar 22, 2023
@rust-log-analyzer

This comment has been minimized.

@jackh726
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 28, 2023

📌 Commit 5456eec has been approved by jackh726

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 28, 2023
Rollup of 8 pull requests

Successful merges:

 - rust-lang#91793 (socket ancillary data implementation for FreeBSD (from 13 and above).)
 - rust-lang#92284 (Change advance(_back)_by to return the remainder instead of the number of processed elements)
 - rust-lang#102472 (stop special-casing `'static` in evaluation)
 - rust-lang#108480 (Use Rayon's TLV directly)
 - rust-lang#109321 (Erase impl regions when checking for impossible to eagerly monomorphize items)
 - rust-lang#109470 (Correctly substitute GAT's type used in `normalize_param_env` in `check_type_bounds`)
 - rust-lang#109562 (Update ar_archive_writer to 0.1.3)
 - rust-lang#109629 (remove obsolete `givens` from regionck)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7e90732 into rust-lang:master Mar 28, 2023
@rustbot rustbot added this to the 1.70.0 milestone Mar 28, 2023
@compiler-errors compiler-errors deleted the gat-normalize-bound branch August 11, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants