stabilized compiler_fences (fixes #41091) by budziq · Pull Request #44595 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation14 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

I did not know what to proceed with "unstable-book" entry. The feature would no longer be unstable so I have deleted it. If it was the wrong call I'll revert it (unfortunately his case is not described in the CONTRIBUTING.md).

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (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.

Thanks! Maybe the docs could move to the function?

Maybe the docs could move to the function?

I've translated part of the unstable-book article into an example. I'm not sure if it didn't came out forced.

📌 Commit 571941b has been approved by alexcrichton

Hmm, I think I would argue for more of the text from the unstable book to be incorporated into the documentation for compiler_fence. This is a very specialized function, and having the docs to explain that is probably a good idea. In particular, I think the last paragraph before the Examples header, and the first paragraph after, could be kept verbatim in the docs for compiler_fence, perhaps under a heading of "Usage"? There's also the question about whether it is appropriate to refer readers to the equivalent function in C++ like the unstable book text does, though I feel less strongly about that.

alexcrichton added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Sep 15, 2017

want to update with @jonhoo's thoughts?

@alexcrichton Gladly! Although I might not have time to approach it this weekend.

In particular, I think the last paragraph before the Examples header, and the first paragraph after, could be kept verbatim

@jonhoo I've originally omitted these paragraphs as these convey the same message as current docs present above compiler_fence (although in much terser form). I can replace one with the other.

There's also the question about whether it is appropriate to refer readers to the equivalent function in C++

I'm not sure about linking to C++ reference but I would prefer not to point to StackOverflow, leaving only the linux kernel link.

I've updated the docs with the best I could come up with :)

📌 Commit 5f62c0c has been approved by alexcrichton

TimNN added a commit to TimNN/rust that referenced this pull request

Sep 17, 2017

…=alexcrichton

stabilized compiler_fences (fixes rust-lang#41091)

TimNN mentioned this pull request

Sep 17, 2017

TimNN added a commit to TimNN/rust that referenced this pull request

Sep 17, 2017

…=alexcrichton

stabilized compiler_fences (fixes rust-lang#41091)

TimNN mentioned this pull request

Sep 17, 2017

TimNN added a commit to TimNN/rust that referenced this pull request

Sep 17, 2017

…=alexcrichton

stabilized compiler_fences (fixes rust-lang#41091)

TimNN mentioned this pull request

Sep 17, 2017

bors added a commit that referenced this pull request

Sep 17, 2017

Rollup of 17 pull requests

Successful merges: #44073, #44088, #44381, #44397, #44509, #44533, #44549, #44553, #44562, #44567, #44595, #44604, #44617, #44622, #44630, #44639, #44647
Failed merges:

budziq deleted the stabilize_compiler_fences branch

September 17, 2017 16:46

The way it is currently written, the `assert_eq!` is not guaranteed to
succeed, despite everything happening in a single thread. To see why,
remember that the compiler is free to swap the stores to
`IMPORTANT_VARIABLE` and `IS_READ` since they are both