[cfe-dev] [libcxx] std::atomic default constructor
Stephan Tolksdorf
st at quanttec.com
Fri Apr 26 14:52:37 PDT 2013
I've attached a diff that fixes the issue in the <atomic> header and
adds corresponding tests. I've used macros to fall back to a
user-provided default constructor if _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
(though I suspect that there won't be many users defining that macro).
The tests use placement new to check that atomic values get properly
zero-initialized. I had to modify the atomic_is_lock_free test, because
"default initialization of an object of const type 'const A' (aka 'const
atomic<int>') requires a user-provided default constructor".
- Stephan
On 26.04.13 17:21, Howard Hinnant wrote:
> I do not know if it will be in time or not for the next clang release. I would be happy review such a patch. I'd prefer a patch that switches on cxx_defaulted_functions, though I note that <atomic> doesn't compile at all in C++03 mode anyway. So the only advantage would be that <atomic> might work on some hypothetical compiler that had cxx_atomic but not cxx_defaulted_functions. I haven't investigated to know if this is a possibility.
>
> Howard
>
> On Apr 26, 2013, at 6:46 AM, Stephan Tolksdorf <st at quanttec.com> wrote:
>
>> Hi Howard,
>>
>> If I prepare a patch for the <atomic> header, can this still be fixed in time for the next Clang release? Or maybe you already have this on your todo list? It seems like a relatively ugly correctness issue that should be easy to fix.
>>
>> Best regards,
>> Stephan
>>
>> On 15.03.13, Howard Hinnant wrote:
>>> Looks like a bug that needs fixing. When I wrote <atomic>, = default wasn't implemented yet. The entire library needs to be scanned for this issue. And the fix needs to be macro'd up so that we don't break C++03 mode more than it already is.
>>>
>>> Howard
>>>
>>> On Mar 15, 2013, at 3:50 PM, Stephan Tolksdorf <st at quanttec.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> Is there a reason that the default constructors for the std::atomic types in libc++'s <atomic> are not explicitly defaulted? In the header the "= default" is commented out and instead a trivial constructor is explicitly defined. This leads to non-standard behaviour, because value initialization doesn't zero-initialize the atomic value.
>>>>
>>>> For example, the default constructor of the following struct Test does not zero-initialize atomicValue as expected:
>>>>
>>>> struct Test {
>>>> int value{}; // is zero-initialized
>>>> std::atomic<int> atomicValue{}; // is not zero-initialized
>>>> };
>>>>
>>>> Best regards,
>>>> Stephan
>>>>
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>>
>>>
>>
>>
>
>
-------------- next part --------------
Index: test/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp
===================================================================
--- test/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp (revision 180617)
+++ test/atomics/atomics.types.operations/atomics.types.operations.req/atomic_is_lock_free.pass.cpp (working copy)
@@ -24,10 +24,10 @@
test()
{
typedef std::atomic<T> A;
- const A ct;
- bool b1 = std::atomic_is_lock_free(&ct);
- const volatile A cvt;
- bool b2 = std::atomic_is_lock_free(&cvt);
+ A t;
+ bool b1 = std::atomic_is_lock_free(static_cast<const A*>(&t));
+ volatile A vt;
+ bool b2 = std::atomic_is_lock_free(static_cast<const volatile A*>(&vt));
}
struct A
Index: test/atomics/atomics.flag/default.pass.cpp
===================================================================
--- test/atomics/atomics.flag/default.pass.cpp (revision 180617)
+++ test/atomics/atomics.flag/default.pass.cpp (working copy)
@@ -14,9 +14,18 @@
// atomic_flag() = default;
#include <atomic>
+#include <new>
#include <cassert>
int main()
{
std::atomic_flag f;
+
+ {
+ typedef std::atomic_flag A;
+ _ALIGNAS_TYPE(A) char storage[sizeof(A)] = {1};
+ A& zero = *new (storage) A();
+ assert(!zero.test_and_set());
+ zero.~A();
+ }
}
Index: test/atomics/atomics.types.generic/bool.pass.cpp
===================================================================
--- test/atomics/atomics.types.generic/bool.pass.cpp (revision 180617)
+++ test/atomics/atomics.types.generic/bool.pass.cpp (working copy)
@@ -50,6 +50,7 @@
// typedef atomic<bool> atomic_bool;
#include <atomic>
+#include <new>
#include <cassert>
int main()
@@ -219,4 +220,11 @@
assert((obj = true) == true);
assert(obj == true);
}
+ {
+ typedef std::atomic<bool> A;
+ _ALIGNAS_TYPE(A) char storage[sizeof(A)] = {1};
+ A& zero = *new (storage) A();
+ assert(zero == false);
+ zero.~A();
+ }
}
Index: test/atomics/atomics.types.generic/address.pass.cpp
===================================================================
--- test/atomics/atomics.types.generic/address.pass.cpp (revision 180617)
+++ test/atomics/atomics.types.generic/address.pass.cpp (working copy)
@@ -66,6 +66,7 @@
// };
#include <atomic>
+#include <new>
#include <type_traits>
#include <cassert>
@@ -112,6 +113,13 @@
assert(obj == T(5*sizeof(X)));
assert((obj -= std::ptrdiff_t(3)) == T(2*sizeof(X)));
assert(obj == T(2*sizeof(X)));
+
+ {
+ _ALIGNAS_TYPE(A) char storage[sizeof(A)] = {23};
+ A& zero = *new (storage) A();
+ assert(zero == 0);
+ zero.~A();
+ }
}
template <class A, class T>
Index: test/atomics/atomics.types.generic/integral.pass.cpp
===================================================================
--- test/atomics/atomics.types.generic/integral.pass.cpp (revision 180617)
+++ test/atomics/atomics.types.generic/integral.pass.cpp (working copy)
@@ -85,6 +85,7 @@
// };
#include <atomic>
+#include <new>
#include <cassert>
template <class A, class T>
@@ -143,6 +144,13 @@
assert(obj == T(7));
assert((obj ^= T(0xF)) == T(8));
assert(obj == T(8));
+
+ {
+ _ALIGNAS_TYPE(A) char storage[sizeof(A)] = {23};
+ A& zero = *new (storage) A();
+ assert(zero == 0);
+ zero.~A();
+ }
}
template <class A, class T>
Index: include/atomic
===================================================================
--- include/atomic (revision 180617)
+++ include/atomic (working copy)
@@ -622,7 +622,12 @@
{return __c11_atomic_compare_exchange_strong(&__a_, &__e, __d, __m, __m);}
_LIBCPP_INLINE_VISIBILITY
- __atomic_base() _NOEXCEPT {} // = default;
+#ifndef _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
+ __atomic_base() _NOEXCEPT = default;
+#else
+ __atomic_base() _NOEXCEPT : __a_() {}
+#endif // _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
+
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : __a_(__d) {}
#ifndef _LIBCPP_HAS_NO_DELETED_FUNCTIONS
@@ -645,7 +650,7 @@
{
typedef __atomic_base<_Tp, false> __base;
_LIBCPP_INLINE_VISIBILITY
- __atomic_base() _NOEXCEPT {} // = default;
+ __atomic_base() _NOEXCEPT _LIBCPP_DEFAULT
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR __atomic_base(_Tp __d) _NOEXCEPT : __base(__d) {}
@@ -726,7 +731,7 @@
{
typedef __atomic_base<_Tp> __base;
_LIBCPP_INLINE_VISIBILITY
- atomic() _NOEXCEPT {} // = default;
+ atomic() _NOEXCEPT _LIBCPP_DEFAULT
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR atomic(_Tp __d) _NOEXCEPT : __base(__d) {}
@@ -746,7 +751,7 @@
{
typedef __atomic_base<_Tp*> __base;
_LIBCPP_INLINE_VISIBILITY
- atomic() _NOEXCEPT {} // = default;
+ atomic() _NOEXCEPT _LIBCPP_DEFAULT
_LIBCPP_INLINE_VISIBILITY
_LIBCPP_CONSTEXPR atomic(_Tp* __d) _NOEXCEPT : __base(__d) {}
@@ -1367,7 +1372,12 @@
{__c11_atomic_store(&__a_, false, __m);}
_LIBCPP_INLINE_VISIBILITY
- atomic_flag() _NOEXCEPT {} // = default;
+#ifndef _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
+ atomic_flag() _NOEXCEPT = default;
+#else
+ atomic_flag() _NOEXCEPT : __a_() {}
+#endif // _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
+
_LIBCPP_INLINE_VISIBILITY
atomic_flag(bool __b) _NOEXCEPT : __a_(__b) {}
Index: include/__config
===================================================================
--- include/__config (revision 180617)
+++ include/__config (working copy)
@@ -212,7 +212,9 @@
# define _LIBCPP_NORETURN __attribute__ ((noreturn))
#endif
+#if !(__has_feature(cxx_defaulted_functions))
#define _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
+#endif // !(__has_feature(cxx_defaulted_functions))
#if !(__has_feature(cxx_deleted_functions))
#define _LIBCPP_HAS_NO_DELETED_FUNCTIONS
@@ -418,6 +420,12 @@
#define _LIBCPP_CONSTEXPR constexpr
#endif
+#ifdef _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS
+#define _LIBCPP_DEFAULT {}
+#else
+#define _LIBCPP_DEFAULT = default;
+#endif
+
#ifdef __GNUC__
#define _NOALIAS __attribute__((malloc))
#else
More information about the cfe-dev
mailing list