[llvm] 347d4d7 - [ADT] Fix Optional<> with llvm::is_trivially_move_constructible

[Steven Wu via llvm-commits]

Author: Steven Wu
Date: 2022-01-18T08:37:43-08:00
New Revision: 347d4d7323c4cf0d89fb14530e27ec5e2b1afeda

URL: https://github.com/llvm/llvm-project/commit/347d4d7323c4cf0d89fb14530e27ec5e2b1afeda
DIFF: https://github.com/llvm/llvm-project/commit/347d4d7323c4cf0d89fb14530e27ec5e2b1afeda.diff

LOG: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible

Fix the compatibility of Optional<> with some GCC versions that it will fail
to compile when T is getting checked for `is_trivially_move_constructible`
as mentioned here: https://reviews.llvm.org/D93510#2538983

Fix the problem by using `llvm::is_trivially_move_constructible`.

Reviewed By: jplayer-nv, tatyana-krasnukha

Differential Revision: https://reviews.llvm.org/D117254

Added: 
    

Modified: 
    llvm/include/llvm/ADT/Optional.h
    llvm/unittests/ADT/OptionalTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/Optional.h b/llvm/include/llvm/ADT/Optional.h
index cdbf2e3532419..2af59865b8ffe 100644
--- a/llvm/include/llvm/ADT/Optional.h
+++ b/llvm/include/llvm/ADT/Optional.h
@@ -34,12 +34,12 @@ namespace optional_detail {
 /// Storage for any type.
 //
 // The specialization condition intentionally uses
-// llvm::is_trivially_copy_constructible instead of
-// std::is_trivially_copy_constructible.  GCC versions prior to 7.4 may
-// instantiate the copy constructor of `T` when
-// std::is_trivially_copy_constructible is instantiated.  This causes
-// compilation to fail if we query the trivially copy constructible property of
-// a class which is not copy constructible.
+// llvm::is_trivially_{copy/move}_constructible instead of
+// std::is_trivially_{copy/move}_constructible. GCC versions prior to 7.4 may
+// instantiate the copy/move constructor of `T` when
+// std::is_trivially_{copy/move}_constructible is instantiated.  This causes
+// compilation to fail if we query the trivially copy/move constructible
+// property of a class which is not copy/move constructible.
 //
 // The current implementation of OptionalStorage insists that in order to use
 // the trivial specialization, the value_type must be trivially copy
@@ -50,12 +50,13 @@ namespace optional_detail {
 //
 // The move constructible / assignable conditions emulate the remaining behavior
 // of std::is_trivially_copyable.
-template <typename T, bool = (llvm::is_trivially_copy_constructible<T>::value &&
-                              std::is_trivially_copy_assignable<T>::value &&
-                              (std::is_trivially_move_constructible<T>::value ||
-                               !std::is_move_constructible<T>::value) &&
-                              (std::is_trivially_move_assignable<T>::value ||
-                               !std::is_move_assignable<T>::value))>
+template <typename T,
+          bool = (llvm::is_trivially_copy_constructible<T>::value &&
+                  std::is_trivially_copy_assignable<T>::value &&
+                  (llvm::is_trivially_move_constructible<T>::value ||
+                   !std::is_move_constructible<T>::value) &&
+                  (std::is_trivially_move_assignable<T>::value ||
+                   !std::is_move_assignable<T>::value))>
 class OptionalStorage {
   union {
     char empty;

diff  --git a/llvm/unittests/ADT/OptionalTest.cpp b/llvm/unittests/ADT/OptionalTest.cpp
index debeb78e8fab7..b664792f2dfb9 100644
--- a/llvm/unittests/ADT/OptionalTest.cpp
+++ b/llvm/unittests/ADT/OptionalTest.cpp
@@ -805,4 +805,15 @@ TEST(OptionalTest, HashValue) {
   EXPECT_EQ(hash_value(B), hash_value(I));
 }
 
+struct NotTriviallyCopyable {
+  NotTriviallyCopyable(); // Constructor out-of-line.
+  virtual ~NotTriviallyCopyable() = default;
+  Optional<MoveOnly> MO;
+};
+
+TEST(OptionalTest, GCCIsTriviallyMoveConstructibleCompat) {
+  Optional<NotTriviallyCopyable> V;
+  EXPECT_FALSE(V);
+}
+
 } // end anonymous namespace