Uh oh!
There was an error while loading. Please reload this page.
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.
rust-highfive
commented
Oct 20, 2018
r? @aidanhs (rust_highfive has picked a reviewer for you, use r? to override) |
TimNN
commented
Oct 30, 2018
Ping from triage @aidanhs / @rust-lang/docs: This PR requires your review. |
GuillaumeGomez
commented
Oct 30, 2018
Please add all missing types/functions links. |
frewsxcv
commented
Oct 30, 2018
@GuillaumeGomez can you provide a resource or an example on how to do that? |
GuillaumeGomez
commented
Oct 31, 2018
@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: pubfnfoo(){}/// I want to link to [`foo`]!pubfnbar(){}In here, you'll generate a link to the /// I want to link to [`foo`][foo]!pubfnbar(){} |
vorner
commented
Nov 2, 2018
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). |
rust-highfive
commented
Nov 2, 2018
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 |
frewsxcv
commented
Nov 10, 2018
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? |
alexcrichton
commented
Nov 13, 2018
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 |
vorner
commented
Nov 13, 2018
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? |
alexcrichton
commented
Nov 13, 2018
That would my own personal preference, yes, although others may feel differently |
GuillaumeGomez
commented
Nov 15, 2018
@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. |
vorner
commented
Nov 15, 2018
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. Closesrust-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. Closesrust-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.