[libcxx-commits] [libcxx] c448ea9 - [libc++] Fix for the Bug 41784

[Louis Dionne via libcxx-commits]

Author: Ruslan Arutyunyan
Date: 2021-02-01T10:14:22-05:00
New Revision: c448ea948c28878735fefec734d8326ca2e4b33a

URL: https://github.com/llvm/llvm-project/commit/c448ea948c28878735fefec734d8326ca2e4b33a
DIFF: https://github.com/llvm/llvm-project/commit/c448ea948c28878735fefec734d8326ca2e4b33a.diff

LOG: [libc++] Fix for the Bug 41784

Add deleted volatile copy-assignment operator in the most derived atomic
to fix the Bug 41784. The root cause: there is an `operator=(T) volatile`
that has better match than the deleted copy-assignment operator of the base
class when `this` is `volatile`. The compiler sees that right operand of
the assignment operator can be converted to `T` and chooses that path
without taking into account the deleted copy-assignment operator of the
base class.

The current behavior on libstdc++ is different from what we have in libc++.
On the same test compilation fails with libstdc++. Proof: https://godbolt.org/z/nebPYd
(everything is the same except the -stdlib option).

I choose the way with explicit definition of copy-assignment for atomic
in the most derived class. But probably we can fix that by moving
`operator=(T)` overloads to the base class from both specializations.
At first glance, it shouldn't break anything.

Differential Revision: https://reviews.llvm.org/D90968

Added: 
    libcxx/test/std/atomics/atomics.types.generic/copy_semantics_traits.pass.cpp
    libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.ptr.volatile.verify.cpp
    libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.volatile.verify.cpp

Modified: 
    libcxx/include/atomic

Removed: 
    


################################################################################
diff  --git a/libcxx/include/atomic b/libcxx/include/atomic
index 0fc799a24319..f95385507413 100644
--- a/libcxx/include/atomic
+++ b/libcxx/include/atomic
@@ -1681,16 +1681,10 @@ struct __atomic_base  // false
 
 #ifndef _LIBCPP_CXX03_LANG
     __atomic_base(const __atomic_base&) = delete;
-    __atomic_base& operator=(const __atomic_base&) = delete;
-    __atomic_base& operator=(const __atomic_base&) volatile = delete;
 #else
 private:
     _LIBCPP_INLINE_VISIBILITY
     __atomic_base(const __atomic_base&);
-    _LIBCPP_INLINE_VISIBILITY
-    __atomic_base& operator=(const __atomic_base&);
-    _LIBCPP_INLINE_VISIBILITY
-    __atomic_base& operator=(const __atomic_base&) volatile;
 #endif
 };
 
@@ -1800,6 +1794,9 @@ struct atomic
     _LIBCPP_INLINE_VISIBILITY
     _Tp operator=(_Tp __d) _NOEXCEPT
         {__base::store(__d); return __d;}
+
+    atomic& operator=(const atomic&) = delete;
+    atomic& operator=(const atomic&) volatile = delete;
 };
 
 // atomic<T*>
@@ -1862,6 +1859,9 @@ struct atomic<_Tp*>
     _Tp* operator-=(ptr
diff _t __op) volatile _NOEXCEPT {return fetch_sub(__op) - __op;}
     _LIBCPP_INLINE_VISIBILITY
     _Tp* operator-=(ptr
diff _t __op) _NOEXCEPT          {return fetch_sub(__op) - __op;}
+
+    atomic& operator=(const atomic&) = delete;
+    atomic& operator=(const atomic&) volatile = delete;
 };
 
 // atomic_is_lock_free

diff  --git a/libcxx/test/std/atomics/atomics.types.generic/copy_semantics_traits.pass.cpp b/libcxx/test/std/atomics/atomics.types.generic/copy_semantics_traits.pass.cpp
new file mode 100644
index 000000000000..20c49187abbd
--- /dev/null
+++ b/libcxx/test/std/atomics/atomics.types.generic/copy_semantics_traits.pass.cpp
@@ -0,0 +1,47 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: c++03
+// UNSUPPORTED: libcpp-has-no-threads
+
+// <atomic>
+
+// template <class T>
+// struct atomic
+// {
+//     atomic(const atomic&) = delete;
+//     atomic& operator=(const atomic&) = delete;
+//     atomic& operator=(const atomic&) volatile = delete;
+// };
+
+// template <class T>
+// struct atomic<T*>
+// {
+//     atomic(const atomic&) = delete;
+//     atomic& operator=(const atomic&) = delete;
+//     atomic& operator=(const atomic&) volatile = delete;
+// };
+
+#include <atomic>
+
+#include <type_traits>
+
+template <typename T>
+using is_volatile_copy_assignable = std::is_assignable<volatile T&, const T&>;
+
+int main(int, char**)
+{
+    static_assert(!std::is_copy_constructible<std::atomic<int> >::value, "");
+    static_assert(!std::is_copy_assignable<std::atomic<int> >::value, "");
+    static_assert(!is_volatile_copy_assignable<std::atomic<int> >::value, "");
+    static_assert(!std::is_copy_constructible<std::atomic<int*> >::value, "");
+    static_assert(!std::is_copy_assignable<std::atomic<int*> >::value, "");
+    static_assert(!is_volatile_copy_assignable<std::atomic<int*> >::value, "");
+
+    return 0;
+}

diff  --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.ptr.volatile.verify.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.ptr.volatile.verify.cpp
new file mode 100644
index 000000000000..d9c46f0c34c5
--- /dev/null
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.ptr.volatile.verify.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// 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>
+
+// std::atomic
+
+// atomic& operator=( const atomic& ) volatile = delete;
+
+#include <atomic>
+
+int main(int, char**)
+{
+    volatile std::atomic<int*> obj1;
+    std::atomic<int*> obj2;
+    obj1 = obj2; // expected-error {{overload resolution selected deleted operator '='}}
+}

diff  --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.volatile.verify.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.volatile.verify.cpp
new file mode 100644
index 000000000000..58d71ecd11d7
--- /dev/null
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.req/copy.assign.volatile.verify.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// 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>
+
+// std::atomic
+
+// atomic& operator=( const atomic& ) volatile = delete;
+
+#include <atomic>
+
+int main(int, char**)
+{
+  volatile std::atomic<int> obj1;
+  std::atomic<int> obj2;
+  obj1 = obj2; // expected-error {{overload resolution selected deleted operator '='}}
+}