From fd20c97c69af72155fc5d611630b4799eeda6db8 Mon Sep 17 00:00:00 2001
From: Lovro Puzar <lovro@fb.com>
Date: Mon, 11 Mar 2013 13:52:42 -0700
Subject: [PATCH] Add explicit assignment operator definitions to Optional
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Summary:
See new test.  Under GCC 4.6 (which is what the folly tests build with) the old code works fine but under 4.7 building fails with:

folly/test/OptionalTest.cpp: In member function ‘virtual void Optional_AssignmentContained_Test::TestBody()’:
folly/test/OptionalTest.cpp:122:14: error: use of deleted function ‘ContainsOptional& ContainsOptional::operator=(const ContainsOptional&)’
folly/test/OptionalTest.cpp:106:21: note: ‘ContainsOptional& ContainsOptional::operator=(const ContainsOptional&)’ is implicitly deleted because the default definition would be ill-formed:
folly/test/OptionalTest.cpp:106:21: error: use of deleted function ‘folly::Optional<int>& folly::Optional<int>::operator=(const folly::Optional<int>&)’
In file included from folly/test/OptionalTest.cpp:17:0:
./folly/Optional.h:84:7: note: ‘folly::Optional<int>& folly::Optional<int>::operator=(const folly::Optional<int>&)’ is implicitly declared as deleted because ‘folly::Optional<int>’ declares a move constructor or move assignment operator
folly/test/OptionalTest.cpp:129:30: error: use of deleted function ‘ContainsOptional& ContainsOptional::operator=(ContainsOptional&&)’
folly/test/OptionalTest.cpp:108:21: note: ‘ContainsOptional& ContainsOptional::operator=(ContainsOptional&&)’ is implicitly deleted because the default definition would be ill-formed:
folly/test/OptionalTest.cpp:108:21: error: non-static data member ‘ContainsOptional::opt_’ does not have a move assignment operator or trivial copy assignment operator
folly/test/OptionalTest.cpp:137:14: error: use of deleted function ‘ContainsOptional& ContainsOptional::operator=(const ContainsOptional&)’

Test Plan:
(1) fbconfig folly/test && fbmake dbg && _build/dbg/folly/test/optional_test
(2) Remove folly/PLATFORM to build with gcc 4.7, then repeat (1).  Without the code fix, the new test fails to build.  With the fix, the test builds and runs fine.

Reviewed By: tjackson@fb.com

FB internal diff: D732402
---
 folly/Optional.h            | 10 +++++++++
 folly/test/OptionalTest.cpp | 44 +++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/folly/Optional.h b/folly/Optional.h
index 82070889..871357ce 100644
--- a/folly/Optional.h
+++ b/folly/Optional.h
@@ -169,6 +169,16 @@ class Optional : boost::totally_ordered<Optional<Value>,
     return *this;
   }
 
+  Optional& operator=(Optional &&other) {
+    assign(std::move(other));
+    return *this;
+  }
+
+  Optional& operator=(const Optional &other) {
+    assign(other);
+    return *this;
+  }
+
   bool operator<(const Optional& other) const {
     if (hasValue() != other.hasValue()) {
       return hasValue() < other.hasValue();
diff --git a/folly/test/OptionalTest.cpp b/folly/test/OptionalTest.cpp
index 4eb56417..68e81649 100644
--- a/folly/test/OptionalTest.cpp
+++ b/folly/test/OptionalTest.cpp
@@ -309,3 +309,47 @@ TEST(Optional, MakeOptional) {
   ASSERT_TRUE(optIntPtr.hasValue());
   EXPECT_EQ(**optIntPtr, 3);
 }
+
+class ContainsOptional {
+ public:
+  ContainsOptional() { }
+  explicit ContainsOptional(int x) : opt_(x) { }
+  bool hasValue() const { return opt_.hasValue(); }
+  int value() const { return opt_.value(); }
+
+  ContainsOptional(const ContainsOptional &other) = default;
+  ContainsOptional& operator=(const ContainsOptional &other) = default;
+  ContainsOptional(ContainsOptional &&other) = default;
+  ContainsOptional& operator=(ContainsOptional &&other) = default;
+
+ private:
+  Optional<int> opt_;
+};
+
+/**
+ * Test that a class containing an Optional can be copy and move assigned.
+ * This was broken under gcc 4.7 until assignment operators were explicitly
+ * defined.
+ */
+TEST(Optional, AssignmentContained) {
+  {
+    ContainsOptional source(5), target;
+    target = source;
+    EXPECT_TRUE(target.hasValue());
+    EXPECT_EQ(5, target.value());
+  }
+
+  {
+    ContainsOptional source(5), target;
+    target = std::move(source);
+    EXPECT_TRUE(target.hasValue());
+    EXPECT_EQ(5, target.value());
+    EXPECT_FALSE(source.hasValue());
+  }
+
+  {
+    ContainsOptional opt_uninit, target(10);
+    target = opt_uninit;
+    EXPECT_FALSE(target.hasValue());
+  }
+}
-- 
2.34.1