From: Simon Martin <simonmartin@fb.com>
Date: Wed, 21 May 2014 20:31:54 +0000 (-0700)
Subject: Future::value() should throw when unset
X-Git-Tag: v0.22.0~528
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=3bb510706c136314efae6899688de2c4246c0e9f;p=folly.git

Future::value() should throw when unset

Summary:
Added a test to call Future::value() before the Promise value is set, expecting an exception.
In a dbg build the test failed due on the assertion in Optional::value().
In a opt build the test failed due as no exception was thrown.
There are 2 points where we could throw our exception:
a) Optional::value() - replacing the assertion
b) Future::value()

I'm not sure which location makes the most sense.
With the assertion in Optional it seems that adding the throw here would not be unexpected but this is outside the wangle code.
So as a first pass I've added the throw in Future::value(), and made a new WangleException for this.

Test Plan:
$ fbconfig folly/wangle
$ fbmake runtests

Reviewed By: hans@fb.com

Subscribers: folly@lists, fugalh

FB internal diff: D1340886
---

diff --git a/folly/wangle/WangleException.h b/folly/wangle/WangleException.h
index b1fd737a..aec22970 100644
--- a/folly/wangle/WangleException.h
+++ b/folly/wangle/WangleException.h
@@ -63,6 +63,12 @@ class PromiseAlreadySatisfied : public WangleException {
       WangleException("Promise already satisfied") { }
 };
 
+class FutureNotReady : public WangleException {
+  public:
+    explicit FutureNotReady() :
+      WangleException("Future not ready") { }
+};
+
 class FutureAlreadyRetrieved : public WangleException {
   public:
     explicit FutureAlreadyRetrieved () :
diff --git a/folly/wangle/detail.h b/folly/wangle/detail.h
index dda83ad4..8a89c641 100644
--- a/folly/wangle/detail.h
+++ b/folly/wangle/detail.h
@@ -38,7 +38,7 @@ class FutureObject {
   FutureObject& operator=(FutureObject const&) = delete;
 
   // not movable (see comment in the implementation of Future::then)
-  FutureObject(FutureObject&&) = delete;
+  FutureObject(FutureObject&&) noexcept = delete;
   FutureObject& operator=(FutureObject&&) = delete;
 
   Try<T>& getTry() {
@@ -85,7 +85,11 @@ class FutureObject {
   }
 
   typename std::add_lvalue_reference<T>::type value() {
-    return value_->value();
+    if (ready()) {
+      return value_->value();
+    } else {
+      throw FutureNotReady();
+    }
   }
 
  private:
diff --git a/folly/wangle/test/FutureTest.cpp b/folly/wangle/test/FutureTest.cpp
index 0202470c..a7a2fa1c 100644
--- a/folly/wangle/test/FutureTest.cpp
+++ b/folly/wangle/test/FutureTest.cpp
@@ -153,6 +153,12 @@ TEST(Future, isReady) {
   EXPECT_TRUE(f.isReady());
   }
 
+TEST(Future, futureNotReady) {
+  Promise<int> p;
+  Future<int> f = p.getFuture();
+  EXPECT_THROW(f.value(), eggs_t);
+}
+
 TEST(Future, hasException) {
   EXPECT_TRUE(makeFuture<int>(eggs).getTry().hasException());
   EXPECT_FALSE(makeFuture(42).getTry().hasException());