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.

Sign up for GitHub

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

Jump to bottom

Check that we don't use Rvalue::Aggregate after the deaggregator #75562

Merged
merged 6 commits into from
Aug 20, 2020
Merged

Check that we don't use Rvalue::Aggregate after the deaggregator #75562

merged 6 commits into from
Aug 20, 2020

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Aug 15, 2020

fixes #75481

r? @wesleywiser

cc @RalfJung (modified the validator)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2020
@oli-obk oli-obk force-pushed the const_prop_no_aggregates branch 2 times, most recently from 79f8c97 to 9f50e14 Compare August 18, 2020 06:44
@oli-obk oli-obk force-pushed the const_prop_no_aggregates branch from 744e74d to de7c836 Compare August 18, 2020 11:01
@RalfJung
Copy link
Member

(The force-push + squashing in one go makes it impossible to only review the delta since the last time I checked...)

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 18, 2020

(The force-push + squashing in one go makes it impossible to only review the delta since the last time I checked...)

I had to rebase, but yea, I should have made separate commits for the changes since then.

@oli-obk oli-obk force-pushed the const_prop_no_aggregates branch from d2494de to 03d38d4 Compare August 18, 2020 11:46
RalfJung
RalfJung approved these changes Aug 18, 2020
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Validator and MIR pass management LGTM now.
I can't check the changes in ConstProp and generator lowering, and I don't know if the mir-opt changes make sense.

src/librustc_middle/query/mod.rs Outdated Show resolved Hide resolved
wesleywiser
wesleywiser approved these changes Aug 20, 2020
View reviewed changes
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

I left a few minor comments but this looks great to me overall!

src/librustc_middle/mir/mod.rs Outdated Show resolved Hide resolved
src/librustc_middle/mir/mod.rs Outdated Show resolved Hide resolved
src/librustc_middle/query/mod.rs Show resolved Hide resolved
src/librustc_mir/interpret/intern.rs Show resolved Hide resolved
src/librustc_mir/transform/const_prop.rs Outdated Show resolved Hide resolved
src/tools/compiletest/src/runtest.rs Show resolved Hide resolved
@wesleywiser
Copy link
Member

Since this impacts mir-optimizations:

@bors rollup=never

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 20, 2020

📌 Commit dcc2027 has been approved by wesleywiser

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

bors commented Aug 20, 2020

⌛ Testing commit dcc2027 with merge 814d252...

@bors bors mentioned this pull request Aug 20, 2020
Merged
@bors
Copy link
Contributor

bors commented Aug 20, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: wesleywiser
Pushing 814d252 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 20, 2020
@bors bors merged commit 814d252 into rust-lang:master Aug 20, 2020
This was referenced Aug 20, 2020
Merged
Merged
@oli-obk oli-obk deleted the const_prop_no_aggregates branch March 16, 2021 12:14
@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
Reviewers

@bjorn3 bjorn3 bjorn3 left review comments

@wesleywiser wesleywiser wesleywiser approved these changes

@RalfJung RalfJung RalfJung approved these changes

Assignees

@wesleywiser wesleywiser

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
Milestone
1.47.0
Development

Successfully merging this pull request may close these issues.

Constprop can create aggregates that won't get deaggregated anymore
7 participants
@oli-obk @RalfJung @wesleywiser @bors @bjorn3 @cuviper @rust-highfive