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

MIR MutVisitor::visit_local cannot mutate all Locals #71008

Closed
jonas-schievink opened this issue Apr 10, 2020 · 1 comment · Fixed by #71013
Closed

MIR MutVisitor::visit_local cannot mutate all Locals #71008

jonas-schievink opened this issue Apr 10, 2020 · 1 comment · Fixed by #71013
Assignees
@jonas-schievink
Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonas-schievink
Copy link
Contributor

jonas-schievink commented Apr 10, 2020 •
edited
Loading

Specifically, when a ProjectionElem::Index is encountered, the contained Local is not passed to visit_local, requiring the MutVisitor to also implement process_projection_elem.

This is a footgun that may cause unsound MIR transforms, which is what happened to me while working on #71003. The CopyProp pass also has to explicitly implement process_projection_elem to be sound.
@jonas-schievink jonas-schievink added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. A-mir-opt Area: MIR optimizations labels Apr 10, 2020
@jonas-schievink
Copy link
Contributor Author

cc @rust-lang/wg-mir-opt

@jonas-schievink jonas-schievink self-assigned this Apr 10, 2020
@jonas-schievink jonas-schievink mentioned this issue Apr 10, 2020
Merged
@jonas-schievink jonas-schievink changed the title MIR MutVisitor cannot mutate all Locals MIR MutVisitor::visit_local cannot mutate all Locals Apr 11, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 12, 2020
@Dylan-DPC
Rollup merge of rust-lang#71013 - jonas-schievink:visit-projection, r…
6d36a4a 
…=eddyb

Pass the `PlaceElem::Index` local to `visit_local`

Fixes rust-lang#71008

cc @rust-lang/wg-mir-opt

r? @spastorino
@bors bors closed this as completed in c076da0 Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jonas-schievink jonas-schievink

Labels
A-mir Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass the PlaceElem::Index local to visit_local
1 participant
@jonas-schievink