user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Auto merge of #34046 - Vtec234:fix-atomic-doc, r=steveklabnik

Browse source 
Fix wrong statement in compare_exchange doc

The documentation for `core::sync::atomic::AtomicSomething::compare_exchange` contains a wrong, or imprecise, statement about the return value. It goes:

The return value is a result indicating whether the new value was written and containing
the previous value. On success this value is guaranteed to be equal to `new`.

In the second sentence, `this value` is gramatically understood as referring to `return value` from the first sentence. Due to how CAS works, the returned value is always what was in the atomic variable _before_ the operation occurred, not what was written into it during the operation. Hence, the fixed doc should say:

The return value is a result indicating whether the new value was written and containing
the previous value. On success this value is guaranteed to be equal to `current`.

This version is confirmed by the runnable examples in variants of `AtomicSomething`, e.g.

    assert_eq!(some_bool.compare_exchange(true, false, Ordering::Acquire, Ordering::Relaxed),
    Ok(true));

where the returned value is `Ok(current)`. This PR fixes all occurrences of this bug I could find.

An alternative solution would be to modify the second sentence so that it refers to the value _written_ into the Atomic rather than what was there before, in which case it would be correct. Example alternative formulation:

On success the value written into the `bool`/`usize`/`whatever` is guaranteed to be equal to `new`.

r? @steveklabnik
This commit is contained in:
bors 2016-06-09 21:29:26 -07:00 • committed by GitHub
commit c09d546ee5
1 changed files with 3 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

6
src/libcore/sync/atomic.rs
View file

@ -289,7 +289,7 @@ impl AtomicBool {
/// Stores a value into the `bool` if the current value is the same as the `current` value.
///
/// The return value is a result indicating whether the new value was written and containing
/// the previous value. On success this value is guaranteed to be equal to `new`.
/// the previous value. On success this value is guaranteed to be equal to `current`.
///
/// `compare_exchange` takes two `Ordering` arguments to describe the memory ordering of this
/// operation. The first describes the required ordering if the operation succeeds while the
@ -633,7 +633,7 @@ impl<T> AtomicPtr<T> {
/// Stores a value into the pointer if the current value is the same as the `current` value.
///
/// The return value is a result indicating whether the new value was written and containing
/// the previous value. On success this value is guaranteed to be equal to `new`.
/// the previous value. On success this value is guaranteed to be equal to `current`.
///
/// `compare_exchange` takes two `Ordering` arguments to describe the memory ordering of this
/// operation. The first describes the required ordering if the operation succeeds while the
@ -886,7 +886,7 @@ macro_rules! atomic_int {
///
/// The return value is a result indicating whether the new value was written and
/// containing the previous value. On success this value is guaranteed to be equal to
/// `new`.
/// `current`.
///
/// `compare_exchange` takes two `Ordering` arguments to describe the memory ordering of
/// this operation. The first describes the required ordering if the operation succeeds