[libcxx] [test] variant: test coverage for LWG2904 and P0602

[CaseyCarter]

Summary

• Incorporate the variant trivial/constexpr copy/move construct/assign tests from the libcxx tree into the std tree.
• Update tests for LWG2904, conditional on !defined(_LIBCPP_VERSION) (These all have nicely greppable LWG2904 comments.)
• Workaround C1XX's buggy __is_trivially_copyable in the variant tests.

All tests pass on both x86 VC++ / x64 clang-4 libc++.

[CaseyCarter]

Fix a weird corner case in variant's move assignment triviality test.

[CaseyCarter]

Fix missing indent in msvc_stdlib_force_include.hpp

[CaseyCarter]

Fix typos in test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp.

[EricWF]

Could you commit all the changes *except* the trivial extensions tests, and then update this to contain only that?

[CaseyCarter]

The constexpr variant tests were enabled separate commit (https://reviews.llvm.org/rL302158). This differential now contains only the fine-grained SMF triviality extension test.

[mpark]

@CaseyCarter: Do these changes pass with the current version?
Also, have you seen the tests in test/libcxx/utilities/variant/variant.variant?
If yes, do those tests and the ones in this diff overlap at all?
Curious as to how we should merge them.

[CaseyCarter]

In D32671#762840, @mpark wrote:

@CaseyCarter: Do these changes pass with the current version?
Also, have you seen the tests in test/libcxx/utilities/variant/variant.variant?
If yes, do those tests and the ones in this diff overlap at all?
Curious as to how we should merge them.

Disclaimer: I know nothing about the libc++ <variant>, and I didn't realize there were tests in the libcxx tree for variant.

These tests technically "pass" with the current version since they do nothing when testing libc++ - they're currently only enabled when testing the MSVC standard library. I hadn't bothered to try them with libc++ variant before, but when I do they all pass except for triviality_test of copy/move assignment with NoCopy as an alternative. Those results indicate to me that you already implement fine-grained SMF triviality, and the discrepancy with NoCopy is probably due to the lack of LWG2904.

Taking a look at the libcxx tree tests:

• There are SMF sfinae tests in these for (what is now) standard behavior that should be moved into the std tree tests.
• Your test_XXX_basic tests are effectively testing the same behavior as these but with "is constexpr" as a proxy for "is trivial", and using more demonstrative code whereas mine only tests traits.

I think I'd like to integrate those tests into the std tree, if that's ok with you.

[mpark]

Yes, you're right that fine-grained SMF triviality is implemented but LWG 2904 is not yet.
I would love it if you can integrate the currently libcxx tests into std tests!

Thank you :)

[EricWF]

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

389

triviality_test should also compare to an explicitly specified expected result, along with checking for consistency between variant and its input types.

test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp

168

These tests pass with libc++ too, so please add || defined(_LIBCPP_VERSION)

test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp

206

These tests pass with libc++ too, so please add || defined(_LIBCPP_VERSION)

[CaseyCarter]

Massive update: Move the libcxx tree tests into std, merge my LWG2904 differential.

test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp

#ifndef TEST_HAS_NO_EXCEPTIONS
     assert(v1.index() == 2);
     assert(std::get<2>(v1) == "hello");
     assert(v2.index() == 2);
     assert(std::get<2>(v2) == "hello");
   }
 #endif
 }
 
+template<class...Ts>
+constexpr bool triviality_test =
+  std::is_trivially_copy_assignable<std::variant<Ts...>>::value ==
+    std::conjunction<
+      std::is_trivially_destructible<Ts>...,
+      std::is_trivially_copy_constructible<Ts>...,
+      std::is_trivially_copy_assignable<Ts>...>::value;
+
+void test_triviality_extension() {
+#if defined(_MSVC_STL_VER)
+  static_assert(triviality_test<int>, "");
+  static_assert(triviality_test<std::string>, "");
+  static_assert(triviality_test<int, long>, "");
+  static_assert(triviality_test<int, std::string>, "");
+  static_assert(triviality_test<int, NoCopy>, "");
+  static_assert(triviality_test<int, NothrowCopy>, "");
+  static_assert(triviality_test<int, CopyOnly>, "");
+  static_assert(triviality_test<int, MoveOnly>, "");
+  static_assert(triviality_test<int, CopyAssign>, "");
+#endif
+}
 
 int main() {
   test_copy_assignment_empty_empty();
   test_copy_assignment_non_empty_empty();
   test_copy_assignment_empty_non_empty();
   test_copy_assignment_same_index();
   test_copy_assignment_different_index();
   test_copy_assignment_sfinae();
   test_copy_assignment_not_noexcept();
+  test_triviality_extension();
 }

test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp

using MET = MakeEmptyT;
     V &vref = (v1 = std::move(v2));
     assert(&vref == &v1);
     assert(v1.index() == 2);
     assert(std::get<2>(v1) == "hello");
   }
 #endif
 }
 
+template<class...Ts>
+constexpr bool triviality_test =
+  // move assignent of variant<Ts...> is trivial when
+  std::is_trivially_move_assignable<std::variant<Ts...>>::value ==
+    std::conjunction<
+      // All Ts are trivially destructible and
+      std::is_trivially_destructible<Ts>...,
+      // either
+      std::disjunction<
+        // All Ts are trivially move (constructible and assignable) so that
+        // variant's move assignment operator is non-deleted and trivial, or
+        std::conjunction<
+          std::is_trivially_move_constructible<Ts>...,
+          std::is_trivially_move_assignable<Ts>...>,
+        // At least one of the Ts is not move (constructible or assignable) so
+        // that variant's move assignment operator is *implicitly* deleted but
+        // all of the Ts are trivially *copy* (constructible and deletable) so
+        // that move assignment actually invokes the trivial copy assignment
+        // operator.
+        std::conjunction<
+          std::disjunction<
+            std::negation<std::is_move_constructible<Ts>>...,
+            std::negation<std::is_move_assignable<Ts>>...>,
+          std::is_trivially_copy_constructible<Ts>...,
+          std::is_trivially_copy_assignable<Ts>...>>>::value;
+
+void test_triviality_extension() {
+#if defined(_MSVC_STL_VER)
+  struct TrivialCopyNontrivialMove {
+    TrivialCopyNontrivialMove(TrivialCopyNontrivialMove const&) = default;
+    TrivialCopyNontrivialMove(TrivialCopyNontrivialMove&&) noexcept {}
+    TrivialCopyNontrivialMove& operator=(TrivialCopyNontrivialMove const&) = default;
+    TrivialCopyNontrivialMove& operator=(TrivialCopyNontrivialMove&&) noexcept {
+      return *this;
+    }
+  };
+
+  static_assert(triviality_test<int>, "");
+  static_assert(triviality_test<std::string>, "");
+  static_assert(triviality_test<int, long>, "");
+  static_assert(triviality_test<int, std::string>, "");
+  static_assert(triviality_test<int, NoCopy>, "");
+  static_assert(triviality_test<int, CopyOnly>, "");
+  static_assert(triviality_test<int, MoveOnly>, "");
+  static_assert(triviality_test<int, MoveAssign>, "");
+  static_assert(triviality_test<int, MoveAssignOnly>, "");
+  static_assert(triviality_test<int, TrivialCopyNontrivialMove>, "");
+#endif
+}
+
 int main() {
   test_move_assignment_empty_empty();
   test_move_assignment_non_empty_empty();
   test_move_assignment_empty_non_empty();
   test_move_assignment_same_index();
   test_move_assignment_different_index();
   test_move_assignment_sfinae();
   test_move_assignment_noexcept();
+  test_triviality_extension();
 }

test/std/utilities/variant/variant.variant/variant.ctor/copy.pass.cpp

 
 // variant(variant const&);
 
 #include <cassert>
 #include <type_traits>
 #include <variant>
 
 #include "test_macros.h"
+#include "test_workarounds.h"
 
 struct NonT {
   NonT(int v) : value(v) {}
   NonT(const NonT &o) : value(o.value) {}
   int value;
 };
 static_assert(!std::is_trivially_copy_constructible<NonT>::value, "");
 

 
 template <size_t Idx>
 constexpr bool test_constexpr_copy_ctor_extension_imp(
     std::variant<long, void*, const int> const& v)
 {
   auto v2 = v;
   return v2.index() == v.index() &&
          v2.index() == Idx &&
          std::get<Idx>(v2) == std::get<Idx>(v);
 }
 
 void test_constexpr_copy_ctor_extension() {
-#ifdef _LIBCPP_VERSION
+#if defined(_LIBCPP_VER) || defined(_MSVC_STL_VER)
   using V = std::variant<long, void*, const int>;
-  static_assert(std::is_trivially_copyable<V>::value, "");
+#ifdef TEST_WORKAROUND_C1XX_BROKEN_IS_TRIVIALLY_COPYABLE
+  static_assert(std::is_trivially_destructible<V>::value, "");
   static_assert(std::is_trivially_copy_constructible<V>::value, "");
+  static_assert(std::is_trivially_move_constructible<V>::value, "");
+  static_assert(!std::is_copy_assignable<V>::value, "");
+  static_assert(!std::is_move_assignable<V>::value, "");
+#else
+  static_assert(std::is_trivially_copyable<V>::value, "");
+#endif
   static_assert(test_constexpr_copy_ctor_extension_imp<0>(V(42l)), "");
   static_assert(test_constexpr_copy_ctor_extension_imp<1>(V(nullptr)), "");
   static_assert(test_constexpr_copy_ctor_extension_imp<2>(V(101)), "");
 #endif
 }
 
+template<class...Ts>
+constexpr bool triviality_test =
+  std::is_trivially_copy_constructible<std::variant<Ts...>>::value ==
+    std::conjunction<std::is_trivially_copy_constructible<Ts>...>::value;
+
+void test_triviality_extension() {
+#if defined(_MSVC_STL_VER)
+  static_assert(triviality_test<int>, "");
+  static_assert(triviality_test<NonT>, "");
+  static_assert(triviality_test<int, long>, "");
+  static_assert(triviality_test<int, NoCopy>, "");
+  static_assert(triviality_test<int, MoveOnly>, "");
+  static_assert(triviality_test<int, MoveOnlyNT>, "");
+  static_assert(triviality_test<int, MakeEmptyT>, "");
+#endif
+}
+
 int main() {
   test_copy_ctor_basic();
   test_copy_ctor_valueless_by_exception();
   test_copy_ctor_sfinae();
   test_constexpr_copy_ctor_extension();
+  test_triviality_extension();
 }

test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp

 // variant(variant&&) noexcept(see below);
 
 #include <cassert>
 #include <string>
 #include <type_traits>
 #include <variant>
 
 #include "test_macros.h"
+#include "test_workarounds.h"
 
 struct ThrowsMove {
   ThrowsMove(ThrowsMove &&) noexcept(false) {}
 };
 
 struct NoCopy {
   NoCopy(const NoCopy &) = delete;
 };
constexpr bool test_constexpr_ctor_extension_imp(
   auto copy = v;
   auto v2 = std::move(copy);
   return v2.index() == v.index() &&
          v2.index() == Idx &&
         std::get<Idx>(v2) == std::get<Idx>(v);
 }
 
 void test_constexpr_move_ctor_extension() {
-#ifdef _LIBCPP_VERSION
+#if defined(_LIBCPP_VER) || defined(_MSVC_STL_VER)
   using V = std::variant<long, void*, const int>;
+#ifdef TEST_WORKAROUND_C1XX_BROKEN_IS_TRIVIALLY_COPYABLE
+  static_assert(std::is_trivially_destructible<V>::value, "");
+  static_assert(std::is_trivially_copy_constructible<V>::value, "");
+  static_assert(std::is_trivially_move_constructible<V>::value, "");
+  static_assert(!std::is_copy_assignable<V>::value, "");
+  static_assert(!std::is_move_assignable<V>::value, "");
+#else
   static_assert(std::is_trivially_copyable<V>::value, "");
+#endif
   static_assert(std::is_trivially_move_constructible<V>::value, "");
   static_assert(test_constexpr_ctor_extension_imp<0>(V(42l)), "");
   static_assert(test_constexpr_ctor_extension_imp<1>(V(nullptr)), "");
   static_assert(test_constexpr_ctor_extension_imp<2>(V(101)), "");
 #endif
 }
 
+template<class...Ts>
+constexpr bool triviality_test =
+  std::is_trivially_move_constructible<std::variant<Ts...>>::value ==
+    std::conjunction<std::is_trivially_move_constructible<Ts>...>::value;
+
+void test_triviality_extension() {
+#if defined(_MSVC_STL_VER)
+  static_assert(triviality_test<int>, "");
+  static_assert(triviality_test<MoveOnly>, "");
+  static_assert(triviality_test<MoveOnlyNT>, "");
+  static_assert(triviality_test<int, long>, "");
+  static_assert(triviality_test<int, MoveOnly>, "");
+  static_assert(triviality_test<int, MoveOnlyNT>, "");
+  static_assert(triviality_test<int, ThrowsMove>, "");
+  static_assert(triviality_test<int, NoCopy>, "");
+  static_assert(triviality_test<int, MakeEmptyT>, "");
+#endif
+}
+
 int main() {
   test_move_ctor_basic();
   test_move_ctor_valueless_by_exception();
   test_move_noexcept();
   test_move_ctor_sfinae();
   test_constexpr_move_ctor_extension();
+  test_triviality_extension();
 }

test/support/msvc_stdlib_force_include.hpp

 
 #include <crtdbg.h>
 #include <stdlib.h>
 
 #if defined(_LIBCPP_VERSION)
     #error This header may not be used when targeting libc++
 #endif
 
+// Indicates that we are using the MSVC standard library.
+#ifndef _MSVC_STL_VER
+    #define _MSVC_STL_VER 42
+#endif
+
 struct AssertionDialogAvoider {
     AssertionDialogAvoider() {
         _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_FILE);
         _CrtSetReportFile(_CRT_ASSERT, _CRTDBG_FILE_STDERR);
 
         _CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE);
         _CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);
     }

test/support/test.workarounds/c1xx_broken_is_trivially_copyable.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// Verify TEST_WORKAROUND_C1XX_BROKEN_IS_TRIVIALLY_COPYABLE.
+
+#include <type_traits>
+
+#include "test_workarounds.h"
+
+struct S {
+  S(S const&) = default;
+  S(S&&) = default;
+  S& operator=(S const&) = delete;
+  S& operator=(S&&) = delete;
+};
+
+int main() {
+#if defined(TEST_WORKAROUND_C1XX_BROKEN_IS_TRIVIALLY_COPYABLE)
+  static_assert(!std::is_trivially_copyable<S>::value, "");
+#else
+  static_assert(std::is_trivially_copyable<S>::value, "");
+#endif
+}

test/support/test_workarounds.h

 
 #ifndef SUPPORT_TEST_WORKAROUNDS_H
 #define SUPPORT_TEST_WORKAROUNDS_H
 
 #include "test_macros.h"
 
 #if defined(TEST_COMPILER_C1XX)
 # define TEST_WORKAROUND_C1XX_BROKEN_NULLPTR_CONVERSION_OPERATOR
+# define TEST_WORKAROUND_C1XX_BROKEN_IS_TRIVIALLY_COPYABLE
 #endif
 
 #endif // SUPPORT_TEST_WORKAROUNDS_H