From: Lee Howes <lwh@fb.com>
Date: Mon, 30 Oct 2017 21:28:33 +0000 (-0700)
Subject: Split SemiFuture and Future into separate types. Add BasicFuture shared between them.
X-Git-Tag: v2017.11.06.00~35
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=46af81b61d7c026deb2735019ae991d36e7b0d39

Split SemiFuture and Future into separate types. Add BasicFuture shared between them.

Summary:
To avoid the risk of bugs caused by a Future being cast to a SemiFuture, and losing some of the properties in the process, this splits SemiFuture and Future into unrelated types, sharing a private superclass for code reuse.
 * Add BasicFuture in futures::detail
 * Make superclass privately inherited.
 * Unset executor when constructing SemiFuture from Future.

Reviewed By: yfeldblum

Differential Revision: D6177780

fbshipit-source-id: dea3116aeec0572bb973c2a561e17785199e86f2
---

diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h
index 7c8c7af6..17f47231 100644
--- a/folly/futures/Future-inl.h
+++ b/folly/futures/Future-inl.h
@@ -129,102 +129,25 @@ inline auto makeCoreCallbackState(Promise<T>&& p, F&& f) noexcept(
   return CoreCallbackState<T, _t<std::decay<F>>>(
       std::move(p), std::forward<F>(f));
 }
-} // namespace detail
-} // namespace futures
-
-template <class T>
-SemiFuture<typename std::decay<T>::type> makeSemiFuture(T&& t) {
-  return makeSemiFuture(Try<typename std::decay<T>::type>(std::forward<T>(t)));
-}
-
-// makeSemiFutureWith(SemiFuture<T>()) -> SemiFuture<T>
-template <class F>
-typename std::enable_if<
-    isSemiFuture<typename std::result_of<F()>::type>::value,
-    typename std::result_of<F()>::type>::type
-makeSemiFutureWith(F&& func) {
-  using InnerType =
-      typename isSemiFuture<typename std::result_of<F()>::type>::Inner;
-  try {
-    return std::forward<F>(func)();
-  } catch (std::exception& e) {
-    return makeSemiFuture<InnerType>(
-        exception_wrapper(std::current_exception(), e));
-  } catch (...) {
-    return makeSemiFuture<InnerType>(
-        exception_wrapper(std::current_exception()));
-  }
-}
-
-// makeSemiFutureWith(T()) -> SemiFuture<T>
-// makeSemiFutureWith(void()) -> SemiFuture<Unit>
-template <class F>
-typename std::enable_if<
-    !(isSemiFuture<typename std::result_of<F()>::type>::value),
-    SemiFuture<Unit::LiftT<typename std::result_of<F()>::type>>>::type
-makeSemiFutureWith(F&& func) {
-  using LiftedResult = Unit::LiftT<typename std::result_of<F()>::type>;
-  return makeSemiFuture<LiftedResult>(
-      makeTryWith([&func]() mutable { return std::forward<F>(func)(); }));
-}
-
-template <class T>
-SemiFuture<T> makeSemiFuture(std::exception_ptr const& e) {
-  return makeSemiFuture(Try<T>(e));
-}
 
 template <class T>
-SemiFuture<T> makeSemiFuture(exception_wrapper ew) {
-  return makeSemiFuture(Try<T>(std::move(ew)));
-}
-
-template <class T, class E>
-typename std::
-    enable_if<std::is_base_of<std::exception, E>::value, SemiFuture<T>>::type
-    makeSemiFuture(E const& e) {
-  return makeSemiFuture(Try<T>(make_exception_wrapper<E>(e)));
-}
-
-template <class T>
-SemiFuture<T> makeSemiFuture(Try<T>&& t) {
-  return SemiFuture<T>(new futures::detail::Core<T>(std::move(t)));
-}
-
-template <class T>
-SemiFuture<T> SemiFuture<T>::makeEmpty() {
-  return SemiFuture<T>(futures::detail::EmptyConstruct{});
-}
-
-template <class T>
-SemiFuture<T>::SemiFuture(SemiFuture<T>&& other) noexcept : core_(other.core_) {
+FutureBase<T>::FutureBase(SemiFuture<T>&& other) noexcept : core_(other.core_) {
   other.core_ = nullptr;
 }
 
 template <class T>
-SemiFuture<T>& SemiFuture<T>::operator=(SemiFuture<T>&& other) noexcept {
-  std::swap(core_, other.core_);
-  return *this;
-}
-
-template <class T>
-SemiFuture<T>::SemiFuture(Future<T>&& other) noexcept : core_(other.core_) {
+FutureBase<T>::FutureBase(Future<T>&& other) noexcept : core_(other.core_) {
   other.core_ = nullptr;
 }
 
-template <class T>
-SemiFuture<T>& SemiFuture<T>::operator=(Future<T>&& other) noexcept {
-  std::swap(core_, other.core_);
-  return *this;
-}
-
 template <class T>
 template <class T2, typename>
-SemiFuture<T>::SemiFuture(T2&& val)
+FutureBase<T>::FutureBase(T2&& val)
     : core_(new futures::detail::Core<T>(Try<T>(std::forward<T2>(val)))) {}
 
 template <class T>
 template <typename T2>
-SemiFuture<T>::SemiFuture(
+FutureBase<T>::FutureBase(
     typename std::enable_if<std::is_same<Unit, T2>::value>::type*)
     : core_(new futures::detail::Core<T>(Try<T>(T()))) {}
 
@@ -233,52 +156,52 @@ template <
     class... Args,
     typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
         type>
-SemiFuture<T>::SemiFuture(in_place_t, Args&&... args)
+FutureBase<T>::FutureBase(in_place_t, Args&&... args)
     : core_(
           new futures::detail::Core<T>(in_place, std::forward<Args>(args)...)) {
 }
 
 template <class T>
-SemiFuture<T>::~SemiFuture() {
-  detach();
+template <class FutureType>
+void FutureBase<T>::assign(FutureType& other) noexcept {
+  std::swap(core_, other.core_);
 }
 
-// This must be defined after the constructors to avoid a bug in MSVC
-// https://connect.microsoft.com/VisualStudio/feedback/details/3142777/out-of-line-constructor-definition-after-implicit-reference-causes-incorrect-c2244
-inline SemiFuture<Unit> makeSemiFuture() {
-  return makeSemiFuture(Unit{});
+template <class T>
+FutureBase<T>::~FutureBase() {
+  detach();
 }
 
 template <class T>
-T& SemiFuture<T>::value() & {
+T& FutureBase<T>::value() & {
   throwIfInvalid();
 
   return core_->getTry().value();
 }
 
 template <class T>
-T const& SemiFuture<T>::value() const& {
+T const& FutureBase<T>::value() const& {
   throwIfInvalid();
 
   return core_->getTry().value();
 }
 
 template <class T>
-T&& SemiFuture<T>::value() && {
+T&& FutureBase<T>::value() && {
   throwIfInvalid();
 
   return std::move(core_->getTry().value());
 }
 
 template <class T>
-T const&& SemiFuture<T>::value() const&& {
+T const&& FutureBase<T>::value() const&& {
   throwIfInvalid();
 
   return std::move(core_->getTry().value());
 }
 
 template <class T>
-inline Future<T> SemiFuture<T>::via(Executor* executor, int8_t priority) && {
+inline Future<T> FutureBase<T>::via(Executor* executor, int8_t priority) && {
   throwIfInvalid();
 
   setExecutor(executor, priority);
@@ -289,36 +212,23 @@ inline Future<T> SemiFuture<T>::via(Executor* executor, int8_t priority) && {
 }
 
 template <class T>
-inline Future<T> SemiFuture<T>::via(Executor* executor, int8_t priority) & {
-  throwIfInvalid();
-  Promise<T> p;
-  auto f = p.getFuture();
-  auto func = [p = std::move(p)](Try<T>&& t) mutable {
-    p.setTry(std::move(t));
-  };
-  using R = futures::detail::callableResult<T, decltype(func)>;
-  thenImplementation<decltype(func), R>(std::move(func), typename R::Arg());
-  return std::move(f).via(executor, priority);
-}
-
-template <class T>
-bool SemiFuture<T>::isReady() const {
+bool FutureBase<T>::isReady() const {
   throwIfInvalid();
   return core_->ready();
 }
 
 template <class T>
-bool SemiFuture<T>::hasValue() {
+bool FutureBase<T>::hasValue() {
   return getTry().hasValue();
 }
 
 template <class T>
-bool SemiFuture<T>::hasException() {
+bool FutureBase<T>::hasException() {
   return getTry().hasException();
 }
 
 template <class T>
-void SemiFuture<T>::detach() {
+void FutureBase<T>::detach() {
   if (core_) {
     core_->detachFuture();
     core_ = nullptr;
@@ -326,21 +236,21 @@ void SemiFuture<T>::detach() {
 }
 
 template <class T>
-Try<T>& SemiFuture<T>::getTry() {
+Try<T>& FutureBase<T>::getTry() {
   throwIfInvalid();
 
   return core_->getTry();
 }
 
 template <class T>
-void SemiFuture<T>::throwIfInvalid() const {
+void FutureBase<T>::throwIfInvalid() const {
   if (!core_) {
     throwNoState();
-}
+  }
 }
 
 template <class T>
-Optional<Try<T>> SemiFuture<T>::poll() {
+Optional<Try<T>> FutureBase<T>::poll() {
   Optional<Try<T>> o;
   if (core_->ready()) {
     o = std::move(core_->getTry());
@@ -349,104 +259,21 @@ Optional<Try<T>> SemiFuture<T>::poll() {
 }
 
 template <class T>
-void SemiFuture<T>::raise(exception_wrapper exception) {
+void FutureBase<T>::raise(exception_wrapper exception) {
   core_->raise(std::move(exception));
 }
 
 template <class T>
 template <class F>
-void SemiFuture<T>::setCallback_(F&& func) {
+void FutureBase<T>::setCallback_(F&& func) {
   throwIfInvalid();
   core_->setCallback(std::forward<F>(func));
 }
 
 template <class T>
-SemiFuture<T>::SemiFuture(futures::detail::EmptyConstruct) noexcept
+FutureBase<T>::FutureBase(futures::detail::EmptyConstruct) noexcept
     : core_(nullptr) {}
 
-template <class T>
-Future<T> Future<T>::makeEmpty() {
-  return Future<T>(futures::detail::EmptyConstruct{});
-}
-
-template <class T>
-Future<T>::Future(Future<T>&& other) noexcept
-    : SemiFuture<T>(std::move(other)) {}
-
-template <class T>
-Future<T>& Future<T>::operator=(Future<T>&& other) noexcept {
-  SemiFuture<T>::operator=(SemiFuture<T>{std::move(other)});
-  return *this;
-}
-
-template <class T>
-template <
-    class T2,
-    typename std::enable_if<
-        !std::is_same<T, typename std::decay<T2>::type>::value &&
-            std::is_constructible<T, T2&&>::value &&
-            std::is_convertible<T2&&, T>::value,
-        int>::type>
-Future<T>::Future(Future<T2>&& other)
-    : Future(std::move(other).then([](T2&& v) { return T(std::move(v)); })) {}
-
-template <class T>
-template <
-    class T2,
-    typename std::enable_if<
-        !std::is_same<T, typename std::decay<T2>::type>::value &&
-            std::is_constructible<T, T2&&>::value &&
-            !std::is_convertible<T2&&, T>::value,
-        int>::type>
-Future<T>::Future(Future<T2>&& other)
-    : Future(std::move(other).then([](T2&& v) { return T(std::move(v)); })) {}
-
-template <class T>
-template <
-    class T2,
-    typename std::enable_if<
-        !std::is_same<T, typename std::decay<T2>::type>::value &&
-            std::is_constructible<T, T2&&>::value,
-        int>::type>
-Future<T>& Future<T>::operator=(Future<T2>&& other) {
-  return operator=(
-      std::move(other).then([](T2&& v) { return T(std::move(v)); }));
-}
-
-// TODO: isSemiFuture
-template <class T>
-template <class T2, typename>
-Future<T>::Future(T2&& val) : SemiFuture<T>(std::forward<T2>(val)) {}
-
-template <class T>
-template <typename T2>
-Future<T>::Future(typename std::enable_if<std::is_same<Unit, T2>::value>::type*)
-    : SemiFuture<T>() {}
-
-template <class T>
-template <
-    class... Args,
-    typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
-        type>
-Future<T>::Future(in_place_t, Args&&... args)
-    : SemiFuture<T>(in_place, std::forward<Args>(args)...) {}
-
-template <class T>
-Future<T>::~Future() {
-}
-
-// unwrap
-
-template <class T>
-template <class F>
-typename std::enable_if<isFuture<F>::value,
-                        Future<typename isFuture<T>::Inner>>::type
-Future<T>::unwrap() {
-  return then([](Future<typename isFuture<T>::Inner> internal_future) {
-      return internal_future;
-  });
-}
-
 // then
 
 // Variant: returns a value
@@ -454,7 +281,7 @@ Future<T>::unwrap() {
 template <class T>
 template <typename F, typename R, bool isTry, typename... Args>
 typename std::enable_if<!R::ReturnsFuture::value, typename R::Return>::type
-SemiFuture<T>::thenImplementation(
+FutureBase<T>::thenImplementation(
     F&& func,
     futures::detail::argResult<isTry, F, Args...>) {
   static_assert(sizeof...(Args) <= 1, "Then must take zero/one argument");
@@ -517,7 +344,7 @@ SemiFuture<T>::thenImplementation(
 template <class T>
 template <typename F, typename R, bool isTry, typename... Args>
 typename std::enable_if<R::ReturnsFuture::value, typename R::Return>::type
-SemiFuture<T>::thenImplementation(
+FutureBase<T>::thenImplementation(
     F&& func,
     futures::detail::argResult<isTry, F, Args...>) {
   static_assert(sizeof...(Args) <= 1, "Then must take zero/one argument");
@@ -550,6 +377,181 @@ SemiFuture<T>::thenImplementation(
 
   return f;
 }
+} // namespace detail
+} // namespace futures
+
+template <class T>
+SemiFuture<typename std::decay<T>::type> makeSemiFuture(T&& t) {
+  return makeSemiFuture(Try<typename std::decay<T>::type>(std::forward<T>(t)));
+}
+
+// makeSemiFutureWith(SemiFuture<T>()) -> SemiFuture<T>
+template <class F>
+typename std::enable_if<
+    isSemiFuture<typename std::result_of<F()>::type>::value,
+    typename std::result_of<F()>::type>::type
+makeSemiFutureWith(F&& func) {
+  using InnerType =
+      typename isSemiFuture<typename std::result_of<F()>::type>::Inner;
+  try {
+    return std::forward<F>(func)();
+  } catch (std::exception& e) {
+    return makeSemiFuture<InnerType>(
+        exception_wrapper(std::current_exception(), e));
+  } catch (...) {
+    return makeSemiFuture<InnerType>(
+        exception_wrapper(std::current_exception()));
+  }
+}
+
+// makeSemiFutureWith(T()) -> SemiFuture<T>
+// makeSemiFutureWith(void()) -> SemiFuture<Unit>
+template <class F>
+typename std::enable_if<
+    !(isSemiFuture<typename std::result_of<F()>::type>::value),
+    SemiFuture<Unit::LiftT<typename std::result_of<F()>::type>>>::type
+makeSemiFutureWith(F&& func) {
+  using LiftedResult = Unit::LiftT<typename std::result_of<F()>::type>;
+  return makeSemiFuture<LiftedResult>(
+      makeTryWith([&func]() mutable { return std::forward<F>(func)(); }));
+}
+
+template <class T>
+SemiFuture<T> makeSemiFuture(std::exception_ptr const& e) {
+  return makeSemiFuture(Try<T>(e));
+}
+
+template <class T>
+SemiFuture<T> makeSemiFuture(exception_wrapper ew) {
+  return makeSemiFuture(Try<T>(std::move(ew)));
+}
+
+template <class T, class E>
+typename std::
+    enable_if<std::is_base_of<std::exception, E>::value, SemiFuture<T>>::type
+    makeSemiFuture(E const& e) {
+  return makeSemiFuture(Try<T>(make_exception_wrapper<E>(e)));
+}
+
+template <class T>
+SemiFuture<T> makeSemiFuture(Try<T>&& t) {
+  return SemiFuture<T>(new futures::detail::Core<T>(std::move(t)));
+}
+
+// This must be defined after the constructors to avoid a bug in MSVC
+// https://connect.microsoft.com/VisualStudio/feedback/details/3142777/out-of-line-constructor-definition-after-implicit-reference-causes-incorrect-c2244
+inline SemiFuture<Unit> makeSemiFuture() {
+  return makeSemiFuture(Unit{});
+}
+
+template <class T>
+SemiFuture<T> SemiFuture<T>::makeEmpty() {
+  return SemiFuture<T>(futures::detail::EmptyConstruct{});
+}
+
+template <class T>
+SemiFuture<T>::SemiFuture(SemiFuture<T>&& other) noexcept
+    : futures::detail::FutureBase<T>(std::move(other)) {}
+
+template <class T>
+SemiFuture<T>::SemiFuture(Future<T>&& other) noexcept
+    : futures::detail::FutureBase<T>(std::move(other)) {
+  // SemiFuture should not have an executor on construction
+  if (this->core_) {
+    this->setExecutor(nullptr);
+  }
+}
+
+template <class T>
+SemiFuture<T>& SemiFuture<T>::operator=(SemiFuture<T>&& other) noexcept {
+  this->assign(other);
+  return *this;
+}
+
+template <class T>
+SemiFuture<T>& SemiFuture<T>::operator=(Future<T>&& other) noexcept {
+  this->assign(other);
+  // SemiFuture should not have an executor on construction
+  if (this->core_) {
+    this->setExecutor(nullptr);
+  }
+  return *this;
+}
+
+template <class T>
+Future<T> Future<T>::makeEmpty() {
+  return Future<T>(futures::detail::EmptyConstruct{});
+}
+
+template <class T>
+Future<T>::Future(Future<T>&& other) noexcept
+    : futures::detail::FutureBase<T>(std::move(other)) {}
+
+template <class T>
+Future<T>& Future<T>::operator=(Future<T>&& other) noexcept {
+  this->assign(other);
+  return *this;
+}
+
+template <class T>
+template <
+    class T2,
+    typename std::enable_if<
+        !std::is_same<T, typename std::decay<T2>::type>::value &&
+            std::is_constructible<T, T2&&>::value &&
+            std::is_convertible<T2&&, T>::value,
+        int>::type>
+Future<T>::Future(Future<T2>&& other)
+    : Future(std::move(other).then([](T2&& v) { return T(std::move(v)); })) {}
+
+template <class T>
+template <
+    class T2,
+    typename std::enable_if<
+        !std::is_same<T, typename std::decay<T2>::type>::value &&
+            std::is_constructible<T, T2&&>::value &&
+            !std::is_convertible<T2&&, T>::value,
+        int>::type>
+Future<T>::Future(Future<T2>&& other)
+    : Future(std::move(other).then([](T2&& v) { return T(std::move(v)); })) {}
+
+template <class T>
+template <
+    class T2,
+    typename std::enable_if<
+        !std::is_same<T, typename std::decay<T2>::type>::value &&
+            std::is_constructible<T, T2&&>::value,
+        int>::type>
+Future<T>& Future<T>::operator=(Future<T2>&& other) {
+  return operator=(
+      std::move(other).then([](T2&& v) { return T(std::move(v)); }));
+}
+
+// unwrap
+
+template <class T>
+template <class F>
+typename std::
+    enable_if<isFuture<F>::value, Future<typename isFuture<T>::Inner>>::type
+    Future<T>::unwrap() {
+  return then([](Future<typename isFuture<T>::Inner> internal_future) {
+    return internal_future;
+  });
+}
+
+template <class T>
+inline Future<T> Future<T>::via(Executor* executor, int8_t priority) & {
+  this->throwIfInvalid();
+  Promise<T> p;
+  auto f = p.getFuture();
+  auto func = [p = std::move(p)](Try<T>&& t) mutable {
+    p.setTry(std::move(t));
+  };
+  using R = futures::detail::callableResult<T, decltype(func)>;
+  this->template thenImplementation<decltype(func), R>(
+      std::move(func), typename R::Arg());
+  return std::move(f).via(executor, priority);
+}
 
 template <typename T>
 template <typename R, typename Caller, typename... Args>
@@ -733,10 +735,6 @@ auto via(Executor* x, Func&& func)
   return via(x).then(std::forward<Func>(func));
 }
 
-template <class T>
-Future<T>::Future(futures::detail::EmptyConstruct) noexcept
-    : SemiFuture<T>(futures::detail::EmptyConstruct{}) {}
-
 // makeFuture
 
 template <class T>
@@ -1262,10 +1260,10 @@ Future<T> Future<T>::within(Duration dur, E e, Timekeeper* tk) {
 template <class T>
 Future<T> Future<T>::delayed(Duration dur, Timekeeper* tk) {
   return collectAll(*this, futures::sleep(dur, tk))
-    .then([](std::tuple<Try<T>, Try<Unit>> tup) {
-      Try<T>& t = std::get<0>(tup);
-      return makeFuture<T>(std::move(t));
-    });
+      .then([](std::tuple<Try<T>, Try<Unit>> tup) {
+        Try<T>& t = std::get<0>(tup);
+        return makeFuture<T>(std::move(t));
+      });
 }
 
 namespace futures {
@@ -1294,7 +1292,7 @@ void waitImpl(FutureType& f, Duration dur) {
   Promise<T> promise;
   auto ret = promise.getFuture();
   auto baton = std::make_shared<FutureBatonType>();
-  f.setCallback_([ baton, promise = std::move(promise) ](Try<T> && t) mutable {
+  f.setCallback_([baton, promise = std::move(promise)](Try<T>&& t) mutable {
     promise.setTry(std::move(t));
     baton->post();
   });
@@ -1347,12 +1345,12 @@ SemiFuture<T>&& SemiFuture<T>::wait(Duration dur) && {
 }
 
 template <class T>
-T SemiFuture<T>::get() {
+T SemiFuture<T>::get() && {
   return std::move(wait().value());
 }
 
 template <class T>
-T SemiFuture<T>::get(Duration dur) {
+T SemiFuture<T>::get(Duration dur) && {
   wait(dur);
   if (this->isReady()) {
     return std::move(this->value());
@@ -1397,6 +1395,21 @@ Future<T>&& Future<T>::waitVia(DrivableExecutor* e) && {
   return std::move(*this);
 }
 
+template <class T>
+T Future<T>::get() {
+  return std::move(wait().value());
+}
+
+template <class T>
+T Future<T>::get(Duration dur) {
+  wait(dur);
+  if (this->isReady()) {
+    return std::move(this->value());
+  } else {
+    throwTimedOut();
+  }
+}
+
 template <class T>
 T Future<T>::getVia(DrivableExecutor* e) {
   return std::move(waitVia(e).value());
diff --git a/folly/futures/Future.h b/folly/futures/Future.h
index 0c4092aa..39957470 100644
--- a/folly/futures/Future.h
+++ b/folly/futures/Future.h
@@ -45,42 +45,41 @@ template <class T>
 class Future;
 
 template <class T>
-class SemiFuture {
+class SemiFuture;
+
+namespace futures {
+namespace detail {
+template <class T>
+class FutureBase {
  public:
   typedef T value_type;
 
-  static SemiFuture<T> makeEmpty(); // equivalent to moved-from
-
-  // not copyable
-  SemiFuture(SemiFuture const&) = delete;
-  SemiFuture& operator=(SemiFuture const&) = delete;
-
-  // movable
-  SemiFuture(SemiFuture&&) noexcept;
-  SemiFuture& operator=(SemiFuture&&) noexcept;
-
-  // safe move-constructabilty from Future
-  /* implicit */ SemiFuture(Future<T>&&) noexcept;
-  SemiFuture& operator=(Future<T>&&) noexcept;
-
   /// Construct a Future from a value (perfect forwarding)
   template <
       class T2 = T,
       typename = typename std::enable_if<
           !isFuture<typename std::decay<T2>::type>::value>::type>
-  /* implicit */ SemiFuture(T2&& val);
+  /* implicit */ FutureBase(T2&& val);
 
   template <class T2 = T>
-  /* implicit */ SemiFuture(
-      typename std::enable_if<std::is_same<Unit, T2>::value>::type* = nullptr);
+  /* implicit */ FutureBase(
+      typename std::enable_if<std::is_same<Unit, T2>::value>::type*);
 
   template <
       class... Args,
       typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
           type = 0>
-  explicit SemiFuture(in_place_t, Args&&... args);
+  explicit FutureBase(in_place_t, Args&&... args);
+
+  FutureBase(FutureBase<T> const&) = delete;
+  FutureBase(SemiFuture<T>&&) noexcept;
+  FutureBase(Future<T>&&) noexcept;
 
-  ~SemiFuture();
+  // not copyable
+  FutureBase(Future<T> const&) = delete;
+  FutureBase(SemiFuture<T> const&) = delete;
+
+  ~FutureBase();
 
   /// Returns a reference to the result, with a reference category and const-
   /// qualification equivalent to the reference category and const-qualification
@@ -120,13 +119,6 @@ class SemiFuture {
       Executor* executor,
       int8_t priority = Executor::MID_PRI) &&;
 
-  /// This variant creates a new future, where the ref-qualifier && version
-  /// moves `this` out. This one is less efficient but avoids confusing users
-  /// when "return f.via(x);" fails.
-  inline Future<T> via(
-      Executor* executor,
-      int8_t priority = Executor::MID_PRI) &;
-
   /** True when the result (or exception) is ready. */
   bool isReady() const;
 
@@ -144,28 +136,6 @@ class SemiFuture {
   /// Note that this moves the Try<T> out.
   Optional<Try<T>> poll();
 
-  /// Block until the future is fulfilled. Returns the value (moved out), or
-  /// throws the exception. The future must not already have a callback.
-  T get();
-
-  /// Block until the future is fulfilled, or until timed out. Returns the
-  /// value (moved out), or throws the exception (which might be a TimedOut
-  /// exception).
-  T get(Duration dur);
-
-  /// Block until this Future is complete. Returns a reference to this Future.
-  SemiFuture<T>& wait() &;
-
-  /// Overload of wait() for rvalue Futures
-  SemiFuture<T>&& wait() &&;
-
-  /// Block until this Future is complete or until the given Duration passes.
-  /// Returns a reference to this Future
-  SemiFuture<T>& wait(Duration) &;
-
-  /// Overload of wait(Duration) for rvalue Futures
-  SemiFuture<T>&& wait(Duration) &&;
-
   /// This is not the method you're looking for.
   ///
   /// This needs to be public because it's used by make* and when*, and it's
@@ -199,25 +169,27 @@ class SemiFuture {
   }
 
  protected:
-  typedef futures::detail::Core<T>* corePtr;
+  friend class Promise<T>;
+  template <class>
+  friend class SemiFuture;
+  template <class>
+  friend class Future;
+
+  using corePtr = futures::detail::Core<T>*;
 
   // shared core state object
   corePtr core_;
 
-  explicit SemiFuture(corePtr obj) : core_(obj) {}
+  explicit FutureBase(corePtr obj) : core_(obj) {}
 
-  explicit SemiFuture(futures::detail::EmptyConstruct) noexcept;
+  explicit FutureBase(futures::detail::EmptyConstruct) noexcept;
 
   void detach();
 
   void throwIfInvalid() const;
 
-  friend class Promise<T>;
-  template <class>
-  friend class SemiFuture;
-
-  template <class T2>
-  friend SemiFuture<T2> makeSemiFuture(Try<T2>&&);
+  template <class FutureType>
+  void assign(FutureType&) noexcept;
 
   Executor* getExecutor() {
     return core_->getExecutor();
@@ -239,21 +211,133 @@ class SemiFuture {
   typename std::enable_if<R::ReturnsFuture::value, typename R::Return>::type
   thenImplementation(F&& func, futures::detail::argResult<isTry, F, Args...>);
 };
+} // namespace detail
+} // namespace futures
 
 template <class T>
-class Future : public SemiFuture<T> {
+class SemiFuture : private futures::detail::FutureBase<T> {
+ private:
+  using Base = futures::detail::FutureBase<T>;
+
  public:
-  typedef T value_type;
+  static SemiFuture<T> makeEmpty(); // equivalent to moved-from
 
-  static Future<T> makeEmpty(); // equivalent to moved-from
+  // Export public interface of FutureBase
+  // FutureBase is inherited privately to avoid subclasses being cast to
+  // a FutureBase pointer
+  using typename Base::value_type;
 
-  // not copyable
-  Future(Future const&) = delete;
-  Future& operator=(Future const&) = delete;
+  /// Construct a Future from a value (perfect forwarding)
+  template <
+      class T2 = T,
+      typename = typename std::enable_if<
+          !isFuture<typename std::decay<T2>::type>::value>::type>
+  /* implicit */ SemiFuture(T2&& val) : Base(std::forward<T2>(val)) {}
 
+  template <class T2 = T>
+  /* implicit */ SemiFuture(
+      typename std::enable_if<std::is_same<Unit, T2>::value>::type* p = nullptr)
+      : Base(p) {}
+
+  template <
+      class... Args,
+      typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
+          type = 0>
+  explicit SemiFuture(in_place_t, Args&&... args)
+      : Base(in_place, std::forward<Args>(args)...) {}
+
+  SemiFuture(SemiFuture<T> const&) = delete;
   // movable
-  Future(Future&&) noexcept;
-  Future& operator=(Future&&) noexcept;
+  SemiFuture(SemiFuture<T>&&) noexcept;
+  // safe move-constructabilty from Future
+  /* implicit */ SemiFuture(Future<T>&&) noexcept;
+
+  using Base::cancel;
+  using Base::getTry;
+  using Base::hasException;
+  using Base::hasValue;
+  using Base::isActive;
+  using Base::isReady;
+  using Base::poll;
+  using Base::raise;
+  using Base::setCallback_;
+  using Base::value;
+  using Base::via;
+
+  SemiFuture& operator=(SemiFuture const&) = delete;
+  SemiFuture& operator=(SemiFuture&&) noexcept;
+  SemiFuture& operator=(Future<T>&&) noexcept;
+
+  /// Block until the future is fulfilled. Returns the value (moved out), or
+  /// throws the exception. The future must not already have a callback.
+  T get() &&;
+
+  /// Block until the future is fulfilled, or until timed out. Returns the
+  /// value (moved out), or throws the exception (which might be a TimedOut
+  /// exception).
+  T get(Duration dur) &&;
+
+  /// Block until this Future is complete. Returns a reference to this Future.
+  SemiFuture<T>& wait() &;
+
+  /// Overload of wait() for rvalue Futures
+  SemiFuture<T>&& wait() &&;
+
+  /// Block until this Future is complete or until the given Duration passes.
+  /// Returns a reference to this Future
+  SemiFuture<T>& wait(Duration) &;
+
+  /// Overload of wait(Duration) for rvalue Futures
+  SemiFuture<T>&& wait(Duration) &&;
+
+ private:
+  template <class>
+  friend class futures::detail::FutureBase;
+
+  using typename Base::corePtr;
+
+  template <class T2>
+  friend SemiFuture<T2> makeSemiFuture(Try<T2>&&);
+
+  explicit SemiFuture(corePtr obj) : Base(obj) {}
+
+  explicit SemiFuture(futures::detail::EmptyConstruct) noexcept
+      : Base(futures::detail::EmptyConstruct{}) {}
+};
+
+template <class T>
+class Future : private futures::detail::FutureBase<T> {
+ private:
+  using Base = futures::detail::FutureBase<T>;
+
+ public:
+  // Export public interface of FutureBase
+  // FutureBase is inherited privately to avoid subclasses being cast to
+  // a FutureBase pointer
+  using typename Base::value_type;
+
+  /// Construct a Future from a value (perfect forwarding)
+  template <
+      class T2 = T,
+      typename = typename std::enable_if<
+          !isFuture<typename std::decay<T2>::type>::value>::type>
+  /* implicit */ Future(T2&& val) : Base(std::forward<T2>(val)) {}
+
+  template <class T2 = T>
+  /* implicit */ Future(
+      typename std::enable_if<std::is_same<Unit, T2>::value>::type* p = nullptr)
+      : Base(p) {}
+
+  template <
+      class... Args,
+      typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
+          type = 0>
+  explicit Future(in_place_t, Args&&... args)
+      : Base(in_place, std::forward<Args>(args)...) {}
+
+  Future(Future<T> const&) = delete;
+  // movable
+  Future(Future<T>&&) noexcept;
 
   // converting move
   template <
@@ -280,25 +364,25 @@ class Future : public SemiFuture<T> {
           int>::type = 0>
   Future& operator=(Future<T2>&&);
 
-  /// Construct a Future from a value (perfect forwarding)
-  template <
-      class T2 = T,
-      typename = typename std::enable_if<
-          !isFuture<typename std::decay<T2>::type>::value &&
-          !isSemiFuture<typename std::decay<T2>::type>::value>::type>
-  /* implicit */ Future(T2&& val);
+  using Base::cancel;
+  using Base::getTry;
+  using Base::hasException;
+  using Base::hasValue;
+  using Base::isActive;
+  using Base::isReady;
+  using Base::poll;
+  using Base::raise;
+  using Base::setCallback_;
+  using Base::value;
+  using Base::via;
 
-  template <class T2 = T>
-  /* implicit */ Future(
-      typename std::enable_if<std::is_same<Unit, T2>::value>::type* = nullptr);
+  static Future<T> makeEmpty(); // equivalent to moved-from
 
-  template <
-      class... Args,
-      typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
-          type = 0>
-  explicit Future(in_place_t, Args&&... args);
+  // not copyable
+  Future& operator=(Future const&) = delete;
 
-  ~Future();
+  // movable
+  Future& operator=(Future&&) noexcept;
 
   /// Call e->drive() repeatedly until the future is fulfilled. Examples
   /// of DrivableExecutor include EventBase and ManualExecutor. Returns a
@@ -313,9 +397,16 @@ class Future : public SemiFuture<T> {
   /// Unwraps the case of a Future<Future<T>> instance, and returns a simple
   /// Future<T> instance.
   template <class F = T>
-  typename std::enable_if<isFuture<F>::value,
-                          Future<typename isFuture<T>::Inner>>::type
-  unwrap();
+  typename std::
+      enable_if<isFuture<F>::value, Future<typename isFuture<T>::Inner>>::type
+      unwrap();
+
+  /// This variant creates a new future, where the ref-qualifier && version
+  /// moves `this` out. This one is less efficient but avoids confusing users
+  /// when "return f.via(x);" fails.
+  inline Future<T> via(
+      Executor* executor,
+      int8_t priority = Executor::MID_PRI) &;
 
   /** When this Future has completed, execute func which is a function that
     takes one of:
@@ -354,8 +445,9 @@ class Future : public SemiFuture<T> {
   ///
   ///   f1.then(std::bind(&Worker::doWork, w));
   template <typename R, typename Caller, typename... Args>
-  Future<typename isFuture<R>::Inner>
-  then(R(Caller::*func)(Args...), Caller *instance);
+  Future<typename isFuture<R>::Inner> then(
+      R (Caller::*func)(Args...),
+      Caller* instance);
 
   /// Execute the callback via the given Executor. The executor doesn't stick.
   ///
@@ -495,6 +587,15 @@ class Future : public SemiFuture<T> {
   /// now. The optional Timekeeper is as with futures::sleep().
   Future<T> delayed(Duration, Timekeeper* = nullptr);
 
+  /// Block until the future is fulfilled. Returns the value (moved out), or
+  /// throws the exception. The future must not already have a callback.
+  T get();
+
+  /// Block until the future is fulfilled, or until timed out. Returns the
+  /// value (moved out), or throws the exception (which might be a TimedOut
+  /// exception).
+  T get(Duration dur);
+
   /// Block until this Future is complete. Returns a reference to this Future.
   Future<T>& wait() &;
 
@@ -587,15 +688,18 @@ class Future : public SemiFuture<T> {
   }
 
  protected:
-  typedef futures::detail::Core<T>* corePtr;
+  friend class Promise<T>;
+  template <class>
+  friend class futures::detail::FutureBase;
+  template <class>
+  friend class Future;
 
-  explicit Future(corePtr obj) : SemiFuture<T>(obj) {}
+  using typename Base::corePtr;
 
-  explicit Future(futures::detail::EmptyConstruct) noexcept;
+  explicit Future(corePtr obj) : Base(obj) {}
 
-  friend class Promise<T>;
-  template <class> friend class Future;
-  friend class SemiFuture<T>;
+  explicit Future(futures::detail::EmptyConstruct) noexcept
+      : Base(futures::detail::EmptyConstruct{}) {}
 
   template <class T2>
   friend Future<T2> makeFuture(Try<T2>&&);
diff --git a/folly/futures/Promise.h b/folly/futures/Promise.h
index 905ce5dd..7bb58d1e 100644
--- a/folly/futures/Promise.h
+++ b/folly/futures/Promise.h
@@ -29,6 +29,8 @@ template <class T> class Future;
 
 namespace futures {
 namespace detail {
+template <class T>
+class FutureBase;
 struct EmptyConstruct {};
 template <typename T, typename F>
 class CoreCallbackState;
@@ -110,8 +112,11 @@ class Promise {
  private:
   typedef typename Future<T>::corePtr corePtr;
   template <class>
+  friend class futures::detail::FutureBase;
+  template <class>
   friend class SemiFuture;
-  template <class> friend class Future;
+  template <class>
+  friend class Future;
   template <class, class>
   friend class futures::detail::CoreCallbackState;
 
diff --git a/folly/futures/test/SemiFutureTest.cpp b/folly/futures/test/SemiFutureTest.cpp
index 1471d42b..29afb08f 100644
--- a/folly/futures/test/SemiFutureTest.cpp
+++ b/folly/futures/test/SemiFutureTest.cpp
@@ -203,7 +203,7 @@ TEST(SemiFuture, MakeFutureFromSemiFutureLValue) {
   Promise<int> p;
   std::atomic<int> result{0};
   auto f = SemiFuture<int>{p.getFuture()};
-  auto future = f.via(&e).then([&](int value) {
+  auto future = std::move(f).via(&e).then([&](int value) {
     result = value;
     return value;
   });