From 692cc4b1f5872a9ad802ba2af8eadee24f540480 Mon Sep 17 00:00:00 2001 From: Subodh Iyengar Date: Mon, 29 Feb 2016 19:38:32 -0800 Subject: [PATCH] Add conversion constructors for Future Summary:previously we were not able to do Future f = makeFuture(); 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 | 16 +++++- folly/futures/Future.h | 12 +++++ folly/futures/detail/Core.h | 24 +++++++++ folly/futures/test/ConversionTest.cpp | 70 +++++++++++++++++++++++++++ folly/test/Makefile.am | 1 + 5 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 folly/futures/test/ConversionTest.cpp diff --git a/folly/futures/Future-inl.h b/folly/futures/Future-inl.h index eabc61a5..7ada66e7 100644 --- a/folly/futures/Future-inl.h +++ b/folly/futures/Future-inl.h @@ -62,10 +62,24 @@ Future& Future::operator=(Future&& other) noexcept { return *this; } +template +template +Future::Future(Future&& other) noexcept + : core_(detail::Core::convert(other.core_)) { + other.core_ = nullptr; +} + +template +template +Future& Future::operator=(Future&& other) noexcept { + std::swap(core_, detail::Core::convert(other.core_)); + return *this; +} + template template Future::Future(T2&& val) - : core_(new detail::Core(Try(std::forward(val)))) {} + : core_(new detail::Core(Try(std::forward(val)))) {} template template diff --git a/folly/futures/Future.h b/folly/futures/Future.h index 5c66d5ac..cde04928 100644 --- a/folly/futures/Future.h +++ b/folly/futures/Future.h @@ -54,6 +54,18 @@ class Future { Future(Future&&) noexcept; Future& operator=(Future&&) noexcept; + // conversion constructors + template < + class U, + typename = typename std::enable_if::value && + sizeof(U) == sizeof(T)>::type> + /* implicit */ Future(Future&&) noexcept; + template < + class U, + typename = typename std::enable_if::value && + sizeof(U) == sizeof(T)>::type> + /* implicit */ Future& operator=(Future&&) noexcept; + /// Construct a Future from a value (perfect forwarding) template ) == size(Core). + // This assumption is used as a proxy to make sure that + // the members of Core and Core line up so that we can use a + // reinterpret cast. + template < + class U, + typename = typename std::enable_if::value && + sizeof(U) == sizeof(T)>::type> + static Core* convert(Core* from) { + return reinterpret_cast*>(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) == size(Core). + // See Core::convert for details. + // lambdaBuf occupies exactly one cache line static constexpr size_t lambdaBufSize = 8 * sizeof(void*); typename std::aligned_storage::type lambdaBuf_; diff --git a/folly/futures/test/ConversionTest.cpp b/folly/futures/test/ConversionTest.cpp new file mode 100644 index 00000000..53b15b1b --- /dev/null +++ b/folly/futures/test/ConversionTest.cpp @@ -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 + +#include +#include + +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> future = + makeFuture>(folly::make_unique("hello world")); + std::unique_ptr a = future.get(); + A* aptr = a.get(); + B* b = dynamic_cast(aptr); + EXPECT_NE(nullptr, b); + EXPECT_EQ("hello world", b->getMsg()); +} + +TEST(Convert, subclassAssign) { + Future> f1 = + makeFuture>(folly::make_unique("hello world")); + Future> f2 = std::move(f1); + std::unique_ptr a = f2.get(); + A* aptr = a.get(); + B* b = dynamic_cast(aptr); + EXPECT_NE(nullptr, b); + EXPECT_EQ("hello world", b->getMsg()); +} + +TEST(Convert, ConversionTests) { + static_assert(std::is_convertible>, + Future>>::value, + "Unique ptr not convertible"); + static_assert(!std::is_convertible>, + Future>>::value, + "Underlying types are not convertible"); + static_assert(!std::is_convertible, Future>::value, + "Underlying types are not the same size"); + static_assert(sizeof(detail::Core>) == + sizeof(detail::Core>), + "Sizes of types are not the same"); +} diff --git a/folly/test/Makefile.am b/folly/test/Makefile.am index e5414567..0843c104 100644 --- a/folly/test/Makefile.am +++ b/folly/test/Makefile.am @@ -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 \ -- 2.34.1