-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
nounwind tests and cleanup #65346
nounwind tests and cleanup #65346
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
#![crate_type = "lib"] | ||
|
||
// CHECK: Function Attrs: norecurse nounwind | ||
pub extern fn foo() {} |
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 test was pretty useless: it always enabled optimizations, so even if we did not emit nounwind
for this function, LLVM would add it. That's why we agreed in #65020 that it could be removed.
54c58ed
to
79c623f
Compare
// unclear whether there is real value in the assumption this | ||
// can unwind. The conservatism here may just be papering over | ||
// a real problem by making some UB a bit harder to hit.) | ||
true |
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.
In #65020 nobody knew why this branch should be needed. And all the tests behave as expected without it. So I think we should remove it.
r=me once |
r? @nagisa |
@bors r=nagisa |
📌 Commit 09d7be3 has been approved by |
nounwind tests and cleanup This is a follow-up to @pnkfelix' rust-lang#65020. In particular it adds some tests as @nagisa asked. It also does a cleanup that the original PR omitted to reduce backporting risks. I hope I finally managed to write an uncontroversial PR in this area. ;) This should not change any behavior, just test it better.
nounwind tests and cleanup This is a follow-up to @pnkfelix' rust-lang#65020. In particular it adds some tests as @nagisa asked. It also does a cleanup that the original PR omitted to reduce backporting risks. I hope I finally managed to write an uncontroversial PR in this area. ;) This should not change any behavior, just test it better.
Rollup of 10 pull requests Successful merges: - #65214 (Split non-CAS atomic support off into target_has_atomic_load_store) - #65246 (vxWorks: implement get_path() and get_mode() for File fmt::Debug) - #65312 (improve performance of signed saturating_mul) - #65336 (Fix typo in task::Waker) - #65346 (nounwind tests and cleanup) - #65347 (Fix #[unwind(abort)] with Rust ABI) - #65366 (Implement Error::source on IntoStringError + Remove superfluous cause impls) - #65369 (Don't discard value names when using address or memory sanitizer) - #65370 (Add `dyn` to `Any` documentation) - #65373 (Fix typo in docs for `Rc`) Failed merges: r? @ghost
This is a follow-up to @pnkfelix' #65020. In particular it adds some tests as @nagisa asked. It also does a cleanup that the original PR omitted to reduce backporting risks.
I hope I finally managed to write an uncontroversial PR in this area. ;) This should not change any behavior, just test it better.