-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
New mir-opt deref_separator #95649
New mir-opt deref_separator #95649
Conversation
r? @estebank (rust-highfive has picked a reviewer for you, use r? to override) |
r? @oli-obk |
This comment has been minimized.
This comment has been minimized.
src/test/mir-opt/inline/issue_58867_inline_as_ref_as_mut.a.Inline.after.mir
Outdated
Show resolved
Hide resolved
// and copying derefed values which we need to create new statement | ||
let temp_place = | ||
Place::from(temp).project_deeper(&place.projection[idx..], tcx); | ||
patch.add_assign( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This add call is now not necessary anymore, as the exact same statement is created via mutation below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you can add though is a storagdead statement for the temp. Maybe that'll eliminate the extra cleanup block that showed up in a bunch of tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mir dump looks clean now 😸
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a bunch of new cleanup blocks showing up. Let's see if a storagedead resolves those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought storagedead would resolve all of those 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, misread the code. Yea, not sure what is going on, i'll investigate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it's just MirPatch doing that eagerly even if unneeded.
This comment has been minimized.
This comment has been minimized.
@bors r+ |
📌 Commit 1cf6d69 has been approved by |
New mir-opt deref_separator This adds a new mir-opt that split certain derefs into this form: `let x = (*a.b).c;` to => `tmp = a.b; let x = (*tmp).c;` Huge thanks to `@oli-obk` for his patient mentoring.
Rollup of 6 pull requests Successful merges: - rust-lang#95342 (Ignore "format the world" commit in git blame) - rust-lang#95353 ([bootstrap] Give a hard error when filtering tests for a file that does not exist) - rust-lang#95649 (New mir-opt deref_separator) - rust-lang#95721 (Fix typo in bootstrap/setup.rs) - rust-lang#95730 (Rename RWLock to RwLock in std::sys.) - rust-lang#95731 (Check that all hidden types are the same and then deduplicate them.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Allow multiple derefs to be splitted in deref_separator Previously in rust-lang#95649 only a single deref within projection was supported and multiple derefs caused a bunch of issues, this PR fixes those issues. `@oli-obk` helped a ton again ❤️
Allow multiple derefs to be splitted in deref_separator Previously in rust-lang#95649 only a single deref within projection was supported and multiple derefs caused a bunch of issues, this PR fixes those issues. ``@oli-obk`` helped a ton again ❤️
Allow multiple derefs to be splitted in deref_separator Previously in rust-lang#95649 only a single deref within projection was supported and multiple derefs caused a bunch of issues, this PR fixes those issues. ```@oli-obk``` helped a ton again ❤️
Make derefer work everwhere Follow up work on previous PR's rust-lang#95649 and rust-lang#95857. r? rust-lang/mir-opt _Co-Authored-By: `@oli-obk_`
This adds a new mir-opt that split certain derefs into this form:
let x = (*a.b).c;
to =>tmp = a.b; let x = (*tmp).c;
Huge thanks to @oli-obk for his patient mentoring.