[libcxx] optional: Implement LWG 2900 and P0602

[CaseyCarter]

Summary

• Implement LWG 2900 "The copy and move constructors of optional are not constexpr" by injecting base classes that differentiate between trivial/non-trivial implementation of each copy/move constructor/assignment.

• Implement P0602 "variant and optional should propagate copy/move triviality" with slightly modified / more correct constraints that I have "upstreamed" to the Zhihao for inclusion in the next revision of the paper.

[CaseyCarter]

Assigning an empty optional to a non-empty optional needs to destroy the contained value, so we need to also require is_trivially_destructible to implement trivial assignments.

[CaseyCarter]

• Define new macro _MSVC_STL_VER in msvc_stdlib_force_include.hpp to indicate that the VC++ standard library is being tested.
• Implement test coverage for P0602 "variant and optional should propagate copy/move triviality".

[CaseyCarter]

Fix comment typo in optional.object.ctor/move.pass.cpp

[CaseyCarter]

Update libc++-specific special member test for extended P0602 implementation.

[CaseyCarter]

Fix a missing indent in the msvc_stdlib_force_include.hpp change.

[CaseyCarter]

Polish: clarify static_assert messages, DeMorgan a clause so it more precisely reflects the static_assert message.

[EricWF]

So Itanium ABI has this quirk where trivial types are passed using different conventions than non-trivial types. This means changing the triviality of std::optional for any instantiation is potentially ABI breaking. I'll need to do more investigation to find out how this will affect libc++, and if we can take this change.

[CaseyCarter]

In D32385#745771, @EricWF wrote:

So Itanium ABI has this quirk where trivial types are passed using different conventions than non-trivial types. This means changing the triviality of std::optional for any instantiation is potentially ABI breaking. I'll need to do more investigation to find out how this will affect libc++, and if we can take this change.

libc++ committed to ABI stability for optional, a new feature of C++17, before the standard shipped?

[EricWF]

libc++ committed to ABI stability for optional, a new feature of C++17, before the standard shipped?

Libc++ doesn't commit to anything, the vendors such as Apple and FreeBSD do. I don't think either of them
have shipped a release with std::optional yet, but I need to be sure they don't have ABI obligations before I break it.

I certainly agree that nobody should be expecting ABI stability for features not yet standardized, and I should
take steps to ensure vendors understand that.

[CaseyCarter]

Fix merge conflicts.

[CaseyCarter]

Incorporate the libcxx test tree special_member_gen test into the std test, making my per-SMF triviality tests unnecessary.

[EricWF]

LGTM. I'll deal with ensuring all vendors are willing to take this ABI break (or that they avoid it).

include/optional

void __assign_from(_That&& __opt)
             if (has_value())
                 reset();
             else
                 __construct(_VSTD::forward<_That>(__opt).__get());
         }
     }
 };
 
-template <class _Tp, bool = is_trivially_copyable<_Tp>::value>
-struct __optional_storage;
-
-template <class _Tp>
-struct __optional_storage<_Tp, true> : __optional_storage_base<_Tp>
+template <class _Tp, bool = is_trivially_copy_constructible<_Tp>::value>
+struct __optional_copy_base : __optional_storage_base<_Tp>
 {
     using __optional_storage_base<_Tp>::__optional_storage_base;
 };
 
 template <class _Tp>
-struct __optional_storage<_Tp, false> : __optional_storage_base<_Tp>
+struct __optional_copy_base<_Tp, false> : __optional_storage_base<_Tp>
 {
-    using value_type = _Tp;
     using __optional_storage_base<_Tp>::__optional_storage_base;
 
     _LIBCPP_INLINE_VISIBILITY
-    __optional_storage() = default;
+    __optional_copy_base() = default;
 
     _LIBCPP_INLINE_VISIBILITY
-    __optional_storage(const __optional_storage& __opt)
+    __optional_copy_base(const __optional_copy_base& __opt)
     {
         this->__construct_from(__opt);
     }
 
     _LIBCPP_INLINE_VISIBILITY
-    __optional_storage(__optional_storage&& __opt)
+    __optional_copy_base(__optional_copy_base&&) = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_copy_base& operator=(const __optional_copy_base&) = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_copy_base& operator=(__optional_copy_base&&) = default;
+};
+
+template <class _Tp, bool = is_trivially_move_constructible<_Tp>::value>
+struct __optional_move_base : __optional_copy_base<_Tp>
+{
+    using __optional_copy_base<_Tp>::__optional_copy_base;
+};
+
+template <class _Tp>
+struct __optional_move_base<_Tp, false> : __optional_copy_base<_Tp>
+{
+    using value_type = _Tp;
+    using __optional_copy_base<_Tp>::__optional_copy_base;
+
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_base() = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_base(const __optional_move_base&) = default;
+
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_base(__optional_move_base&& __opt)
         noexcept(is_nothrow_move_constructible_v<value_type>)
     {
         this->__construct_from(_VSTD::move(__opt));
     }
 
     _LIBCPP_INLINE_VISIBILITY
-    __optional_storage& operator=(const __optional_storage& __opt)
+    __optional_move_base& operator=(const __optional_move_base&) = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_base& operator=(__optional_move_base&&) = default;
+};
+
+template <class _Tp, bool =
+    is_trivially_destructible<_Tp>::value &&
+    is_trivially_copy_constructible<_Tp>::value &&
+    is_trivially_copy_assignable<_Tp>::value>
+struct __optional_copy_assign_base : __optional_move_base<_Tp>
+{
+    using __optional_move_base<_Tp>::__optional_move_base;
+};
+
+template <class _Tp>
+struct __optional_copy_assign_base<_Tp, false> : __optional_move_base<_Tp>
+{
+    using __optional_move_base<_Tp>::__optional_move_base;
+
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_copy_assign_base() = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_copy_assign_base(const __optional_copy_assign_base&) = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_copy_assign_base(__optional_copy_assign_base&&) = default;
+
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_copy_assign_base& operator=(const __optional_copy_assign_base& __opt)
     {
         this->__assign_from(__opt);
         return *this;
     }
 
     _LIBCPP_INLINE_VISIBILITY
-    __optional_storage& operator=(__optional_storage&& __opt)
+    __optional_copy_assign_base& operator=(__optional_copy_assign_base&&) = default;
+};
+
+template <class _Tp, bool =
+    is_trivially_destructible<_Tp>::value &&
+    is_trivially_move_constructible<_Tp>::value &&
+    is_trivially_move_assignable<_Tp>::value>
+struct __optional_move_assign_base : __optional_copy_assign_base<_Tp>
+{
+    using __optional_copy_assign_base<_Tp>::__optional_copy_assign_base;
+};
+
+template <class _Tp>
+struct __optional_move_assign_base<_Tp, false> : __optional_copy_assign_base<_Tp>
+{
+    using value_type = _Tp;
+    using __optional_copy_assign_base<_Tp>::__optional_copy_assign_base;
+
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_assign_base() = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_assign_base(const __optional_move_assign_base& __opt) = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_assign_base(__optional_move_assign_base&&) = default;
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_assign_base& operator=(const __optional_move_assign_base&) = default;
+
+    _LIBCPP_INLINE_VISIBILITY
+    __optional_move_assign_base& operator=(__optional_move_assign_base&& __opt)
         noexcept(is_nothrow_move_assignable_v<value_type> &&
                  is_nothrow_move_constructible_v<value_type>)
     {
         this->__assign_from(_VSTD::move(__opt));
         return *this;
     }
 };
 
 template <class _Tp>
 using __optional_sfinae_ctor_base_t = __sfinae_ctor_base<
     is_copy_constructible<_Tp>::value,
     is_move_constructible<_Tp>::value
 >;
 
 template <class _Tp>
 using __optional_sfinae_assign_base_t = __sfinae_assign_base<
     (is_copy_constructible<_Tp>::value && is_copy_assignable<_Tp>::value),
     (is_move_constructible<_Tp>::value && is_move_assignable<_Tp>::value)
 >;
 
 template <class _Tp>
 class optional
-    : private __optional_storage<_Tp>
+    : private __optional_move_assign_base<_Tp>
     , private __optional_sfinae_ctor_base_t<_Tp>
     , private __optional_sfinae_assign_base_t<_Tp>
 {
-    using __base = __optional_storage<_Tp>;
+    using __base = __optional_move_assign_base<_Tp>;
 public:
     using value_type = _Tp;
 
 private:
      // Disable the reference extension using this static assert.
     static_assert(!is_same_v<value_type, in_place_t>,
         "instantiation of optional with in_place_t is ill-formed");
     static_assert(!is_same_v<__uncvref_t<value_type>, nullopt_t>,

test/libcxx/utilities/optional/optional.object/special_member_gen.pass.cpp

 #include <cassert>
 
 #include "archetypes.hpp"
 
 template <class T>
 struct SpecialMemberTest {
     using O = std::optional<T>;
 
-    template <template <class> class TestMF>
-    static constexpr bool check_same() {
-        return TestMF<O>::value == TestMF<T>::value;
-    }
-
-    // Test that optional inherits the correct trivial/non-trivial members
-    static_assert(check_same<std::is_trivially_destructible>(), "");
-    static_assert(check_same<std::is_trivially_copyable>(), "");
+    static_assert(std::is_trivially_destructible_v<O> ==
+        std::is_trivially_destructible_v<T>,
+        "optional<T> is trivially destructible if and only if T is.");
+    static_assert(std::is_trivially_copy_constructible_v<O> ==
+        std::is_trivially_copy_constructible_v<T>,
+        "optional<T> is trivially copy constructible if and only if T is.");
+    static_assert(std::is_trivially_move_constructible_v<O> ==
+        std::is_trivially_move_constructible_v<T> ||
+        (!std::is_move_constructible_v<T> && std::is_trivially_copy_constructible_v<T>),
+        "optional<T> is trivially move constructible if T is trivially move constructible, "
+        "or if T is trivially copy constructible and is not move constructible.");
+    static_assert(std::is_trivially_copy_assignable_v<O> ==
+        (std::is_trivially_destructible_v<T> &&
+         std::is_trivially_copy_constructible_v<T> &&
+         std::is_trivially_copy_assignable_v<T>),
+        "optional<T> is trivially copy assignable if and only if T is trivially destructible, "
+        "trivially copy constructible, and trivially copy assignable.");
+    static_assert(std::is_trivially_move_assignable_v<O> ==
+        (std::is_trivially_destructible_v<T> &&
+         ((std::is_trivially_move_constructible_v<T> && std::is_trivially_move_assignable_v<T>) ||
+          ((!std::is_move_constructible_v<T> || !std::is_move_assignable_v<T>) &&
+           std::is_trivially_copy_constructible_v<T> && std::is_trivially_copy_assignable_v<T>))),
+        "optional<T> is trivially move assignable if T is trivially destructible, and either "
+        "(1) trivially move constructible and trivially move assignable, or "
+        "(2) not move constructible or not move assignable, and "
+        "trivially copy constructible and trivially copy assignable.");
 };
 
 template <class ...Args> static void sink(Args&&...) {}
 
 template <class ...TestTypes>
 struct DoTestsMetafunction {
     DoTestsMetafunction() { sink(SpecialMemberTest<TestTypes>{}...); }
 };

test/std/utilities/optional/optional.object/optional.object.assign/copy.pass.cpp

 
 template <class Tp>
 constexpr bool assign_value(optional<Tp>&& lhs) {
     const optional<Tp> rhs(101);
     lhs = rhs;
     return lhs.has_value() && rhs.has_value() && *lhs == *rhs;
 }
 
+
+template<class T>
+constexpr bool triviality_test =
+    std::is_trivially_copy_assignable<optional<T>>::value ==
+        (std::is_trivially_destructible<T>::value &&
+         std::is_trivially_copy_constructible<T>::value &&
+         std::is_trivially_copy_assignable<T>::value);
+
+void test_triviality_extension() {
+#if defined(_LIBCPP_VER) || defined(_MSVC_STL_VER)
+    static_assert(triviality_test<int>, "");
+    static_assert(triviality_test<optional<int>>, "");
+    static_assert(triviality_test<TrivialTestTypes::TestType>, "");
+    static_assert(triviality_test<TestTypes::TestType>, "");
+    static_assert(triviality_test<X>, "");
+#endif
+}
+
 int main()
 {
     {
         using O = optional<int>;
         LIBCPP_STATIC_ASSERT(assign_empty(O{42}), "");
         LIBCPP_STATIC_ASSERT(assign_value(O{42}), "");
         assert(assign_empty(O{42}));
         assert(assign_value(O{42}));
#ifndef TEST_HAS_NO_EXCEPTIONS
         }
         catch (int i)
         {
             assert(i == 6);
             assert(static_cast<bool>(opt) == false);
         }
     }
 #endif
+
+    test_triviality_extension();
 }

test/std/utilities/optional/optional.object/optional.object.assign/move.pass.cpp

struct X
     ~X() { assert(alive > 0); --alive; }
 };
 
 struct Y {};
 
 bool X::throw_now = false;
 int X::alive = 0;
 
+template<class T>
+constexpr bool triviality_test =
+    std::is_trivially_move_assignable<optional<T>>::value ==
+        (std::is_trivially_destructible<T>::value &&
+         std::is_trivially_move_constructible<T>::value &&
+         std::is_trivially_move_assignable<T>::value);
+
+void test_triviality_extension() {
+#if defined(_LIBCPP_VER) || defined(_MSVC_STL_VER)
+    static_assert(triviality_test<int>, "");
+    static_assert(triviality_test<optional<int>>, "");
+    static_assert(triviality_test<TrivialTestTypes::TestType>, "");
+    static_assert(triviality_test<TestTypes::TestType>, "");
+    static_assert(triviality_test<X>, "");
+    static_assert(triviality_test<Y>, "");
+#endif
+}
+
 int main()
 {
     {
         static_assert(std::is_nothrow_move_assignable<optional<int>>::value, "");
         optional<int> opt;
         constexpr optional<int> opt2;
         opt = std::move(opt2);
         static_assert(static_cast<bool>(opt2) == false, "");
#ifndef TEST_HAS_NO_EXCEPTIONS
     }
     assert(X::alive == 0);
 #endif // TEST_HAS_NO_EXCEPTIONS
     {
         static_assert(std::is_nothrow_move_assignable<optional<Y>>::value, "");
     }
     {
         struct ThrowsMove {
             ThrowsMove() noexcept {}
             ThrowsMove(ThrowsMove const&) noexcept {}
             ThrowsMove(ThrowsMove &&) noexcept(false) {}
             ThrowsMove& operator=(ThrowsMove const&) noexcept { return *this; }
             ThrowsMove& operator=(ThrowsMove &&) noexcept { return *this; }
         };
         static_assert(!std::is_nothrow_move_assignable<optional<ThrowsMove>>::value, "");
         struct ThrowsMoveAssign {
             ThrowsMoveAssign() noexcept {}
             ThrowsMoveAssign(ThrowsMoveAssign const&) noexcept {}
             ThrowsMoveAssign(ThrowsMoveAssign &&) noexcept {}
             ThrowsMoveAssign& operator=(ThrowsMoveAssign const&) noexcept { return *this; }
             ThrowsMoveAssign& operator=(ThrowsMoveAssign &&) noexcept(false) { return *this; }
         };
         static_assert(!std::is_nothrow_move_assignable<optional<ThrowsMoveAssign>>::value, "");
         struct NoThrowMove {
             NoThrowMove() noexcept(false) {}
             NoThrowMove(NoThrowMove const&) noexcept(false) {}
             NoThrowMove(NoThrowMove &&) noexcept {}
             NoThrowMove& operator=(NoThrowMove const&) noexcept { return *this; }
             NoThrowMove& operator=(NoThrowMove&&) noexcept { return *this; }
         };
         static_assert(std::is_nothrow_move_assignable<optional<NoThrowMove>>::value, "");
     }
+
+    test_triviality_extension();
 }

test/std/utilities/optional/optional.object/optional.object.ctor/copy.pass.cpp

constexpr bool constexpr_test(InitArgs&&... args)
     optional<T> lhs = rhs;
     return (lhs.has_value() == rhs.has_value()) &&
            (lhs.has_value() ? *lhs == *rhs : true);
 }
 
 void test_throwing_ctor() {
 #ifndef TEST_HAS_NO_EXCEPTIONS
     struct Z {
         Z() : count(0) {}
         Z(Z const& o) : count(o.count + 1)
         { if (count == 2) throw 6; }
         int count;
     };
     const Z z;
     const optional<Z> rhs(z);
     try
     {
         optional<Z> lhs(rhs);
         assert(false);
     }
#if defined(_LIBCPP_VERSION) && 0 // FIXME these extensions are currently disabled.
     assert(T::destroyed == 1);
     {
         static_assert(!std::is_copy_constructible<std::optional<T&&>>::value, "");
         static_assert(!std::is_copy_constructible<std::optional<T const&&>>::value, "");
     }
 #endif
 }
 
+template<class T>
+constexpr bool triviality_test =
+    std::is_trivially_copy_constructible<optional<T>>::value ==
+        std::is_trivially_copy_constructible<T>::value;
+
+void test_triviality_extension() {
+#if defined(_LIBCPP_VER) || defined(_MSVC_STL_VER)
+    static_assert(triviality_test<int>, "");
+    static_assert(triviality_test<optional<int>>, "");
+    static_assert(triviality_test<TestTypes::TestType>, "");
+    static_assert(triviality_test<ConstexprTestTypes::TestType>, "");
+    static_assert(triviality_test<NonTrivialTypes::AllCtors>, "");
+    static_assert(triviality_test<TrivialTestTypes::TestType>, "");
+#endif
+}
+
 int main()
 {
     test<int>();
     test<int>(3);
     static_assert(constexpr_test<int>(), "" );
     static_assert(constexpr_test<int>(3), "" );
 
     {
TestTypes::TestType::reset();
         test<TestType>();
         test<TestType>(42);
     }
     {
         using namespace TrivialTestTypes;
         test<TestType>();
         test<TestType>(42);
     }
-    {
-        test_throwing_ctor();
-    }
-    {
-        test_reference_extension();
-    }
+    test_triviality_extension();
     {
     constexpr std::optional<int> o1{4};
     constexpr std::optional<int> o2 = o1;
     static_assert( *o2 == 4, "" );
     }
 }

test/std/utilities/optional/optional.object/optional.object.ctor/move.pass.cpp

constexpr bool constexpr_test(InitArgs&&... args)
     optional<T> lhs = std::move(rhs);
     return (lhs.has_value() == orig.has_value()) &&
            (lhs.has_value() ? *lhs == *orig : true);
 }
 
 void test_throwing_ctor() {
 #ifndef TEST_HAS_NO_EXCEPTIONS
     struct Z {
         Z() : count(0) {}
         Z(Z&& o) : count(o.count + 1)
         { if (count == 2) throw 6; }
         int count;
     };
     Z z;
     optional<Z> rhs(std::move(z));
     try
     {
         optional<Z> lhs(std::move(rhs));
         assert(false);
     }
#if defined(_LIBCPP_VERSION) && 0 // FIXME these extensions are currently disabled.
     assert(T::destroyed == 1);
     {
         static_assert(!std::is_copy_constructible<std::optional<T&&>>::value, "");
         static_assert(!std::is_copy_constructible<std::optional<T const&&>>::value, "");
     }
 #endif
 }
 
+template<class T>
+constexpr bool triviality_test =
+    std::is_trivially_move_constructible<optional<T>>::value ==
+        std::is_trivially_move_constructible<T>::value;
+
+void test_triviality_extension() {
+#if defined(_LIBCPP_VER) || defined(_MSVC_STL_VER)
+    static_assert(triviality_test<int>, "");
+    static_assert(triviality_test<optional<int>>, "");
+    static_assert(triviality_test<TestTypes::TestType>, "");
+    static_assert(triviality_test<ConstexprTestTypes::TestType>, "");
+    static_assert(triviality_test<NonTrivialTypes::AllCtors>, "");
+    static_assert(triviality_test<TrivialTestTypes::TestType>, "");
+#endif
+}
 
 int main()
 {
     test<int>();
     test<int>(3);
     static_assert(constexpr_test<int>(), "" );
     static_assert(constexpr_test<int>(3), "" );
   
TestTypes::TestType::reset();
         test<TestType>();
         test<TestType>(42);
     }
     {
         using namespace TrivialTestTypes;
         test<TestType>();
         test<TestType>(42);
     }
-    {
-        test_throwing_ctor();
-    }
+
+
     {
         struct ThrowsMove {
           ThrowsMove() noexcept(false) {}
           ThrowsMove(ThrowsMove const&) noexcept(false) {}
           ThrowsMove(ThrowsMove &&) noexcept(false) {}
         };
         static_assert(!std::is_nothrow_move_constructible<optional<ThrowsMove>>::value, "");
         struct NoThrowMove {
           NoThrowMove() noexcept(false) {}
           NoThrowMove(NoThrowMove const&) noexcept(false) {}
           NoThrowMove(NoThrowMove &&) noexcept(true) {}
         };
         static_assert(std::is_nothrow_move_constructible<optional<NoThrowMove>>::value, "");
     }
-    {
-        test_reference_extension();
-    }
+
+    test_triviality_extension();
+
     {
     constexpr std::optional<int> o1{4};
     constexpr std::optional<int> o2 = std::move(o1);
     static_assert( *o2 == 4, "" );
     }
 }

test/std/utilities/optional/optional.object/special_member_gen.pass.cpp

struct SpecialMemberTest {
     static_assert(std::is_move_constructible_v<O> ==
         (std::is_copy_constructible_v<T> || std::is_move_constructible_v<T>),
         "optional<T> is move constructible if and only if T is copy or move constructible.");
     static_assert(std::is_copy_assignable_v<O> ==
         (std::is_copy_constructible_v<T> && std::is_copy_assignable_v<T>),
         "optional<T> is copy assignable if and only if T is both copy "
         "constructible and copy assignable.");
     static_assert(std::is_move_assignable_v<O> ==
-        ((std::is_copy_constructible_v<T> && std::is_copy_assignable_v<T>) ||
-         (std::is_move_constructible_v<T> && std::is_move_assignable_v<T>)),
-        "optional<T> is move assignable if and only if T is both move assignable and "
-        "move constructible, or both copy constructible and copy assignable.");
+        ((std::is_move_constructible_v<T> && std::is_move_assignable_v<T>) ||
+         (std::is_copy_constructible_v<T> && std::is_copy_assignable_v<T>)),
+        "optional<T> is move assignable if and only if T is both move constructible and "
+        "move assignable, or both copy constructible and copy assignable.");
 };
 
 template <class ...Args> static void sink(Args&&...) {}
 
 template <class ...TestTypes>
 struct DoTestsMetafunction {
     DoTestsMetafunction() { sink(SpecialMemberTest<TestTypes>{}...); }
 };