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

polymorphize: unevaluated constants #75260

Merged
merged 2 commits into from
Aug 8, 2020

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Aug 7, 2020

This PR makes polymorphization visit the promoted MIR of unevaluated constants with available promoted MIR instead of visiting the substitutions of that constant - which will mark all of the generic parameters as used; in addition polymorphization will now visit non-promoted unevaluated constants rather than visit their substs.

r? @lcnr

This commit makes polymorphization visited the MIR of unevaluated
constants with available promoted MIR instead of visiting the
substitutions of that constant - which will mark all of the generic
parameters as used.

Signed-off-by: David Wood <david@davidtw.co>
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 7, 2020
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

In case you don't want to look into the more general case here,

r=me

src/librustc_mir/monomorphize/polymorphize.rs Show resolved Hide resolved
This commit makes polymorphization visit non-promoted unevaluated
constants rather than visit their substs directly.

Signed-off-by: David Wood <david@davidtw.co>
@davidtwco davidtwco requested a review from lcnr August 7, 2020 16:23
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

nice 👍 once the PR title and description has been updated,

r=me

@davidtwco davidtwco changed the title polymorphize: visit promoted MIR polymorphize: unevaluated constants Aug 7, 2020
@davidtwco
Copy link
Member Author

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Aug 7, 2020

📌 Commit d97f89b has been approved by lcnr

@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 Aug 7, 2020
@bors
Copy link
Contributor

bors commented Aug 8, 2020

⌛ Testing commit d97f89b with merge 3f091ba...

@bors
Copy link
Contributor

bors commented Aug 8, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: lcnr
Pushing 3f091ba to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 8, 2020
@bors bors merged commit 3f091ba into rust-lang:master Aug 8, 2020
@davidtwco davidtwco deleted the polymorphization-promoted-substs branch August 8, 2020 18:22
davidtwco added a commit to davidtwco/rust that referenced this pull request Aug 10, 2020
This commit constrains the support added for handling unevaluated consts
in polymorphization (introduced in rust-lang#75260) by:

- Skipping associated constants as this causes cycle errors.
- Skipping promoted constants when they contain `Self` as this ensures
  `T` is used in constants of the form `<Self as Foo<T>>`.

Signed-off-by: David Wood <david@davidtw.co>
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 10, 2020
…xes, r=eddyb

instance: only polymorphize upvar substs

This PR restricts the substitution polymorphization added in rust-lang#75255 to only apply to the tupled upvar substitution, rather than all substitutions, fixing a bunch of regressions when polymorphization is
enabled.

Due to an oversight on my part, when landing rust-lang#75260 and rust-lang#75255, some tests started failing when polymorphization was enabled that I didn't notice until after landing - this PR fixes the regressions from rust-lang#75255. rust-lang#75336 has been filed to make sure that we don't forget to try make this change again in future, as it does enable some optimisations.

r? @lcnr
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Aug 11, 2020
…xes, r=lcnr

polymorphize: constrain unevaluated const handling

This PR constrains the support added for handling unevaluated consts in polymorphization (introduced in rust-lang#75260) by:

- Skipping associated constants as this causes cycle errors.
- Skipping promoted constants when they contain `Self` as this ensures `T` is used in constants of the form `<Self as Foo<T>>`.

Due to an oversight on my part, when landing rust-lang#75260 and rust-lang#75255, some tests started failing when polymorphization was enabled that I didn't notice until after landing - this PR fixes the regressions from rust-lang#75260.

r? @lcnr
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants