Move promotion into its own pass #65949

ecstatic-morse · 2019-10-29T23:09:38Z

edited

This adds a PromoteTemps pass, which runs after the old QualifyAndPromoteConsts pass, that only does promotion (no const-checking). Everything related to promotion has been removed from QualifyAndPromoteConstants: it no longer even visits the body of a non-const fn.

As a result we no longer need to keep the BitSet of promotable locals that was returned by mir_const_qualif. Rvalue-static promotion in a const is now done in promote_consts, and it operates on a set of Candidates instead. This will allow me–in a later PR–to create promoted MIR fragments for consts when necessary, which could resolve some shortcomings of the current approach (removing StorageDead).

r? @eddyb

src/librustc_mir/transform/promote_consts.rs

eddyb · 2019-10-30T13:12:37Z

I would prefer to remove the minimum possible (maybe just number two?) to avoid needless work

Is popping bubble wrap needless work 😁? I can do some of it if you want.

ecstatic-morse · 2019-10-30T14:04:47Z

Is popping bubble wrap needless work grin? I can do some of it if you want.

I actually laughed out loud at this.

Sorry, my post reads as a bit petulant to me today. The last time I did a major change to qualify_consts it didn't turn out very well (#63860). I was secretly hoping the next major change to it would be made with rm 😄.

I didn't want to waste time removing stuff that you didn't really want removed, so if you tell me you want all dead code (including the full checklist) gone I can do that. Alternatively, if it really is like popping bubble wrap for you, have at it.

eddyb · 2019-10-31T06:51:56Z

Just remembered I saw somewhere someone point out that beta branches on November 5th.

I'd suggest we remove the old promotion logic from qualify_consts just after that, to make sure the comparison trickles into beta and stable.

ecstatic-morse · 2019-10-31T16:18:43Z

I'd suggest we remove the old promotion logic from qualify_consts just after that, to make sure the comparison trickles into beta and stable.

Agreed. This would be ideal.

bors · 2019-11-06T21:22:13Z

☔ The latest upstream changes (presumably #65728) made this pull request unmergeable. Please resolve the merge conflicts.

`remove_storage_dead_and_drop` has been copied to `promote_temps` and now operates on an array of `Candidate`s instead of a bitset.

We bailed out of `QualifyAndPromoteConsts` immediately if the `min_const_fn` checks failed, which sometimes resulted in additional, spurious errors since promotion was skipped. We now do promotion in a completely separate pass, so this is no longer an issue.

We don't do promotion here anymore, so `Checker` will never even visit the body of a non-const `fn`.

ecstatic-morse · 2019-11-08T23:24:36Z

@eddyb This is ready for review now that #66066 has been merged.

eddyb · 2019-11-09T03:33:19Z

src/test/ui/consts/min_const_fn/min_const_fn.stderr

-   |                                                               ^--
-   |                                                               ||
-   |                                                               |temporary value created here
-   |                                                               returns a reference to data owned by the current function


What was this bug, looks confusing, maybe related to min_const_fn checks? cc @oli-obk

Nevermind, there's a commit that explains this!

eddyb · 2019-11-09T03:34:28Z

@bors r+

bors · 2019-11-09T03:34:29Z

📌 Commit a3b0369 has been approved by eddyb

@eddyb

…=eddyb Move promotion into its own pass **edited** This adds a `PromoteTemps` pass, which runs after the old `QualifyAndPromoteConsts` pass, that *only* does promotion (no const-checking). Everything related to promotion has been removed from `QualifyAndPromoteConstants`: it no longer even visits the body of a non-const `fn`. As a result we no longer need to keep the `BitSet` of promotable locals that was returned by `mir_const_qualif`. Rvalue-static promotion in a `const` is now done in `promote_consts`, and it operates on a set of `Candidate`s instead. This will allow me–in a later PR–to create promoted MIR fragments for `const`s when necessary, which could resolve some shortcomings of the current approach (removing `StorageDead`). r? @eddyb

@ghost

Rollup of 6 pull requests Successful merges: - #65949 (Move promotion into its own pass) - #65994 (Point at where clauses where the associated item was restricted) - #66050 (Fix C aggregate-passing ABI on powerpc) - #66134 (Point at formatting descriptor string when it is invalid) - #66172 (Stabilize @file command line arguments) - #66226 (add link to unstable book for asm! macro) Failed merges: r? @ghost

rust-highfive assigned eddyb Oct 29, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 29, 2019

ecstatic-morse changed the title ~~Move promotion into its own pass~~ [WIP] Move promotion into its own pass Oct 29, 2019

ecstatic-morse changed the title ~~[WIP] Move promotion into its own pass~~ [DO NOT MERGE] Move promotion into its own pass Oct 29, 2019

ecstatic-morse mentioned this pull request Oct 29, 2019

Tracking issue for RFC 2342, "Allow if and match in constants" #49146

Closed

4 tasks

ecstatic-morse force-pushed the promote-only-pass branch from 48eafee to 89f0507 Compare October 29, 2019 23:38

This comment has been minimized.

Sign in to view

ecstatic-morse force-pushed the promote-only-pass branch from 89f0507 to e635488 Compare October 30, 2019 00:01

This comment has been minimized.

Sign in to view

ecstatic-morse force-pushed the promote-only-pass branch from 7967fbe to d7536b2 Compare October 30, 2019 01:19

Centril reviewed Oct 30, 2019

View reviewed changes

src/librustc_mir/transform/promote_consts.rs Outdated Show resolved Hide resolved

ecstatic-morse force-pushed the promote-only-pass branch from f483911 to 1fd12cf Compare November 3, 2019 23:03

ecstatic-morse mentioned this pull request Nov 5, 2019

Add a failing UI test for multiple loops of all kinds in a const #66096

Merged

ecstatic-morse force-pushed the promote-only-pass branch from 1fd12cf to d856a4f Compare November 6, 2019 23:02

ecstatic-morse changed the title ~~[DO NOT MERGE] Move promotion into its own pass~~ Move promotion into its own pass Nov 6, 2019

ecstatic-morse force-pushed the promote-only-pass branch from d856a4f to e1abd02 Compare November 6, 2019 23:21

ecstatic-morse added 5 commits November 8, 2019 14:52

Stop returning promotables from mir_const_qualif

3783ef6

Add a PromoteTemps pass

170272b

`remove_storage_dead_and_drop` has been copied to `promote_temps` and now operates on an array of `Candidate`s instead of a bitset.

Use new PromoteTemps for promotion

b316384

Clean up dead code in qualify_consts

a3b0369

We don't do promotion here anymore, so `Checker` will never even visit the body of a non-const `fn`.

ecstatic-morse force-pushed the promote-only-pass branch from e1abd02 to a3b0369 Compare November 8, 2019 23:00

eddyb reviewed Nov 9, 2019

View reviewed changes

eddyb approved these changes Nov 9, 2019

View reviewed changes

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 Nov 9, 2019

Centril mentioned this pull request Nov 9, 2019

Rollup of 6 pull requests #66242

Merged

bors merged commit a3b0369 into rust-lang:master Nov 9, 2019

ecstatic-morse deleted the promote-only-pass branch October 6, 2020 01:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Move promotion into its own pass #65949

Move promotion into its own pass #65949

Uh oh!

ecstatic-morse commented Oct 29, 2019 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

eddyb commented Oct 30, 2019

Uh oh!

ecstatic-morse commented Oct 30, 2019 •

edited

Loading

Uh oh!

eddyb commented Oct 31, 2019

Uh oh!

ecstatic-morse commented Oct 31, 2019 •

edited

Loading

Uh oh!

bors commented Nov 6, 2019

Uh oh!

ecstatic-morse commented Nov 8, 2019

Uh oh!

eddyb Nov 9, 2019

Uh oh!

eddyb Nov 9, 2019

Uh oh!

eddyb commented Nov 9, 2019

Uh oh!

bors commented Nov 9, 2019

Uh oh!

Uh oh!

Move promotion into its own pass #65949

Move promotion into its own pass #65949

Uh oh!

Conversation

ecstatic-morse commented Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

eddyb commented Oct 30, 2019

Uh oh!

ecstatic-morse commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eddyb commented Oct 31, 2019

Uh oh!

ecstatic-morse commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Nov 6, 2019

Uh oh!

ecstatic-morse commented Nov 8, 2019

Uh oh!

eddyb Nov 9, 2019

Choose a reason for hiding this comment

Uh oh!

eddyb Nov 9, 2019

Choose a reason for hiding this comment

Uh oh!

eddyb commented Nov 9, 2019

Uh oh!

bors commented Nov 9, 2019

Uh oh!

Uh oh!

ecstatic-morse commented Oct 29, 2019 •

edited

Loading

ecstatic-morse commented Oct 30, 2019 •

edited

Loading

ecstatic-morse commented Oct 31, 2019 •

edited

Loading