Making Optional throw exceptions instead of assert

Summary: I'm upgrading assertions to throws, since these are fatal in all circumstances. If something is explicitly `Optional`, it makes sense to fail loudly if it is misused in this manner.

Reviewed By: @yfeldblum

Differential Revision: D2247612

diff --git a/folly/Optional.h b/folly/Optional.h
index f4bc9ff4ce7a36bb47eb82f7a5d49a4d5d7a7e10..9947fd4adf05a864ef149e80da30e4f5982946e5 100644 (file)
--- a/folly/Optional.h
+++ b/folly/Optional.h
@@ -54,8 +54,8 @@
  *    cout << *v << endl;
  *  }
  */
-#include <cassert>
 #include <cstddef>
+#include <stdexcept>
 #include <type_traits>
 #include <utility>
 
@@ -82,6 +82,12 @@ const None none = nullptr;
 # pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
 #endif // __GNUC__
 
+class OptionalEmptyException : public std::runtime_error {
+ public:
+  OptionalEmptyException()
+      : std::runtime_error("Empty Optional cannot be unwrapped") {}
+};
+
 template<class Value>
 class Optional {
  public:
@@ -206,15 +212,18 @@ class Optional {
   }
 
   const Value& value() const {
-    assert(hasValue());
+    require_value();
     return value_;
   }
 
   Value& value() {
-    assert(hasValue());
+    require_value();
     return value_;
   }
 
+  Value* get_pointer() { return hasValue_ ? &value_ : nullptr; }
+  const Value* get_pointer() const { return hasValue_ ? &value_ : nullptr; }
+
   bool hasValue() const { return hasValue_; }
 
   explicit operator bool() const {
@@ -239,6 +248,12 @@ class Optional {
   }
 
  private:
+  void require_value() const {
+    if (!hasValue_) {
+      throw OptionalEmptyException();
+    }
+  }
+
   template<class... Args>
   void construct(Args&&... args) {
     const void* ptr = &value_;
@@ -258,12 +273,12 @@ class Optional {
 
 template<class T>
 const T* get_pointer(const Optional<T>& opt) {
-  return opt ? &opt.value() : nullptr;
+  return opt.get_pointer();
 }
 
 template<class T>
 T* get_pointer(Optional<T>& opt) {
-  return opt ? &opt.value() : nullptr;
+  return opt.get_pointer();
 }
 
 template<class T>

diff --git a/folly/test/OptionalTest.cpp b/folly/test/OptionalTest.cpp
index 2cfef6c52d41e08b9cf71e72cca4046306316eb7..72c2734591670aa2bb0ec7e0c6be987bd7c0e9c5 100644 (file)
--- a/folly/test/OptionalTest.cpp
+++ b/folly/test/OptionalTest.cpp
@@ -497,4 +497,9 @@ TEST(Optional, AssignmentContained) {
   }
 }
 
+TEST(Optional, Exceptions) {
+  Optional<int> empty;
+  EXPECT_THROW(empty.value(), OptionalEmptyException);
+}
+
 }