[atomic] Structs with default constructor
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
On Monday 07 July 2014 09:38:11 Marcel Raad wrote:
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
Yes, this is the property of the current implementation. The type has to be trivial (3.9/9). The previous versions compiled with types with non-trivial default constructors but the constructors were not actually called.
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
Yes, this is the property of the current implementation. The type has to be trivial (3.9/9). The previous versions compiled with types with non-trivial default constructors but the constructors were not actually called.
andrey, the standard requires that the type is trivially copyable, but it allows non-trivial constructors. this should be fixed for the 1.56 release, especially as it breaks existing code (including boost.lockfree). tim
On Monday 07 July 2014 14:23:13 Tim Blechmann wrote:
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
Yes, this is the property of the current implementation. The type has to be trivial (3.9/9). The previous versions compiled with types with non-trivial default constructors but the constructors were not actually called.
andrey, the standard requires that the type is trivially copyable, but it allows non-trivial constructors. this should be fixed for the 1.56 release, especially as it breaks existing code (including boost.lockfree).
Yes, I know the standard requires the type to be trivially copyable. My point is that the code did not support this previously and silently misbehaved, and now it explicitly fails to compile. The code was broken long before 1.56. Implementing support for non-trivial default constructors is possible but tricky - mostly because we have to explicitly call the default constructor on the storage, and while doing this we have to deal with alignment issues. Also I don't know if we should also explicitly call the destructor. I'm willing to resolve this at some point, just not sure that 1.56 is the deadline I'm able to target.
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
Yes, this is the property of the current implementation. The type has to be trivial (3.9/9). The previous versions compiled with types with non-trivial default constructors but the constructors were not actually called.
andrey, the standard requires that the type is trivially copyable, but it allows non-trivial constructors. this should be fixed for the 1.56 release, especially as it breaks existing code (including boost.lockfree).
Yes, I know the standard requires the type to be trivially copyable. My point is that the code did not support this previously and silently misbehaved, and now it explicitly fails to compile. The code was broken long before 1.56.
it used to work flawless with the tagged_ptr/tagged_index structs in boost.lockfree, though this did not touch any borderline cases, which require special care for alignment.
Implementing support for non-trivial default constructors is possible but tricky - mostly because we have to explicitly call the default constructor on the storage, and while doing this we have to deal with alignment issues. Also I don't know if we should also explicitly call the destructor. I'm willing to resolve this at some point, just not sure that 1.56 is the deadline I'm able to target.
this will lead to the point that boost.lockfree is broken on the clang toolchain, because the issue which you introduced is *exactly* the same issue as with std::atomic in libc++ [1]. i know more than one project for which this issue is a showstopper ... [1] http://llvm.org/bugs/show_bug.cgi?id=18097
On Monday 07 July 2014 15:53:21 Tim Blechmann wrote:
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
Yes, this is the property of the current implementation. The type has to be trivial (3.9/9). The previous versions compiled with types with non-trivial default constructors but the constructors were not actually called.
andrey, the standard requires that the type is trivially copyable, but it allows non-trivial constructors. this should be fixed for the 1.56 release, especially as it breaks existing code (including boost.lockfree).
Yes, I know the standard requires the type to be trivially copyable. My point is that the code did not support this previously and silently misbehaved, and now it explicitly fails to compile. The code was broken long before 1.56. it used to work flawless with the tagged_ptr/tagged_index structs in boost.lockfree, though this did not touch any borderline cases, which require special care for alignment.
Implementing support for non-trivial default constructors is possible but tricky - mostly because we have to explicitly call the default constructor on the storage, and while doing this we have to deal with alignment issues. Also I don't know if we should also explicitly call the destructor. I'm willing to resolve this at some point, just not sure that 1.56 is the deadline I'm able to target.
this will lead to the point that boost.lockfree is broken on the clang toolchain, because the issue which you introduced is *exactly* the same issue as with std::atomic in libc++ [1].
i know more than one project for which this issue is a showstopper ...
I didn't see that Boost.Lockfree was broken. Ok, I'll try to resolve this before the release.
On Monday 07 July 2014 18:17:14 you wrote:
On Monday 07 July 2014 15:53:21 Tim Blechmann wrote:
Boost.Atomic 1.55 accepted structs with a user-defined default constructor as template argument, the version from the 1.56 test build doesn't (because of the union_cast). Is this by design? I could be wrong, but I thought the only requirement for structs to use them in atomics was that they were trivially coyable.
Yes, this is the property of the current implementation. The type has to be trivial (3.9/9). The previous versions compiled with types with non-trivial default constructors but the constructors were not actually called.
andrey, the standard requires that the type is trivially copyable, but it allows non-trivial constructors. this should be fixed for the 1.56 release, especially as it breaks existing code (including boost.lockfree).
Yes, I know the standard requires the type to be trivially copyable. My point is that the code did not support this previously and silently misbehaved, and now it explicitly fails to compile. The code was broken long before 1.56.
it used to work flawless with the tagged_ptr/tagged_index structs in boost.lockfree, though this did not touch any borderline cases, which require special care for alignment.
Implementing support for non-trivial default constructors is possible but tricky - mostly because we have to explicitly call the default constructor on the storage, and while doing this we have to deal with alignment issues. Also I don't know if we should also explicitly call the destructor. I'm willing to resolve this at some point, just not sure that 1.56 is the deadline I'm able to target.
this will lead to the point that boost.lockfree is broken on the clang toolchain, because the issue which you introduced is *exactly* the same issue as with std::atomic in libc++ [1].
i know more than one project for which this issue is a showstopper ...
I didn't see that Boost.Lockfree was broken. Ok, I'll try to resolve this before the release.
https://github.com/boostorg/atomic/commit/4dee330229c4f54b157ebde9dbd301bb33... Turned out to be simpler than I anticipated.
Andrey Semashev
https://github.com/boostorg/atomic/commit/4dee330229c4f54b157ebde9dbd301bb33 e64f2c
Turned out to be simpler than I anticipated.
I verified that it works as expected with my code in VS2013. Thanks a lot!
this will lead to the point that boost.lockfree is broken on the clang toolchain, because the issue which you introduced is *exactly* the same issue as with std::atomic in libc++ [1].
i know more than one project for which this issue is a showstopper ...
I didn't see that Boost.Lockfree was broken. Ok, I'll try to resolve this before the release.
https://github.com/boostorg/atomic/commit/4dee330229c4f54b157ebde9dbd301bb33...
Turned out to be simpler than I anticipated.
thanks a lot!!!!
participants (3)
-
Andrey Semashev -
Marcel Raad -
Tim Blechmann