-
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
Cleanup codegen_fn_attrs
#109091
Cleanup codegen_fn_attrs
#109091
Conversation
r? @TaKO8Ki (rustbot has picked a reviewer for you, use r? to override) |
b814653
to
6070de2
Compare
6070de2
to
de7b9da
Compare
de7b9da
to
1d70bd4
Compare
@bors r+ |
Cleanup `codegen_fn_attrs` The `match` control flow construct has been stable since 1.0, we should use it here. Sorry for the hard to review diff, I did try to at least split it into two commits. But looking at before-after side-by-side (instead of whatever github is doing) is probably the easiest way to make sure that I didn't forget about anything. On top of rust-lang#109088, you can wait for that
Hmm, doesn't seem that likely as it's happening when building LLVM, but I'll take another look here anyways. |
I took another look at the diff and it all seemed fine to me, so I'll leave it open and we'll see whether it fails again or whether that was spurious or some other PR. |
⌛ Testing commit 1d70bd4 with merge c807155be55a3833d1b8290a928d9a45d236922f... |
💔 Test failed - checks-actions |
@bors retry |
⌛ Testing commit 1d70bd4 with merge 1b866165b1836283a1f3551125717b2b6a2cf79a... |
💔 Test failed - checks-actions |
@bors rollup=never |
The job Click to see the possible cause of the failure (guessed by this bot)
|
⌛ Testing commit 1d70bd4 with merge dfe4e76d991aab5ef43ed9430b2042b19f4c241c... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
@bors retry p=0 |
☀️ Test successful - checks-actions |
Finished benchmarking commit (7a06007): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. |
The
match
control flow construct has been stable since 1.0, we should use it here.Sorry for the hard to review diff, I did try to at least split it into two commits. But looking at before-after side-by-side (instead of whatever github is doing) is probably the easiest way to make sure that I didn't forget about anything.
On top of #109088, you can wait for that