Add move ctor/assignment to Optional<T>

Code review feedback for r175580 by Jordan Rose.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@175729 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ADT/Optional.h b/include/llvm/ADT/Optional.h
index c5dc29946cb8dcd498e0f6b5b1b8a53a53ae655a..fd1da74bb79378da55ed7078135a6c38b80a762c 100644 (file)
--- a/include/llvm/ADT/Optional.h
+++ b/include/llvm/ADT/Optional.h
@@ -46,12 +46,41 @@ public:
   Optional(T &&y) : hasVal(true) {
     new (storage.buffer) T(std::forward<T>(y));
   }
+  Optional(Optional<T> &&O) : hasVal(O) {
+    if (O) {
+      new (storage.buffer) T(std::move(*O));
+      O.reset();
+    }
+  }
+  Optional &operator=(T &&y) {
+    if (hasVal)
+      **this = std::move(y);
+    else {
+      new (storage.buffer) T(std::move(y));
+      hasVal = true;
+    }
+    return *this;
+  }
+  Optional &operator=(Optional &&O) {
+    if (!O)
+      reset();
+    else {
+      *this = std::move(*O);
+      O.reset();
+    }
+    return *this;
+  }
 #endif
 
   static inline Optional create(const T* y) {
     return y ? Optional(*y) : Optional();
   }
 
+  // FIXME: these assignments (& the equivalent const T&/const Optional& ctors)
+  // could be made more efficient by passing by value, possibly unifying them
+  // with the rvalue versions above - but this could place a different set of
+  // requirements (notably: the existence of a default ctor) when implemented
+  // in that way. Careful SFINAE to avoid such pitfalls would be required.
   Optional &operator=(const T &y) {
     if (hasVal)
       **this = y;

diff --git a/unittests/ADT/OptionalTest.cpp b/unittests/ADT/OptionalTest.cpp
index 6fd8bbaf0961087b61a4f2410654c81cd89e9df2..6d37bf4e187d2b2d2a70103201e5c2028b5e80e8 100644 (file)
--- a/unittests/ADT/OptionalTest.cpp
+++ b/unittests/ADT/OptionalTest.cpp
@@ -40,6 +40,36 @@ unsigned NonDefaultConstructible::CopyConstructions = 0;
 unsigned NonDefaultConstructible::Destructions = 0;
 unsigned NonDefaultConstructible::CopyAssignments = 0;
 
+struct MoveOnly {
+  static unsigned MoveConstructions;
+  static unsigned Destructions;
+  static unsigned MoveAssignments;
+  int val;
+  explicit MoveOnly(int val) : val(val) {
+  }
+  MoveOnly(MoveOnly&& other) {
+    val = other.val;
+    ++MoveConstructions;
+  }
+  MoveOnly &operator=(MoveOnly&& other) {
+    val = other.val;
+    ++MoveAssignments;
+    return *this;
+  }
+  ~MoveOnly() {
+    ++Destructions;
+  }
+  static void ResetCounts() {
+    MoveConstructions = 0;
+    Destructions = 0;
+    MoveAssignments = 0;
+  }
+};
+
+unsigned MoveOnly::MoveConstructions = 0;
+unsigned MoveOnly::Destructions = 0;
+unsigned MoveOnly::MoveAssignments = 0;
+
 // Test fixture
 class OptionalTest : public testing::Test {
 };
@@ -169,5 +199,84 @@ TEST_F(OptionalTest, NullCopyConstructionTest) {
   EXPECT_EQ(0u, NonDefaultConstructible::Destructions);
 }
 
+TEST_F(OptionalTest, MoveOnlyNull) {
+  MoveOnly::ResetCounts();
+  Optional<MoveOnly> O;
+  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(0u, MoveOnly::Destructions);
+}
+
+TEST_F(OptionalTest, MoveOnlyConstruction) {
+  MoveOnly::ResetCounts();
+  Optional<MoveOnly> O(MoveOnly(3));
+  EXPECT_TRUE((bool)O);
+  EXPECT_EQ(3, O->val);
+  EXPECT_EQ(1u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(1u, MoveOnly::Destructions);
+}
+
+TEST_F(OptionalTest, MoveOnlyMoveConstruction) {
+  Optional<MoveOnly> A(MoveOnly(3));
+  MoveOnly::ResetCounts();
+  Optional<MoveOnly> B(std::move(A));
+  EXPECT_FALSE((bool)A);
+  EXPECT_TRUE((bool)B);
+  EXPECT_EQ(3, B->val);
+  EXPECT_EQ(1u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(1u, MoveOnly::Destructions);
+}
+
+TEST_F(OptionalTest, MoveOnlyAssignment) {
+  MoveOnly::ResetCounts();
+  Optional<MoveOnly> O;
+  O = MoveOnly(3);
+  EXPECT_TRUE((bool)O);
+  EXPECT_EQ(3, O->val);
+  EXPECT_EQ(1u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(1u, MoveOnly::Destructions);
+}
+
+TEST_F(OptionalTest, MoveOnlyInitializingAssignment) {
+  Optional<MoveOnly> A(MoveOnly(3));
+  Optional<MoveOnly> B;
+  MoveOnly::ResetCounts();
+  B = std::move(A);
+  EXPECT_FALSE((bool)A);
+  EXPECT_TRUE((bool)B);
+  EXPECT_EQ(3, B->val);
+  EXPECT_EQ(1u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(1u, MoveOnly::Destructions);
+}
+
+TEST_F(OptionalTest, MoveOnlyNullingAssignment) {
+  Optional<MoveOnly> A;
+  Optional<MoveOnly> B(MoveOnly(3));
+  MoveOnly::ResetCounts();
+  B = std::move(A);
+  EXPECT_FALSE((bool)A);
+  EXPECT_FALSE((bool)B);
+  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(1u, MoveOnly::Destructions);
+}
+
+TEST_F(OptionalTest, MoveOnlyAssigningAssignment) {
+  Optional<MoveOnly> A(MoveOnly(3));
+  Optional<MoveOnly> B(MoveOnly(4));
+  MoveOnly::ResetCounts();
+  B = std::move(A);
+  EXPECT_FALSE((bool)A);
+  EXPECT_TRUE((bool)B);
+  EXPECT_EQ(3, B->val);
+  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
+  EXPECT_EQ(1u, MoveOnly::MoveAssignments);
+  EXPECT_EQ(1u, MoveOnly::Destructions);
+}
+
 } // end anonymous namespace