-
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
Move remaining tests with NLL differences to revisions #97258
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
@cjgillot while this is technically blocked on #97206, each commit is independent (including from that). I decided to just use that as a base, since it modifies NLL tests. It's also already approved and in the bors queue . So, I would appreciate a review whenever you have time (I wouldn't mind just being able to just have this approved for when the other lands, for example). |
482b7d3
to
4233ec6
Compare
Note to self, need to update this once #96515 lands (https://github.com/rust-lang/rust/pull/96515/files#diff-6a6b025c2f35f56e17f04a29082743bcecc56b4ec83a27cbea3b676c9369dea6R1) edit: done |
05e5740
to
765592b
Compare
@@ -1,4 +1,4 @@ | |||
// revisions: nll | |||
// revisions: base nll | |||
//[nll]compile-flags: -Z borrowck=mir | |||
|
|||
//[g2p]compile-flags: -Z borrowck=mir -Z two-phase-beyond-autoref |
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.
Does this option still exist in the compiler?
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'm not sure - preexisting thought and I'd rather not make this PR anything other than mechanical. (Though I should try to remember to come back to this at some point)
Nope, no cases to draw attention to. I don't think there was anything really surprising in the nll diagnostics compared to base. Obviously, I already filed issues for the cases where we obviously are slightly worse. But there was nothing that is like "oh, that's not good". |
…nll differences to separate test
@bors r=cjgillot p=1 (bitrotty) |
📌 Commit 383fbee has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (07e7b43): comparison url. Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results
CyclesResults
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Based on #97206
I've already filed issues for any important differences that I've spotted: #97252 #97253 #97256 #97267
There is a lot here, but each commit is self-contained as a separate directory. I can split into separate PRs as wanted or needed.