- User Since
- Dec 17 2020, 4:00 PM (4 w, 4 d)
Fri, Jan 15
Quick reminder that I don't have commit access, and need someone to commit this patch on my behalf. Thanks!
Thu, Jan 14
Added !defined(__clang__) conditions to static_assert preprocessing. Also added large comment describing the mental gymnastics of the OptionalStorage specialization condition.
I just triaged the gcc 5.4 build break. It appears that std::is_trivially_copy_constructible<T> instantiates the copy constructor (which is... strange). So we can't query a compile-time property without the compiler forcing instantiation of the deleted copy constructor in this case.
Taking a look at the gcc 5.4 failure now.
Tue, Jan 12
@dblaikie, I don't have commit access. Are you the right person to ask to commit this patch on my behalf?
Mon, Jan 11
Properly update the patch to incorporate the separate static_asserts.
Split conditions into individual static_asserts.
Fri, Jan 8
With current change, pre-merge checks pass. I think I've resolved all issues / requests.
Added preprocessing around static_asserts added to OptionalTests.cpp to only include them in recent windows builds.
Tue, Jan 5
Mon, Jan 4
- Expanded the check for the trivial specialization of OptionalStorage
- Formatted the OptionalStorage template list.
- Added a test to ensure that deleting move constructor / assignment will still choose the trivial OptionalStorage specialization.
Dec 18 2020
Clarified comments above deleted volatile copy constructor / assignment in test classes.
any idea why this only fails for MSVC? It'd be good to understand more why this didn't trip up other builds so we can avoid those sort of divergences in the future, perhaps.
- Use std::is_trivially_copyable instead of llvm::is_trivially_copyable in Optional storage template specialization logic.
- Added test cases to OptionalTest.cpp which will fail compilation (verified on MSVC 16.8.3) if new conditions are absent from Optional storage template specialization logic.
Dec 17 2020
I spent some time root causing an llvm::Optional build error on MSVC 16.8.3 today related to using std::is_trivially_copyable to set llvm::is_trivially_copyable<T>::value. It looks like the same failure mentioned in this review.