Raise noncontinuable exception when aborting on earlier Windows versions #76203
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @LukasKalbertodt (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
jyn514
added
O-windows
nxtn
force-pushed
the
failfast
branch
4 times, most recently
from
bb00fe1 to
c6ff359
Compare
|
I'm not qualified to review this PR. @rust-lang/libs can anyone of you take this? |
| } | ||
| } else { | ||
| match compat::lookup("kernel32", "RaiseFailFastException") { | ||
| Some(..) => c::RaiseFailFastException(ptr::null(), ptr::null(), 0), |
There was a problem hiding this comment.
I think this will cause a runtime missing symbol error on older Windows versions here since you are directly linking to the RaiseFailFastException symbol.
There was a problem hiding this comment.
RaiseFailFastException requires Windows 7 so we'd need a fallback for XP/Vista.
| } | ||
| } else { | ||
| c::RaiseFailFastException(ptr::null(), ptr::null(), 0); |
There was a problem hiding this comment.
Why is there no fallback to RaiseException here?
|
☔ The latest upstream changes (presumably #76964) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels: |
crlf0710
added
the
S-waiting-on-author
|
@NextTurn Ping from triage: could you address the comments above? Thanks! |
| } | ||
| } else { | ||
| match compat::lookup("kernel32", "RaiseFailFastException") { |
There was a problem hiding this comment.
If you put RaiseFailFastException in the compat_fn!{} block in c.rs, you can use c::RaiseFailFastException::is_available() here (since #76979).
|
@NextTurn thanks for taking the time to contribute. I have to close this due to inactivity. If you wish and you have the time you can open a new PR with these changes and we'll take it from there. Thanks |
Dylan-DPC-zz
added
S-inactive
See #33814 (comment) for background.