cplusplus / nbballot

Handling of NB comments in response to ballots
14 stars 4 forks source link

US359 31.07.5 [atomics.ref.memop] Bad return value specification for "integral" #355

Closed wg21bot closed 4 years ago

wg21bot commented 5 years ago

In the specification of member operations common to atomic_ref and atomic_ref<T> specializations in [atomics.ref.memop] (31.7.5), all of the member functions are specified to return T, which is only correct for the atomic_ref<T>. The corresponding member operations for atomic<integral> and atomic<T> in [atomics.types.memop] (31.8.5) return T. Additionally, there is an extra int parameter in the specification of the second operator--. That declaration is supposed to be the predecrement operator, not the postdecrement operator.

Proposed change: Change the specification of member operations common to atomic_ref<integral> and atomic_ref<T*> specializations in [atomics.ref.memop] (31.7.5) as follows:

T* operator++(int) const noexcept;
  Effects: Equivalent to: return fetch_add(1);
T* operator--(int) const noexcept;
  Effects: Equivalent to: return fetch_sub(1);
T* operator++() const noexcept;
  Effects: Equivalent to: return fetch_add(1) + 1;
T* operator--(int) const noexcept;
  Effects: Equivalent to: return fetch_sub(1) - 1;

Alternatively, we could consider rewording these member operations entirely for both atomic<T> and atomic_ref<T>. For example, we could just add them to both the integer and pointer specializations, which would be clearer, but would duplicate the wording.

dkolsen-pgi commented 5 years ago

The insert and delete markings (or in this case just deletions) didn't make it into the issue description. The suggestion is to change 31.7.5 to this:

T operator++(int) const noexcept;
  Effects: Equivalent to: return fetch_add(1);
T operator--(int) const noexcept;
  Effects: Equivalent to: return fetch_sub(1);
T operator++() const noexcept;
  Effects: Equivalent to: return fetch_add(1) + 1;
T operator--() const noexcept;
  Effects: Equivalent to: return fetch_sub(1) - 1;
jensmaurer commented 5 years ago

@dkolsen-pgi , Thanks, fixed. Although the T return type is obviously wrong for an atomic_ref<T> specialization; this needs to return T (or value_type).

dkolsen-pgi commented 5 years ago

This section applies to both integer specializations and pointer specializations. Returning T* doesn't work for the integer specializations. Changing the return type to value_type would work. Or we could go with getting rid of this section and (almost) duplicating the text in both the integer specialization and pointer specialization sections (as mentioned at the end of the proposed change). std::atomic has a similar section. If we go beyond the minimal change proposed for atomic_ref, we should also consider it for atomic.

ogiroux commented 4 years ago

Recommend adopting the comment's proposed wording (i.e. ignoring the alternative), in response to US 359: SF F N A SA 5 12 1 0 0 Unanimous consent

mclow commented 4 years ago

Resolved by the adoption of P1960

jensmaurer commented 4 years ago

LWG in Belfast: Accepted with modification. See paper P1960R0.