A less misleading intro to atomic::Ordering #55233
Conversation
This goes into more detail, but without suggesting misleading things. It also tries to point out several footguns about atomic orderings. Fixes #55196.
|
r? @aidanhs (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
frewsxcv
added
T-libs-api
|
Ping from triage @aidanhs / @rust-lang/docs: This PR requires your review. |
|
Please add all missing types/functions links. |
|
@GuillaumeGomez can you provide a resource or an example on how to do that? |
|
@frewsxcv All docs have the old linking style. Or you can try the new one (just like you would import an item in most cases). For example: pub fn foo() {}
/// I want to link to [`foo`]!
pub fn bar() {}In here, you'll generate a link to the /// I want to link to [`foo`][foo]!
pub fn bar() {} |
|
For some reason, I had to provide the targets for the links, only naming the variants didn't seem to do anything. Anyway, fixup with the links is there for review (I'll rebase and squash the fixups once the review is done, but I don't like doing history rewrites during). |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
From a readability perspective, this looks good to me, but I can't vouch for the correctness of the text. Can someone from @rust-lang/libs read this over? |
|
I would personally be wary of trying to include too much information about the specifics of memory orderings in our documentation, can we perhaps simply link to LLVM/C++ documentation? I would expect those to be thoroughly vetted and good sources to read up on, and because our model is the same that should suffice as well |
|
So, would the way forward be to strip the original of the wrong information (making it terser) and maybe put something like this PR into nomicon or such place? |
|
That would my own personal preference, yes, although others may feel differently |
|
@alexcrichton Seems like a good idea actually. Give an access to more information if you want it but remaining all about the type usage explanation. Yes, I really like it. |
|
OK, I'll do the minimal version, then. Let's close this one. |
…stjepang atomic::Ordering: Get rid of misleading parts of intro Remove the parts of atomic::Ordering's intro that wrongly claimed that SeqCst prevents all reorderings around it. Closes rust-lang#55196 This is a (minimal) alternative to rust-lang#55233. I also wonder if it would be worth adding at least some warnings that atomics are often a footgun/hard to use correctly, similarly like `mem::transmute` or other functions have.
…stjepang atomic::Ordering: Get rid of misleading parts of intro Remove the parts of atomic::Ordering's intro that wrongly claimed that SeqCst prevents all reorderings around it. Closes rust-lang#55196 This is a (minimal) alternative to rust-lang#55233. I also wonder if it would be worth adding at least some warnings that atomics are often a footgun/hard to use correctly, similarly like `mem::transmute` or other functions have.
This goes into more detail, but without suggesting misleading things. It
also tries to point out several footguns about atomic orderings.
Fixes #55196.
I'm not sure if I went too informal, though, or if the intro isn't too long. Suggestions to improvement are, of course, welcome.