-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Extend invalid_atomic_ordering for compare_exchange{,_weak} and fetch_update #6025
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @phansch (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. |
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.
LGTM implementation wise! However, I haven't used atomics at all, so would prefer a second review from @flip1995
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.
LGTM. But I think the examples in the documentation could benefit from some explanation.
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.
Thanks! everything LGTM now. Waiting for rustup.
@bors r=flip1995 |
📌 Commit 09f7a37 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: The invalid_atomic_ordering lint can now detect misuse of
compare_exchange
,compare_exchange_weak
, andfetch_update
.I was surprised not to find an issue or existing support here, since these are the functions which are always hardest to get the ordering right on for me (as the allowed orderings for
fail
depend on thesuccess
parameter).I believe this lint now covers all atomic methods which care about their ordering now, but I could be wrong.
Hopefully I didn't forget to do anything for the PR!