[libcxx-commits] [PATCH] D90968: Fix for the Bug 41784

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

In addition to the comments I made, can you please add tests using the type traits like `static_assert(!std::is_copy_constructible_v<std::atomic<int>>);`?



================
Comment at: libcxx/include/atomic:1687
     _LIBCPP_INLINE_VISIBILITY
     __atomic_base(const __atomic_base&);
 #endif
----------------
Do we have the exact same issue with copy-construction? The following works, but I don't think it should:

```
#include <atomic>

int main(int, char**) {
    volatile std::atomic<int> a1;
    std::atomic<int> a2(a1);
}
```



================
Comment at: libcxx/include/atomic:1798
+
+    // delete explicitly in the most derived class to explicitly in the most derived class to satisfy
+    // the requirement that atomic (possibly volatile qualified) variables are not copyable
----------------
I don't feel like this comment adds something useful.


================
Comment at: libcxx/include/atomic:1866
+
+    // delete explicitly in the most derived class to explicitly in the most derived class to satisfy
+    // the requirement that atomic (possibly volatile qualified) variables are not copyable
----------------
I don't feel like this comment adds something useful.


================
Comment at: libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.ptr.volatile.fail.cpp:17-19
+int
+main()
+{
----------------
The signature of `main` must be `int (int, char**)` to satisfy freestanding compilation (where `main` isn't a special function).


================
Comment at: libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.volatile.fail.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
This test should be `copy.assign.volatile.verify.cpp`.

Tests ending in `.verify.cpp` are treated as Clang-verify tests. `.fail.cpp` tests too, but with special rules that we'd like to deprecate.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90968/new/

https://reviews.llvm.org/D90968