golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.38k stars 17.59k forks source link

sync/atomic: describe return value of atomic.Bool CompareAndSwap #66095

Open Lercher opened 7 months ago

Lercher commented 7 months ago

Proposal Details

This is the current documentation for atomic.Bool CompareAndSwap:

// CompareAndSwap executes the compare-and-swap operation for the boolean value x.
func (x *Bool) CompareAndSwap(old, new bool) (swapped bool) {
    return CompareAndSwapUint32(&x.v, b32(old), b32(new))
}

This only happens for the special case Bool: From reading only the types bool * bool -> bool it is not clear if the return value is the first or second parameter, or some other bool value. The documentation comment just states the obvious (CompareAndSwap does compare-and-swap) and swapped is ambiguous IMHO b/c it could mean the swapped-in value new, the swapped-out value old, or that the swap operation was done.

The proposal is to clarify the doc comment as e.g. (assuming this is even correct)

// CompareAndSwap executes the compare-and-swap operation for the boolean value x.
// It returns true if the swap was done.

I'm not native English speaking, so, there is probably better wording.

References

ianlancetaylor commented 7 months ago

In this case the name of the result tells you the meaning of the result. This is one of the cases where it is useful to name a result parameter: for documentation.

ianlancetaylor commented 7 months ago

There's no reason for this to be a proposal, taking it out of the proposal process.

gopherbot commented 6 months ago

Change https://go.dev/cl/572179 mentions this issue: sync/atomic: add clarifying sentence to comment for *Bool.CompareAndSwap