Conversation
|
r? @TimNN (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
I like these clarifications! 👍
Another thing we could do is specify which orderings are valid for each operation (in the docs of each of the methods like store, load, swap, etc.).
| /// `compare_and_swap` also takes an [`Ordering`] argument which describes the memory | ||
| /// ordering of this operation. | ||
| /// ordering of this operation. Notice that even when using [`AcqRel`], the operation | ||
| /// might fail and hence just perform an `Acquire` load, but not have `Release` semantics. |
There was a problem hiding this comment.
This also applies to compare_exchange and compare_exchange_weak.
There was a problem hiding this comment.
No it does not. Those have a separate argument for the ordering used in case of failure.
Hm... that would make for quite a large diff, and it is already stated in the |
src/libcore/sync/atomic.rs
Outdated
| /// Corresponds to LLVM's [`Release`] ordering. | ||
| /// | ||
| /// [`Release`]: http://llvm.org/docs/Atomics.html#release | ||
| /// [`Acquire`]: http://llvm.org/docs/Atomics.html#acquire |
There was a problem hiding this comment.
Could you change these links to HTTPS as well?
src/libcore/sync/atomic.rs
Outdated
| /// might fail and hence just perform an `Acquire` load, but not have `Release` semantics. | ||
| /// | ||
| /// [`Ordering`]: enum.Ordering.html | ||
| /// [`Ordering`]: enum.Ordering.html#variant.AcqRel |
Done. I hope I didn't miss any, nor screw up one of them.^^ |
|
@stjepang: Since you already commented, and are much more familiar with the subject the I am, I think, could you review this PR? |
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
I've read through the whole set of changes and everything seems correct and clear. 👍
|
📌 Commit 6a018a0 has been approved by |
bors
added
S-waiting-on-bors
atomic ordering docs Discussion in rust-lang/rfcs#2503 revealed that this could be improved. I hope this helps.
Rollup of 15 pull requests Successful merges: - #52773 (Avoid unnecessary pattern matching against Option and Result) - #53082 (Fix doc link (again)) - #53094 (Automatically expand section if url id point to one of its component) - #53106 (atomic ordering docs) - #53110 (Account for --remap-path-prefix in save-analysis) - #53116 (NetBSD: fix signedess of char) - #53179 (Whitelist wasm32 simd128 target feature) - #53183 (Suggest comma when missing in macro call) - #53207 (Add individual docs for rotate_{left, right}) - #53211 ([nll] enable feature(nll) on various crates for bootstrap) - #53214 ([nll] enable feature(nll) on various crates for bootstrap: part 2) - #53215 (Slightly refactor syntax_ext/format) - #53217 (inline some short functions) - #53219 ([nll] enable feature(nll) on various crates for bootstrap: part 3) - #53222 (A few cleanups for rustc_target)
5 participants
Discussion in rust-lang/rfcs#2503 revealed that this could be improved. I hope this helps.