[libc++] Improve atomic_fetch_(add|sub).*.

[Mordante]

Summary

While looking at the review comments in D103765 there was an oddity in
the tests for the following functions:

• atomic_fetch_add
• atomic_fetch_add_explicit
• atomic_fetch_sub
• atomic_fetch_sub_explicit

Libc++ allows usage of
atomic_fetch_add<int>(atomic<int*>*, atomic<int*>::difference_type);
MSVC and GCC reject this code: https://godbolt.org/z/9d8WzohbE

This makes the atomic fetch(add|sub).* Standard conforming and removes the non-conforming extensions.

Fixes PR47908

[• Quuxplusone]

libcxx/include/atomic

2207–2208

IMO, libc++ is wrong to constrain these functions at all.
https://eel.is/c++draft/atomics.nonmembers
does not mention any constraints. The only requirement on _Tp here is that typename atomic<_Tp>::difference_type must exist.

So, I would simply eliminate the enable_ifs here. Write

template <class _Tp>
_LIBCPP_INLINE_VISIBILITY
_Tp atomic_fetch_add(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
{
    return __o->fetch_add(__op);
}

to match the Standard specification exactly.

Compare to atomic_store on line 1908, for example.

[Mordante]

I'll look at implementing the proper constrains for the final patch.
For now I would like to determine whether we want to keep or remove the non-standard extension.
(Not sure whether they're extensions or bugs.)

libcxx/include/atomic

2207–2208

Agreed it would be better to add the constrains in the class.
This seems to be required already. In libc++ you can add to an atomic function pointer.
https://godbolt.org/z/effMvEr8j
This isn't allowed http://eel.is/c++draft/atomics.types.pointer#5

[• Quuxplusone]

libcxx/include/atomic

2222–2238

This is not a compiler extension; I mean,

atomic<int*> a;
atomic_fetch_add(&a, 1);

must keep working no matter what. https://godbolt.org/z/ajdEYPhfK
I suggest that you keep it working — and simplify the implementation — by removing the bogus constraints on line 2187, and then delete lines 2196-2212.
(I'm willing to commandeer, if what I'm saying isn't making sense to you.)

[Mordante]

libcxx/include/atomic

2222–2238

This is not a compiler extension; I mean,

atomic<int*> a;
atomic_fetch_add(&a, 1);

must keep working no matter what. https://godbolt.org/z/ajdEYPhfK

Correct, but that's not an extension. These two are https://godbolt.org/z/rs1oxYEMv

	atomic<int*> a;
	atomic_fetch_add<int>(&a, 1);

	atomic<int(*)()> f;
	atomic_fetch_add(&f, 1);

I suggest that you keep it working — and simplify the implementation — by removing the bogus constraints on line 2187, and then delete lines 2196-2212.
(I'm willing to commandeer, if what I'm saying isn't making sense to you.)

What you say makes sense, but I just want to know whether we want to keep these 2 extensions (bugs?).
There's no need to commandeer, I just want to decide which direction to take before implementing the changes and the tests.

[• Quuxplusone]

libcxx/include/atomic

2222–2238

Ah. Yeah, "obviously" it's OK to remove those usages.

• Using atomic_fetch_add<int> with an explicit template argument violates the general SD-8-esque rules about explicit template arguments, e.g. make_pair<int, int>.

• Incrementing a function pointer doesn't make sense semantically; currently we codegen it to a no-op; it was never supposed to be supported.

Lines 2196-2212 were clearly just trying to implement atomic_fetch_add for pointers (after accidentally/bogusly failing to support pointers in the primary template, due to that bogus constraint that needs removing).

[ldionne]

Just to record this publicly:

The question is do we want to keep the current non-conforming code or do we want to remove it?

Remove them. Almost every time we've implemented an extension, it has ended up biting us. It just keeps biting us time after time. This is not a blanket statement to remove all extensions, but in this case, let's aim to be as close to the spec as possible, period.

Thanks for looking at this.

[Mordante]

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.pass.cpp

61–62

Use an array as @Quuxplusone suggested in D103769.

[Mordante]

Rebased.
Remove the invalid overloads.
Validates the requirement the pointer points to a complete object type.
Use array's in the existing unit tests to avoid UB, as suggested by @Quuxplusone.
Test whether the functions are noexcept, as a small drive-by improvement.
Document the changes in behavior in the release notes.
Updated the patch to fix the issue for all add and sub functions.

[Mordante]

libcxx/include/atomic

1846–1847

I'm not too happy with this way to constrain the function. I'm open to better suggestions.

[• Quuxplusone]

libcxx/include/atomic

1846–1847

I recommend:

_LIBCPP_INLINE_VISIBILITY
_Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT
    {return __cxx_atomic_fetch_add(&this->__a_, __op, __m);}

This follows the Standard's specification exactly, and also is simple (no weird metaprogramming involved). Also, this is what libstdc++ does. (MSVC essentially handles this via a partial specialization: they conditional_t between _Atomic_pointer and _Atomic_storage depending on whether the pointer is an object pointer type. But we don't need to do that, the standard doesn't do that, and libstdc++ doesn't do that. Let's Keep It Simple.)

If you want to explicitly nod to the "Mandates:" in http://eel.is/c++draft/atomics.types.pointer#5 , then I recommend

_LIBCPP_INLINE_VISIBILITY
_Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
    static_assert(is_object<typename remove_pointer<_Tp>::type>::value, "Must be an object pointer type");
    return __cxx_atomic_fetch_add(&this->__a_, __op, __m);
}

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp

26 ↗

(On Diff #357800)

.verify.cpp tests are run only on libc++? This test wouldn't pass against libstdc++, for example, because libstdc++ doesn't SFINAE away fetch_sub in these cases. (There'd still be an error; it'd just be a different error, because subtracting from a void* is ill-formed.)

[Mordante]

Thanks for the review!

libcxx/include/atomic

1846–1847

I'll have a look at the other two implementations. However we need to do something else we do arithmetic on a function pointer. Or do you mean that Clang should protect us against that behavior? When I don't disable the function pointer here it allows function pointers.

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp

26 ↗

(On Diff #357800)

Correct these tests are only executed with Clang and libc++. So here it's valid to validate against Clang specific diagnostics.

[Mordante]

libcxx/include/atomic

1846–1847

I did some testing. Clang accepts pointers to functions, so we need an static_assert to guard against these. The other invalid pointers generate a compiler diagnostic.

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp

26 ↗

(On Diff #357800)

I recently learned these still should be in the libcxx instead of the std part of the tests.

[Mordante]

Addresses review comments.

[ldionne]

libcxx/docs/ReleaseNotes.rst

74

[• Quuxplusone]

Other than the troublesome release note, this LGTM (AFAIrecall and from a quick refresher).

libcxx/docs/ReleaseNotes.rst

62–63

Here and on line 68, I strongly recommend removing the sentence This isn't allowed by the Standard.. I mean, we wouldn't make the change if it weren't allowed, right?
I think what you meant is The old behavior wasn't conforming; the new behavior is (more) conforming. But we don't need to say that either; those who trust their library vendor will assume that the change moves in the right direction (not the wrong direction), and those who don't trust, can just go read the Standard themselves if they want some kind of rationale for why we're suddenly implementing the Standard behavior.

[Mordante]

libcxx/docs/ReleaseNotes.rst

62–63

I don't feel it's wrong to mention it. However I'm not strongly attached to the sentence, so I've removed it.

libcxx/docs/ReleaseNotes.rst

 - ...
 
 Build System Changes
 --------------------
 
 - Building the libc++ shared or static library requires a C++ 20 capable compiler.
   Use ``-DLLVM_ENABLE_PROJECTS='clang;compiler-rt' -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi'``
   to build libc++ using a fresh build of Clang.
+
+- The functions ``std::atomic<T*>::fetch_(add|sub)`` and
+  ``std::atomic_fetch_(add|sub)`` no longer accept a function pointer. While
+  this is technically an API break, the invalid syntax isn't supported by
+  libstc++ and MSVC STL.  See https://godbolt.org/z/49fvzz98d.
+
+- The call of the functions ``std::atomic_(add|sub)(std::atomic<T*>*, ...)``
+  with the explicit template argument ``T`` are now ill-formed. While this is
+  technically an API break, the invalid syntax isn't supported by libstc++ and
+  MSVC STL. See https://godbolt.org/z/v9959re3v.
+
+  Due to this change it's now possible to call these functions with the
+  explicit template argument ``T*``. This allows using the same syntax on the
+  major Standard library implementations.
+  See https://godbolt.org/z/oEfzPhTTb.
+
+  Calls to these functions where the template argument was deduced by the
+  compiler are unaffected by this change.

libcxx/include/atomic

struct atomic<_Tp*>
 
     _LIBCPP_INLINE_VISIBILITY
     _Tp* operator=(_Tp* __d) volatile _NOEXCEPT
         {__base::store(__d); return __d;}
     _LIBCPP_INLINE_VISIBILITY
     _Tp* operator=(_Tp* __d) _NOEXCEPT
         {__base::store(__d); return __d;}
 
     _LIBCPP_INLINE_VISIBILITY
-    _Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst)
-                                                                        volatile _NOEXCEPT
-        {return __cxx_atomic_fetch_add(&this->__a_, __op, __m);}
+    _Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+        // __atomic_fetch_add accepts function pointers, guard against them.
+        static_assert(!is_function<typename remove_pointer<_Tp>::type>::value, "Pointer to function isn't allowed");
+        return __cxx_atomic_fetch_add(&this->__a_, __op, __m);
+    }
+
     _LIBCPP_INLINE_VISIBILITY
-    _Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT
-        {return __cxx_atomic_fetch_add(&this->__a_, __op, __m);}
+    _Tp* fetch_add(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
+        // __atomic_fetch_add accepts function pointers, guard against them.
+        static_assert(!is_function<typename remove_pointer<_Tp>::type>::value, "Pointer to function isn't allowed");
+        return __cxx_atomic_fetch_add(&this->__a_, __op, __m);
+    }
+
     _LIBCPP_INLINE_VISIBILITY
-    _Tp* fetch_sub(ptrdiff_t __op, memory_order __m = memory_order_seq_cst)
-                                                                        volatile _NOEXCEPT
-        {return __cxx_atomic_fetch_sub(&this->__a_, __op, __m);}
+    _Tp* fetch_sub(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) volatile _NOEXCEPT {
+        // __atomic_fetch_add accepts function pointers, guard against them.
+        static_assert(!is_function<typename remove_pointer<_Tp>::type>::value, "Pointer to function isn't allowed");
+        return __cxx_atomic_fetch_sub(&this->__a_, __op, __m);
+    }
+
     _LIBCPP_INLINE_VISIBILITY
-    _Tp* fetch_sub(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT
-        {return __cxx_atomic_fetch_sub(&this->__a_, __op, __m);}
+    _Tp* fetch_sub(ptrdiff_t __op, memory_order __m = memory_order_seq_cst) _NOEXCEPT {
+        // __atomic_fetch_add accepts function pointers, guard against them.
+        static_assert(!is_function<typename remove_pointer<_Tp>::type>::value, "Pointer to function isn't allowed");
+        return __cxx_atomic_fetch_sub(&this->__a_, __op, __m);
+    }
 
     _LIBCPP_INLINE_VISIBILITY
     _Tp* operator++(int) volatile _NOEXCEPT            {return fetch_add(1);}
     _LIBCPP_INLINE_VISIBILITY
     _Tp* operator++(int) _NOEXCEPT                     {return fetch_add(1);}
     _LIBCPP_INLINE_VISIBILITY
     _Tp* operator--(int) volatile _NOEXCEPT            {return fetch_sub(1);}
     _LIBCPP_INLINE_VISIBILITY

 void atomic_notify_all(atomic<_Tp>* __o) _NOEXCEPT
 {
     __o->notify_all();
 }
 
 // atomic_fetch_add
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
 atomic_fetch_add(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
 {
     return __o->fetch_add(__op);
 }
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
 atomic_fetch_add(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
 {
     return __o->fetch_add(__op);
 }
 
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_add(volatile atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op) _NOEXCEPT
-{
-    return __o->fetch_add(__op);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_add(atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op) _NOEXCEPT
-{
-    return __o->fetch_add(__op);
-}
-
 // atomic_fetch_add_explicit
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
-atomic_fetch_add_explicit(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
-{
-    return __o->fetch_add(__op, __m);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
-atomic_fetch_add_explicit(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
+_Tp atomic_fetch_add_explicit(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
 {
     return __o->fetch_add(__op, __m);
 }
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_add_explicit(volatile atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op, memory_order __m) _NOEXCEPT
-{
-    return __o->fetch_add(__op, __m);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_add_explicit(atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op, memory_order __m) _NOEXCEPT
+_Tp atomic_fetch_add_explicit(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
 {
     return __o->fetch_add(__op, __m);
 }
 
 // atomic_fetch_sub
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
-atomic_fetch_sub(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
-{
-    return __o->fetch_sub(__op);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
-atomic_fetch_sub(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
-{
-    return __o->fetch_sub(__op);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_sub(volatile atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op) _NOEXCEPT
+_Tp atomic_fetch_sub(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
 {
     return __o->fetch_sub(__op);
 }
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_sub(atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op) _NOEXCEPT
+_Tp atomic_fetch_sub(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op) _NOEXCEPT
 {
     return __o->fetch_sub(__op);
 }
 
 // atomic_fetch_sub_explicit
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
-atomic_fetch_sub_explicit(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
-{
-    return __o->fetch_sub(__op, __m);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-typename enable_if
-<
-    is_integral<_Tp>::value && !is_same<_Tp, bool>::value && !is_const<_Tp>::value,
-    _Tp
->::type
-atomic_fetch_sub_explicit(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
-{
-    return __o->fetch_sub(__op, __m);
-}
-
-template <class _Tp>
-_LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_sub_explicit(volatile atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op, memory_order __m) _NOEXCEPT
+_Tp atomic_fetch_sub_explicit(volatile atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
 {
     return __o->fetch_sub(__op, __m);
 }
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY
-_Tp*
-atomic_fetch_sub_explicit(atomic<_Tp*>* __o, typename atomic<_Tp*>::difference_type __op, memory_order __m) _NOEXCEPT
+_Tp atomic_fetch_sub_explicit(atomic<_Tp>* __o, typename atomic<_Tp>::difference_type __op, memory_order __m) _NOEXCEPT
 {
     return __o->fetch_sub(__op, __m);
 }
 
 // atomic_fetch_and
 
 template <class _Tp>
 _LIBCPP_INLINE_VISIBILITY

libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <atomic>
+
+// template <class T>
+//     T* atomic_fetch_add(volatile atomic<T*>* obj, ptrdiff_t op)
+// template <class T>
+//     T* atomic_fetch_add(atomic<T*>* obj, ptrdiff_t op);
+
+#include <atomic>
+
+void void_pointer() {
+  {
+    volatile std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_add(&obj, 0);
+  }
+  {
+    std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_add(&obj, 0);
+  }
+}
+
+struct Incomplete;
+
+void pointer_to_incomplete_type() {
+  {
+    volatile std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_add(&obj, 0);
+  }
+  {
+    std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_add(&obj, 0);
+  }
+}
+
+void function_pointer() {
+  {
+    volatile std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_add(&fun, 0);
+  }
+  {
+    std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_add(&fun, 0);
+  }
+}
+
+struct S {
+  void fun(int);
+};
+
+void member_function_pointer() {
+  {
+    volatile std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_add' in}}
+    std::atomic_fetch_add(&fun, 0);
+  }
+  {
+    std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_add' in}}
+    std::atomic_fetch_add(&fun, 0);
+  }
+}

libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add_explicit.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <atomic>
+
+// template <class T>
+//     T*
+//     atomic_fetch_add_explicit(volatile atomic<T*>* obj, ptrdiff_t op,
+//                               memory_order m);
+// template <class T>
+//     T*
+//     atomic_fetch_add_explicit(atomic<T*>* obj, ptrdiff_t op, memory_order m);
+
+#include <atomic>
+
+void void_pointer() {
+  {
+    volatile std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_add_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_add_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+}
+
+struct Incomplete;
+
+void pointer_to_incomplete_type() {
+  {
+    volatile std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_add_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_add_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+}
+
+void function_pointer() {
+  {
+    volatile std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_add_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_add_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+}
+
+struct S {
+  void fun(int);
+};
+
+void member_function_pointer() {
+  {
+    volatile std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_add' in}}
+    std::atomic_fetch_add_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_add' in}}
+    std::atomic_fetch_add_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+}

libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <atomic>
+
+// template <class T>
+//     T* atomic_fetch_sub(volatile atomic<T*>* obj, ptrdiff_t op)
+// template <class T>
+//     T* atomic_fetch_sub(atomic<T*>* obj, ptrdiff_t op);
+
+#include <atomic>
+
+void void_pointer() {
+  {
+    volatile std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_sub(&obj, 0);
+  }
+  {
+    std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_sub(&obj, 0);
+  }
+}
+
+struct Incomplete;
+
+void pointer_to_incomplete_type() {
+  {
+    volatile std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_sub(&obj, 0);
+  }
+  {
+    std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_sub(&obj, 0);
+  }
+}
+
+void function_pointer() {
+  {
+    volatile std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_sub(&fun, 0);
+  }
+  {
+    std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_sub(&fun, 0);
+  }
+}
+
+struct S {
+  void fun(int);
+};
+
+void member_function_pointer() {
+  {
+    volatile std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_sub' in}}
+    std::atomic_fetch_sub(&fun, 0);
+  }
+  {
+    std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_sub' in}}
+    std::atomic_fetch_sub(&fun, 0);
+  }
+}

libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.verify.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <atomic>
+
+// template <class T>
+//     T*
+//     atomic_fetch_sub_explicit(volatile atomic<T*>* obj, ptrdiff_t op,
+//                               memory_order m);
+// template <class T>
+//     T*
+//     atomic_fetch_sub_explicit(atomic<T*>* obj, ptrdiff_t op, memory_order m);
+
+#include <atomic>
+
+void void_pointer() {
+  {
+    volatile std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_sub_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<void*> obj;
+    // expected-error@atomic:* {{incomplete type 'void' where a complete type is required}}
+    std::atomic_fetch_sub_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+}
+
+struct Incomplete;
+
+void pointer_to_incomplete_type() {
+  {
+    volatile std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_sub_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<Incomplete*> obj;
+    // expected-error@atomic:* {{incomplete type 'Incomplete' where a complete type is required}}
+    std::atomic_fetch_sub_explicit(&obj, 0, std::memory_order_relaxed);
+  }
+}
+
+void function_pointer() {
+  {
+    volatile std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_sub_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<void (*)(int)> fun;
+    // expected-error@atomic:* {{static_assert failed due to requirement '!is_function<void (int)>::value' "Pointer to function isn't allowed"}}
+    std::atomic_fetch_sub_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+}
+
+struct S {
+  void fun(int);
+};
+
+void member_function_pointer() {
+  {
+    volatile std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_sub' in}}
+    std::atomic_fetch_sub_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+  {
+    std::atomic<void (S::*)(int)> fun;
+    // expected-error@atomic:* {{no member named 'fetch_sub' in}}
+    std::atomic_fetch_sub_explicit(&fun, 0, std::memory_order_relaxed);
+  }
+}

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.pass.cpp

 //     atomic_fetch_add(volatile atomic<Integral>* obj, Integral op);
 //
 // template <class Integral>
 //     Integral
 //     atomic_fetch_add(atomic<Integral>* obj, Integral op);
 //
 // template <class T>
 //     T*
-//     atomic_fetch_add(volatile atomic<T*>* obj, ptrdiff_t op);
+//     atomic_fetch_add(volatile atomic<T>* obj, typename atomic<T>::difference_type) noexcept;
 //
 // template <class T>
 //     T*
-//     atomic_fetch_add(atomic<T*>* obj, ptrdiff_t op);
+//     atomic_fetch_add(atomic<T*>* obj, typename atomic<T>::difference_type) noexcept;
 
 #include <atomic>
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 #include "atomic_helpers.h"
 
 template <class T>
 struct TestFn {
   void operator()() const {
     {
         typedef std::atomic<T> A;
         A t(T(1));
         assert(std::atomic_fetch_add(&t, T(2)) == T(1));
         assert(t == T(3));
+        ASSERT_NOEXCEPT(std::atomic_fetch_add(&t, 0));
     }
     {
         typedef std::atomic<T> A;
         volatile A t(T(1));
         assert(std::atomic_fetch_add(&t, T(2)) == T(1));
         assert(t == T(3));
+        ASSERT_NOEXCEPT(std::atomic_fetch_add(&t, 0));
     }
   }
 };
 
 template <class T>
 void testp()
 {
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        A t(T(1 * sizeof(X)));
-        assert(std::atomic_fetch_add(&t, 2) == T(1*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is nonconforming
-        std::atomic_fetch_add<X>(&t, 0);
-#else
+        X a[3] = {0};
+        A t(&a[0]);
+        assert(std::atomic_fetch_add(&t, 2) == &a[0]);
         std::atomic_fetch_add<T>(&t, 0);
-#endif // _LIBCPP_VERSION
-        assert(t == T(3*sizeof(X)));
+        assert(t == &a[2]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_add(&t, 0));
     }
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        volatile A t(T(1 * sizeof(X)));
-        assert(std::atomic_fetch_add(&t, 2) == T(1*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is nonconforming
-        std::atomic_fetch_add<X>(&t, 0);
-#else
+        X a[3] = {0};
+        volatile A t(&a[0]);
+        assert(std::atomic_fetch_add(&t, 2) == &a[0]);
         std::atomic_fetch_add<T>(&t, 0);
-#endif // _LIBCPP_VERSION
-        assert(t == T(3*sizeof(X)));
+        assert(t == &a[2]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_add(&t, 0));
     }
 }
 
 int main(int, char**)
 {
     TestEachIntegralType<TestFn>()();
     testp<int*>();
     testp<const int*>();
 
   return 0;
 }

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add_explicit.pass.cpp

 struct TestFn {
   void operator()() const {
     {
         typedef std::atomic<T> A;
         A t(T(1));
         assert(std::atomic_fetch_add_explicit(&t, T(2),
                                             std::memory_order_seq_cst) == T(1));
         assert(t == T(3));
+        ASSERT_NOEXCEPT(std::atomic_fetch_add_explicit(&t, 0, std::memory_order_relaxed));
     }
     {
         typedef std::atomic<T> A;
         volatile A t(T(1));
         assert(std::atomic_fetch_add_explicit(&t, T(2),
                                             std::memory_order_seq_cst) == T(1));
         assert(t == T(3));
+        ASSERT_NOEXCEPT(std::atomic_fetch_add_explicit(&t, 0, std::memory_order_relaxed));
     }
   }
 };
 
 template <class T>
 void
 testp()
 {
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        A t(T(1 * sizeof(X)));
-        assert(std::atomic_fetch_add_explicit(&t, 2,
-                                  std::memory_order_seq_cst) == T(1*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is not conforming
-        std::atomic_fetch_add_explicit<X>(&t, 0, std::memory_order_relaxed);
-#else
+        X a[3] = {0};
+        A t(&a[0]);
+        assert(std::atomic_fetch_add_explicit(&t, 2, std::memory_order_seq_cst) == &a[0]);
         std::atomic_fetch_add_explicit<T>(&t, 0, std::memory_order_relaxed);
-#endif // _LIBCPP_VERSION
-        assert(t == T(3*sizeof(X)));
+        assert(t == &a[2]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_add_explicit(&t, 0, std::memory_order_relaxed));
     }
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        volatile A t(T(1 * sizeof(X)));
-        assert(std::atomic_fetch_add_explicit(&t, 2,
-                                  std::memory_order_seq_cst) == T(1*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is not conforming
-        std::atomic_fetch_add_explicit<X>(&t, 0, std::memory_order_relaxed);
-#else
+        X a[3] = {0};
+        volatile A t(&a[0]);
+        assert(std::atomic_fetch_add_explicit(&t, 2, std::memory_order_seq_cst) == &a[0]);
         std::atomic_fetch_add_explicit<T>(&t, 0, std::memory_order_relaxed);
-#endif // _LIBCPP_VERSION
-        assert(t == T(3*sizeof(X)));
+        assert(t == &a[2]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_add_explicit(&t, 0, std::memory_order_relaxed));
     }
 }
 
 int main(int, char**)
 {
     TestEachIntegralType<TestFn>()();
     testp<int*>();
     testp<const int*>();
 
   return 0;
 }

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub.pass.cpp

 template <class T>
 struct TestFn {
   void operator()() const {
     {
         typedef std::atomic<T> A;
         A t(T(3));
         assert(std::atomic_fetch_sub(&t, T(2)) == T(3));
         assert(t == T(1));
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub(&t, 0));
     }
     {
         typedef std::atomic<T> A;
         volatile A t(T(3));
         assert(std::atomic_fetch_sub(&t, T(2)) == T(3));
         assert(t == T(1));
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub(&t, 0));
     }
   }
 };
 
 template <class T>
 void testp()
 {
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        A t(T(3 * sizeof(X)));
-        assert(std::atomic_fetch_sub(&t, 2) == T(3*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is nonconforming
-        std::atomic_fetch_sub<X>(&t, 0);
-#else
+        X a[3] = {0};
+        A t(&a[2]);
+        assert(std::atomic_fetch_sub(&t, 2) == &a[2]);
         std::atomic_fetch_sub<T>(&t, 0);
-#endif // _LIBCPP_VERSION
-        assert(t == T(1*sizeof(X)));
+        assert(t == &a[0]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub(&t, 0));
     }
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        volatile A t(T(3 * sizeof(X)));
-        assert(std::atomic_fetch_sub(&t, 2) == T(3*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is nonconforming
-        std::atomic_fetch_sub<X>(&t, 0);
-#else
+        X a[3] = {0};
+        volatile A t(&a[2]);
+        assert(std::atomic_fetch_sub(&t, 2) == &a[2]);
         std::atomic_fetch_sub<T>(&t, 0);
-#endif // _LIBCPP_VERSION
-        assert(t == T(1*sizeof(X)));
+        assert(t == &a[0]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub(&t, 0));
     }
 }
 
 int main(int, char**)
 {
     TestEachIntegralType<TestFn>()();
     testp<int*>();
     testp<const int*>();
 
   return 0;
 }

libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_sub_explicit.pass.cpp

 struct TestFn {
   void operator()() const {
     {
         typedef std::atomic<T> A;
         A t(T(3));
         assert(std::atomic_fetch_sub_explicit(&t, T(2),
                                             std::memory_order_seq_cst) == T(3));
         assert(t == T(1));
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub_explicit(&t, 0, std::memory_order_relaxed));
     }
     {
         typedef std::atomic<T> A;
         volatile A t(T(3));
         assert(std::atomic_fetch_sub_explicit(&t, T(2),
                                             std::memory_order_seq_cst) == T(3));
         assert(t == T(1));
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub_explicit(&t, 0, std::memory_order_relaxed));
     }
   }
 };
 
 template <class T>
 void testp()
 {
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        A t(T(3 * sizeof(X)));
-        assert(std::atomic_fetch_sub_explicit(&t, 2,
-                                  std::memory_order_seq_cst) == T(3*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is nonconforming
-        std::atomic_fetch_sub_explicit<X>(&t, 0, std::memory_order_relaxed);
-#else
+        X a[3] = {0};
+        A t(&a[2]);
+        assert(std::atomic_fetch_sub_explicit(&t, 2, std::memory_order_seq_cst) == &a[2]);
         std::atomic_fetch_sub_explicit<T>(&t, 0, std::memory_order_relaxed);
-#endif // _LIBCPP_VERSION
-        assert(t == T(1*sizeof(X)));
+        assert(t == &a[0]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub_explicit(&t, 0, std::memory_order_relaxed));
     }
     {
         typedef std::atomic<T> A;
         typedef typename std::remove_pointer<T>::type X;
-        volatile A t(T(3 * sizeof(X)));
-        assert(std::atomic_fetch_sub_explicit(&t, 2,
-                                  std::memory_order_seq_cst) == T(3*sizeof(X)));
-#ifdef _LIBCPP_VERSION // libc++ is nonconforming
-        std::atomic_fetch_sub_explicit<X>(&t, 0, std::memory_order_relaxed);
-#else
+        X a[3] = {0};
+        volatile A t(&a[2]);
+        assert(std::atomic_fetch_sub_explicit(&t, 2, std::memory_order_seq_cst) == &a[2]);
         std::atomic_fetch_sub_explicit<T>(&t, 0, std::memory_order_relaxed);
-#endif // _LIBCPP_VERSION
-        assert(t == T(1*sizeof(X)));
+        assert(t == &a[0]);
+        ASSERT_NOEXCEPT(std::atomic_fetch_sub_explicit(&t, 0, std::memory_order_relaxed));
     }
 }
 
 int main(int, char**)
 {
     TestEachIntegralType<TestFn>()();
     testp<int*>();
     testp<const int*>();
 
   return 0;
 }