From 066812039873991416f04ebb3c051ac8cb669d14 Mon Sep 17 00:00:00 2001
From: Kyle Edwards <kyle.edwards@kitware.com>
Date: Tue, 20 Oct 2020 17:34:27 -0400
Subject: cm::optional: Fix move assignment

---
 Tests/CMakeLib/testOptional.cxx | 28 ++++++++++++++++++++++++
 Utilities/std/cm/optional       | 47 +++++++++++++++++++++++++++++------------
 2 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/Tests/CMakeLib/testOptional.cxx b/Tests/CMakeLib/testOptional.cxx
index de09c0f..2d7dd7c 100644
--- a/Tests/CMakeLib/testOptional.cxx
+++ b/Tests/CMakeLib/testOptional.cxx
@@ -82,6 +82,18 @@ public:
   int Value = 0;
 };
 
+class NoMoveAssignEventLogger : public EventLogger
+{
+public:
+  using EventLogger::EventLogger;
+
+  NoMoveAssignEventLogger(const NoMoveAssignEventLogger&) = default;
+  NoMoveAssignEventLogger(NoMoveAssignEventLogger&&) = default;
+
+  NoMoveAssignEventLogger& operator=(const NoMoveAssignEventLogger&) = default;
+  NoMoveAssignEventLogger& operator=(NoMoveAssignEventLogger&&) = delete;
+};
+
 #define ASSERT_TRUE(x)                                                        \
   do {                                                                        \
     if (!(x)) {                                                               \
@@ -328,12 +340,28 @@ static bool testCopyAssign(std::vector<Event>& expected)
   o1 = o4; // Intentionally duplicated to test assigning an empty optional to
   // an empty optional
 
+  cm::optional<NoMoveAssignEventLogger> o5{ 1 };
+  auto const* v5 = &*o5;
+  const cm::optional<NoMoveAssignEventLogger> o6{ 2 };
+  auto const* v6 = &*o6;
+  o5 = std::move(o6);
+  const NoMoveAssignEventLogger e7{ 3 };
+  o5 = std::move(e7);
+
   expected = {
     { Event::VALUE_CONSTRUCT, v2, nullptr, 4 },
     { Event::COPY_CONSTRUCT, v1, v2, 4 },
     { Event::VALUE_CONSTRUCT, v3, nullptr, 5 },
     { Event::COPY_ASSIGN, v1, v3, 5 },
     { Event::DESTRUCT, v1, nullptr, 5 },
+    { Event::VALUE_CONSTRUCT, v5, nullptr, 1 },
+    { Event::VALUE_CONSTRUCT, v6, nullptr, 2 },
+    { Event::COPY_ASSIGN, v5, v6, 2 },
+    { Event::VALUE_CONSTRUCT, &e7, nullptr, 3 },
+    { Event::COPY_ASSIGN, v5, &e7, 3 },
+    { Event::DESTRUCT, &e7, nullptr, 3 },
+    { Event::DESTRUCT, v6, nullptr, 2 },
+    { Event::DESTRUCT, v5, nullptr, 3 },
     { Event::DESTRUCT, v3, nullptr, 5 },
     { Event::DESTRUCT, v2, nullptr, 4 },
   };
diff --git a/Utilities/std/cm/optional b/Utilities/std/cm/optional
index 9a5d840..4eb7f27 100644
--- a/Utilities/std/cm/optional
+++ b/Utilities/std/cm/optional
@@ -68,16 +68,22 @@ public:
 
   optional& operator=(nullopt_t) noexcept;
   optional& operator=(const optional& other);
-  optional& operator=(optional&& other) noexcept;
 
-  template <
-    typename U = T,
-    typename = typename std::enable_if<
-      !std::is_same<typename std::decay<U>::type, cm::optional<T>>::value &&
-      std::is_constructible<T, U>::value && std::is_assignable<T&, U>::value &&
+  template <typename U = T>
+  typename std::enable_if<std::is_constructible<T, U&&>::value &&
+                            std::is_assignable<T&, U&&>::value,
+                          optional&>::type
+  operator=(optional<U>&& other) noexcept;
+
+  template <typename U = T>
+  typename std::enable_if<
+    !std::is_same<typename std::decay<U>::type, cm::optional<T>>::value &&
+      std::is_constructible<T, U&&>::value &&
+      std::is_assignable<T&, U&&>::value &&
       (!std::is_scalar<T>::value ||
-       !std::is_same<typename std::decay<U>::type, T>::value)>::type>
-  optional& operator=(U&& v);
+       !std::is_same<typename std::decay<U>::type, T>::value),
+    optional&>::type
+  operator=(U&& v);
 
   const T* operator->() const;
   T* operator->();
@@ -140,13 +146,17 @@ optional<T>::optional(nullopt_t) noexcept
 template <typename T>
 optional<T>::optional(const optional& other)
 {
-  *this = other;
+  if (other.has_value()) {
+    this->emplace(*other);
+  }
 }
 
 template <typename T>
 optional<T>::optional(optional&& other) noexcept
 {
-  *this = std::move(other);
+  if (other.has_value()) {
+    this->emplace(std::move(*other));
+  }
 }
 
 template <typename T>
@@ -192,7 +202,11 @@ optional<T>& optional<T>::operator=(const optional& other)
 }
 
 template <typename T>
-optional<T>& optional<T>::operator=(optional&& other) noexcept
+template <typename U>
+typename std::enable_if<std::is_constructible<T, U&&>::value &&
+                          std::is_assignable<T&, U&&>::value,
+                        optional<T>&>::type
+optional<T>::operator=(optional<U>&& other) noexcept
 {
   if (other.has_value()) {
     if (this->has_value()) {
@@ -207,8 +221,15 @@ optional<T>& optional<T>::operator=(optional&& other) noexcept
 }
 
 template <typename T>
-template <typename U, typename>
-optional<T>& optional<T>::operator=(U&& v)
+template <typename U>
+typename std::enable_if<
+  !std::is_same<typename std::decay<U>::type, cm::optional<T>>::value &&
+    std::is_constructible<T, U&&>::value &&
+    std::is_assignable<T&, U&&>::value &&
+    (!std::is_scalar<T>::value ||
+     !std::is_same<typename std::decay<U>::type, T>::value),
+  optional<T>&>::type
+optional<T>::operator=(U&& v)
 {
   if (this->has_value()) {
     this->value() = v;
-- 
cgit v0.12