[PATCH] D117254: [ADT] Fix Optional<> with llvm::is_trivially_move_constructible

Steven Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 14 14:43:48 PST 2022


steven_wu updated this revision to Diff 400155.
steven_wu added a comment.

Rebase to TOT


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117254/new/

https://reviews.llvm.org/D117254

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


Index: llvm/unittests/ADT/OptionalTest.cpp
===================================================================
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -805,4 +805,15 @@
   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
Index: llvm/include/llvm/ADT/Optional.h
===================================================================
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -34,12 +34,12 @@
 /// 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
@@ -52,7 +52,7 @@
 // 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 ||
+                              (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))>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117254.400155.patch
Type: text/x-patch
Size: 2396 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220114/2034782d/attachment.bin>


More information about the llvm-commits mailing list