Skip to content
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

Ordering::AcqRel implementation contradicts the docs #49127

Closed
imp opened this issue Mar 18, 2018 · 2 comments
Closed

Ordering::AcqRel implementation contradicts the docs #49127

imp opened this issue Mar 18, 2018 · 2 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@imp
Copy link

imp commented Mar 18, 2018

Documentation on Ordering::AcqRel states:

AcqRel
When coupled with a load, uses `Acquire` ordering, and with a store `Release` ordering.

However, this code

use std::sync::atomic::{AtomicBool, Ordering};

fn main() {
    let b = AtomicBool::new(false);
    b.store(true, Ordering::AcqRel);
    b.load(Ordering::AcqRel);
}

panics on both store and load (if the call to store() is commented out)

thread 'main' panicked at 'there is no such thing as an acquire/release store', /checkout/src/libcore/sync/atomic.rs:1482:19
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Either documentation should be updated to describe why this is not relevant to store/load, or implementation should be fixed to actually deliver on the promise.

@hanna-kruppe
Copy link
Contributor

The implementation is correct. AcquireRelease is an ordering for operations that both load and store. If you just have a load or a store, you can and should simply use acquire or release respectively.

@kennytm kennytm added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Mar 18, 2018
@imp
Copy link
Author

imp commented Mar 18, 2018

Ok, makes sense. Then the documentation should reflect that more clearly.

steveklabnik added a commit to steveklabnik/rust that referenced this issue Mar 19, 2018
This implied things that are not true.

Fixes rust-lang#49127
kennytm added a commit to kennytm/rust that referenced this issue Mar 26, 2018
Clarify AcqRel's docs

This implied things that are not true.

Fixes rust-lang#49127
TimNN added a commit to TimNN/rust that referenced this issue Mar 26, 2018
Clarify AcqRel's docs

This implied things that are not true.

Fixes rust-lang#49127
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

3 participants