-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Refactor the MirPass for QualifyAndPromoteConstants
#63994
Conversation
This PR will conflict heavily with the mentioned two PRs. I'd like to block it on getting the others merged |
@oli-obk I checked the diff and it will only conflict with each of those PRs in 3-7 lines at most |
This comment has been minimized.
This comment has been minimized.
656330c
to
0a8a3dd
Compare
Rebased & simplified things a bit. |
Ping from triage: |
r? @spastorino |
@bors r=spastorino,oli-obk |
📌 Commit 0a8a3dd has been approved by |
…oli-obk Refactor the `MirPass for QualifyAndPromoteConstants` This is an accumulation of drive-by commits while working on `Vec::new` as a stable `const fn`. The PR is probably easiest read commit-by-commit. r? @oli-obk cc @eddyb @ecstatic-morse -- your two PRs #63812 and #63860 respectively will conflict with this a tiny bit but it should be trivial to reintegrate your changes atop of this.
☀️ Test successful - checks-azure |
fn remove_drop_and_storage_dead_on_promoted_locals( | ||
body: &mut Body<'tcx>, | ||
promoted_temps: &BitSet<Local>, | ||
) { |
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 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.
Open an issue?
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 is from another issue, but I don't remember which one, heh.
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.
Not sure about the "should" here, I was just very surprised to learn that this is how promotion works.
The discussion was at #63955 (comment).
Would be nice if the PR author could toggle the default "whitespace" diff setting (the diff is way more readable with "No whitespace"). |
Wow; I just looked now. You are so right. |
AFAIK this is a setting you pick, not something the PR author sets? I can toggle this behind the gear icon in the diff view. |
Oh I see. I read it as asking the PR author here to do something.^^ |
Not a great workaround, but: In addition to the setting, you can add |
This is an accumulation of drive-by commits while working on
Vec::new
as a stableconst fn
.The PR is probably easiest read commit-by-commit.
r? @oli-obk
cc @eddyb @ecstatic-morse -- your two PRs #63812 and #63860 respectively will conflict with this a tiny bit but it should be trivial to reintegrate your changes atop of this.