Optional bugfixes

Summary: Tests were run in 'opt', which masked issues alterted by asserts.

Test Plan: Unit tests

Reviewed By: delong.j@fb.com

FB internal diff: D611957

diff --git a/folly/Optional.h b/folly/Optional.h
index 311f1642ad10f41539b16cf220d99d45b84db617..98b8775df76fc877a22c6c14e991955dac86640b 100644 (file)
--- a/folly/Optional.h
+++ b/folly/Optional.h
@@ -90,12 +90,20 @@ class Optional : boost::totally_ordered<Optional<Value>,
   }
 
   Optional(const Optional& src) {
-    construct(src.value());
+    if (src.hasValue()) {
+      construct(src.value());
+    } else {
+      hasValue_ = false;
+    }
   }
 
   Optional(Optional&& src) {
-    construct(std::move(src.value()));
-    src.clear();
+    if (src.hasValue()) {
+      construct(std::move(src.value()));
+      src.clear();
+    } else {
+      hasValue_ = false;
+    }
   }
 
   /* implicit */ Optional(const None& empty)
@@ -192,7 +200,7 @@ class Optional : boost::totally_ordered<Optional<Value>,
   void clear() {
     if (hasValue()) {
       hasValue_ = false;
-      value().~Value();
+      value_.~Value();
     }
   }
 

diff --git a/folly/test/OptionalTest.cpp b/folly/test/OptionalTest.cpp
index 1268ff662d74c9ca9444bfd9fc25f2d540bdf893..0e240a0144b0e31ea1e7e8a44ebf4f0016fad993 100644 (file)
--- a/folly/test/OptionalTest.cpp
+++ b/folly/test/OptionalTest.cpp
@@ -95,6 +95,15 @@ TEST(Optional, Simple) {
   EXPECT_FALSE(opt);
 }
 
+TEST(Optional, EmptyConstruct) {
+  Optional<int> opt;
+  EXPECT_FALSE(opt);
+  Optional<int> test1(opt);
+  EXPECT_FALSE(test1);
+  Optional<int> test2(std::move(opt));
+  EXPECT_FALSE(test2);
+}
+
 TEST(Optional, Unique) {
   Optional<unique_ptr<int>> opt;