[libcxx] std::atomic default constructor

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

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

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

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

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

atomic.diff (6.06 KB)

Hi Stephan,

Sorry for the delay in reviewing this. I questioned every decision you made, and every time came up with the same answer you did. Nice job. :slight_smile:

My only complaint is that there's no entry for you in CREDITS.TXT.

Please feel free to commit this, or if you would prefer, I will.

Thanks!
Howard

Hi Stephan,

Sorry for the delay in reviewing this. I questioned every decision you made, and every time came up with the same answer you did. Nice job. :slight_smile:

Thanks!

My only complaint is that there's no entry for you in CREDITS.TXT.

You could use something like the following:

N: Stephan Tolksdorf
E: st@quanttec.com
D: Minor <atomic> fix

Please feel free to commit this, or if you would prefer, I will.

Please commit, I don't have commit rights.

Thank you for the review and the committing!

- Stephan