Add conversion constructors for Future
authorSubodh Iyengar <subodh@fb.com>
Tue, 1 Mar 2016 03:38:32 +0000 (19:38 -0800)
committerFacebook Github Bot 1 <facebook-github-bot-1-bot@fb.com>
Tue, 1 Mar 2016 03:50:33 +0000 (19:50 -0800)
Summary:previously we were not able to do

Future<Base> f = makeFuture<Derived>();

This is because Future did not declare a conversion
constructor.

Adding a proper conversion ctor for Future is very
tricky because of the way Core is managed under the hood.
Core is not movable, and cannot be moved otherwise we
would have to retain pointers to the Future and Promises which
pointed to a particular core. This would be inefficient.

Instead we compromise and allow a very small subset of conversions
from objects whose templated types are convertible and also the
sizes of their Cores are also the same. As a result, we can
convert between types like unique_ptrs however not always between
full objects.

Reviewed By: jsedgwick

Differential Revision: D2943775

fb-gh-sync-id: 7c2388f2fb49d789c80ae2477814e960a099771b
shipit-source-id: 7c2388f2fb49d789c80ae2477814e960a099771b

folly/futures/Future-inl.h
folly/futures/Future.h
folly/futures/detail/Core.h
folly/futures/test/ConversionTest.cpp [new file with mode: 0644]
folly/test/Makefile.am

index eabc61a524ded3835d0ee2990dee146c5ee0c4d0..7ada66e7d8a2cf2651f55170e9eea302156dde4a 100644 (file)
@@ -62,10 +62,24 @@ Future<T>& Future<T>::operator=(Future<T>&& other) noexcept {
   return *this;
 }
 
+template <class T>
+template <typename U, typename>
+Future<T>::Future(Future<U>&& other) noexcept
+    : core_(detail::Core<T>::convert(other.core_)) {
+  other.core_ = nullptr;
+}
+
+template <class T>
+template <typename U, typename>
+Future<T>& Future<T>::operator=(Future<U>&& other) noexcept {
+  std::swap(core_, detail::Core<T>::convert(other.core_));
+  return *this;
+}
+
 template <class T>
 template <class T2, typename>
 Future<T>::Future(T2&& val)
-  : core_(new detail::Core<T>(Try<T>(std::forward<T2>(val)))) {}
+    : core_(new detail::Core<T>(Try<T>(std::forward<T2>(val)))) {}
 
 template <class T>
 template <typename, typename>
index 5c66d5ace0f0ec9855b6ee77e6ba28b717af6aa9..cde04928b7ea2ed834cf3abce50021d2ec393597 100644 (file)
@@ -54,6 +54,18 @@ class Future {
   Future(Future&&) noexcept;
   Future& operator=(Future&&) noexcept;
 
+  // conversion constructors
+  template <
+      class U,
+      typename = typename std::enable_if<std::is_convertible<U, T>::value &&
+                                         sizeof(U) == sizeof(T)>::type>
+  /* implicit */ Future(Future<U>&&) noexcept;
+  template <
+      class U,
+      typename = typename std::enable_if<std::is_convertible<U, T>::value &&
+                                         sizeof(U) == sizeof(T)>::type>
+  /* implicit */ Future& operator=(Future<U>&&) noexcept;
+
   /// Construct a Future from a value (perfect forwarding)
   template <class T2 = T, typename =
             typename std::enable_if<
index 74c9ef28795ebe9b1bb291b7f61701d48a759c6c..70f887d2365ccba8de93116ddd1531f9f16c5537 100644 (file)
@@ -99,6 +99,26 @@ class Core {
   Core(Core&&) noexcept = delete;
   Core& operator=(Core&&) = delete;
 
+  // Core is assumed to be convertible only if the type is convertible
+  // and the size is the same. This is a compromise for the complexity
+  // of having to make Core truly have a conversion constructor which
+  // would cause various other problems.
+  // If we made Core move constructible then we would need to update the
+  // Promise and Future with the location of the new Core. This is complex
+  // and may be inefficient.
+  // Core should only be modified so that for size(T) == size(U),
+  // sizeof(Core<T>) == size(Core<U>).
+  // This assumption is used as a proxy to make sure that
+  // the members of Core<T> and Core<U> line up so that we can use a
+  // reinterpret cast.
+  template <
+      class U,
+      typename = typename std::enable_if<std::is_convertible<U, T>::value &&
+                                         sizeof(U) == sizeof(T)>::type>
+  static Core<T>* convert(Core<U>* from) {
+    return reinterpret_cast<Core<T>*>(from);
+  }
+
   /// May call from any thread
   bool hasResult() const {
     switch (fsm_.getState()) {
@@ -371,6 +391,10 @@ class Core {
     }
   }
 
+  // Core should only be modified so that for size(T) == size(U),
+  // sizeof(Core<T>) == size(Core<U>).
+  // See Core::convert for details.
+
   // lambdaBuf occupies exactly one cache line
   static constexpr size_t lambdaBufSize = 8 * sizeof(void*);
   typename std::aligned_storage<lambdaBufSize>::type lambdaBuf_;
diff --git a/folly/futures/test/ConversionTest.cpp b/folly/futures/test/ConversionTest.cpp
new file mode 100644 (file)
index 0000000..53b15b1
--- /dev/null
@@ -0,0 +1,70 @@
+/*
+ * Copyright 2016 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <folly/futures/Future.h>
+#include <type_traits>
+
+using namespace folly;
+
+class A {
+ public:
+  virtual ~A() {}
+};
+
+class B : public A {
+ public:
+  explicit B(std::string msg) : msg_(msg) {}
+
+  const std::string& getMsg() { return msg_; }
+  std::string msg_;
+};
+
+TEST(Convert, subclassConstruct) {
+  Future<std::unique_ptr<A>> future =
+      makeFuture<std::unique_ptr<B>>(folly::make_unique<B>("hello world"));
+  std::unique_ptr<A> a = future.get();
+  A* aptr = a.get();
+  B* b = dynamic_cast<B*>(aptr);
+  EXPECT_NE(nullptr, b);
+  EXPECT_EQ("hello world", b->getMsg());
+}
+
+TEST(Convert, subclassAssign) {
+  Future<std::unique_ptr<B>> f1 =
+      makeFuture<std::unique_ptr<B>>(folly::make_unique<B>("hello world"));
+  Future<std::unique_ptr<A>> f2 = std::move(f1);
+  std::unique_ptr<A> a = f2.get();
+  A* aptr = a.get();
+  B* b = dynamic_cast<B*>(aptr);
+  EXPECT_NE(nullptr, b);
+  EXPECT_EQ("hello world", b->getMsg());
+}
+
+TEST(Convert, ConversionTests) {
+  static_assert(std::is_convertible<Future<std::unique_ptr<B>>,
+                                    Future<std::unique_ptr<A>>>::value,
+                "Unique ptr not convertible");
+  static_assert(!std::is_convertible<Future<std::unique_ptr<A>>,
+                                     Future<std::unique_ptr<B>>>::value,
+                "Underlying types are not convertible");
+  static_assert(!std::is_convertible<Future<B>, Future<A>>::value,
+                "Underlying types are not the same size");
+  static_assert(sizeof(detail::Core<std::unique_ptr<A>>) ==
+                    sizeof(detail::Core<std::unique_ptr<B>>),
+                "Sizes of types are not the same");
+}
index e5414567a1d306751b430d5ca311c0836dff2069..0843c104b444119325fc78d81514ebb0d26424fa 100644 (file)
@@ -215,6 +215,7 @@ TESTS += indestructible_test
 futures_test_SOURCES = \
     ../futures/test/CollectTest.cpp \
     ../futures/test/ContextTest.cpp \
+    ../futures/test/ConversionTest.cpp \
     ../futures/test/CoreTest.cpp \
     ../futures/test/ThreadedExecutorTest.cpp \
     ../futures/test/EnsureTest.cpp \