folly::Function: improve conversion of return types

Summary:Treat any return type as convertible to void:

As of C++17, std::function<void(Args...)> can be set to callables
returning non-void types when called with parameters Args....
This diff adds that capability to folly::Function. It also adds
unit tests, not only for ignoring return types, but also for
correctly converting between the return type of the embedded
callabled and the return type of the encapsulating folly::Function.

Allow conversion of one folly::Function type to another one which
declares a return type the original one can be converted to:

E.g. allow to construct a Function<double()> from a
Function<int()> or a Function<Base*()> from a
Function<Derived*()>.

Reviewed By: yfeldblum

Differential Revision: D3095583

fb-gh-sync-id: 6d924dc6e97f759d8109db4200e1cb9333a98d31
fbshipit-source-id: 6d924dc6e97f759d8109db4200e1cb9333a98d31

diff --git a/folly/Function-inl.h b/folly/Function-inl.h
index 05f790834448c1f56d90cdc604be9fe7e7b69b06..7c34f92cc07b330cde8b936256e9f2c7c8416c5b 100644 (file)
--- a/folly/Function-inl.h
+++ b/folly/Function-inl.h
@@ -271,22 +271,17 @@ template <
     typename OtherFunctionType,
     FunctionMoveCtor OtherNTM,
     size_t OtherEmbedFunctorSize>
-Function<FunctionType, NTM, EmbedFunctorSize>::
-    Function(
-        Function<OtherFunctionType,
-        OtherNTM,
-        OtherEmbedFunctorSize>&& other) noexcept(
-        OtherNTM == FunctionMoveCtor::NO_THROW &&
+Function<FunctionType, NTM, EmbedFunctorSize>::Function(
+    Function<OtherFunctionType, OtherNTM, OtherEmbedFunctorSize>&& other,
+    typename std::enable_if<std::is_same<
+        typename Traits::NonConstFunctionType,
+        typename detail::function::FunctionTypeTraits<
+            OtherFunctionType>::NonConstFunctionType>::value>::
+        type*) noexcept(OtherNTM == FunctionMoveCtor::NO_THROW &&
         EmbedFunctorSize >= OtherEmbedFunctorSize) {
   using OtherFunction =
       Function<OtherFunctionType, OtherNTM, OtherEmbedFunctorSize>;
 
-  static_assert(
-      std::is_same<
-          typename Traits::NonConstFunctionType,
-          typename OtherFunction::Traits::NonConstFunctionType>::value,
-      "Function: cannot move into a Function with different "
-      "parameter signature");
   static_assert(
       !Traits::IsConst::value || OtherFunction::Traits::IsConst::value,
       "Function: cannot move Function<R(Args...)> into "

diff --git a/folly/Function-pre.h b/folly/Function-pre.h
index 1196f57128cc2ed194608c5622f609257d25751e..293efbcb891fddf90efb19ae929c254e0a5119c4 100644 (file)
--- a/folly/Function-pre.h
+++ b/folly/Function-pre.h
@@ -34,6 +34,15 @@ struct SelectNonConstFunctionTag {
   using QualifiedPointer = T*;
 };
 
+// Helper to check whether the return type of a callable matches that of
+// a folly::Function object. Either because the former is convertible to
+// the latter, or the latter is void (possibly cv-qualified)
+template <typename CallableR, typename FollyFunctionR>
+using ReturnTypeMatches = std::integral_constant<
+    bool,
+    std::is_convertible<CallableR, FollyFunctionR>::value ||
+        std::is_same<typename std::decay<FollyFunctionR>::type, void>::value>;
+
 // Helper class to extract properties from a function type
 template <typename T>
 struct FunctionTypeTraits;
@@ -71,7 +80,7 @@ struct FunctionTypeTraits<R(Args...)> {
   using DefaultSelectFunctionTag = SelectNonConstFunctionTag;
   template <typename F>
   using IsCallable =
-      std::is_convertible<typename std::result_of<F&(Args...)>::type, R>;
+      ReturnTypeMatches<typename std::result_of<F&(Args...)>::type, R>;
   template <typename T>
   using QualifiedPointer = T*;
   template <typename Obj>
@@ -111,7 +120,7 @@ struct FunctionTypeTraits<R(Args...) const> {
   using DefaultSelectFunctionTag = SelectConstFunctionTag;
   template <typename F>
   using IsCallable =
-      std::is_convertible<typename std::result_of<F const&(Args...)>::type, R>;
+      ReturnTypeMatches<typename std::result_of<F const&(Args...)>::type, R>;
   template <typename T>
   using QualifiedPointer = T const*;
   template <typename Obj>
@@ -138,17 +147,27 @@ struct FunctionTypeTraits<R(Args...) const> {
   class ExecutorMixin;
 };
 
-// Helper template for checking if a type is a Function
-template <typename T>
-struct IsFunction : public std::false_type {};
-
-template <typename FunctionType, FunctionMoveCtor NTM, size_t EmbedFunctorSize>
-struct IsFunction<::folly::Function<FunctionType, NTM, EmbedFunctorSize>>
-    : public std::true_type {};
+// Helper template for checking if a type T is a Function with the same
+// function type as OtherFunctionType (except for const-ness which may differ)
+template <typename T, typename OtherFunctionType>
+struct IsFunction : std::false_type {};
+
+template <
+    typename FunctionType,
+    FunctionMoveCtor NTM,
+    size_t EmbedFunctorSize,
+    typename OtherFunctionType>
+struct IsFunction<
+    ::folly::Function<FunctionType, NTM, EmbedFunctorSize>,
+    OtherFunctionType>
+    : std::is_same<
+          typename FunctionTypeTraits<FunctionType>::NonConstFunctionType,
+          typename FunctionTypeTraits<
+              OtherFunctionType>::NonConstFunctionType> {};
 
 // Helper template to check if a functor can be called with arguments of type
-// Args..., if it returns a type convertible to R, and also is not a
-// Function.
+// Args..., if it returns a type convertible to R (or R is void), and also is
+// not a folly::Function.
 // Function objects can constructed or assigned from types for which
 // IsCallableHelper is true_type.
 template <typename FunctionType>
@@ -163,11 +182,12 @@ struct IsCallableHelper {
 };
 
 template <typename F, typename FunctionType>
-struct IsCallable : public std::integral_constant<
-                        bool,
-                        (!IsFunction<typename std::decay<F>::type>::value &&
-                         decltype(IsCallableHelper<FunctionType>::template test<
-                                  typename std::decay<F>::type>(0))::value)> {};
+struct IsCallable
+    : public std::integral_constant<
+          bool,
+          (!IsFunction<typename std::decay<F>::type, FunctionType>::value &&
+           decltype(IsCallableHelper<FunctionType>::template test<
+                    typename std::decay<F>::type>(0))::value)> {};
 
 // MaybeUnaryOrBinaryFunction: helper template class for deriving
 // Function from std::unary_function or std::binary_function
@@ -245,8 +265,8 @@ class FunctionTypeTraits<R(Args...)>::ExecutorMixin {
 
   template <typename Ex>
   static R invokeFunctor(ExecutorIf* executor, Args&&... args) {
-    return folly::detail::function::invoke(
-        *Ex::getFunctor(executor), std::forward<Args>(args)...);
+    return static_cast<R>(folly::detail::function::invoke(
+        *Ex::getFunctor(executor), std::forward<Args>(args)...));
   }
 
   // invokePtr is of type
@@ -282,8 +302,8 @@ class FunctionTypeTraits<R(Args...) const>::ExecutorMixin {
 
   template <typename Ex>
   static R invokeFunctor(ExecutorIf const* executor, Args&&... args) {
-    return folly::detail::function::invoke(
-        *Ex::getFunctor(executor), std::forward<Args>(args)...);
+    return static_cast<R>(folly::detail::function::invoke(
+        *Ex::getFunctor(executor), std::forward<Args>(args)...));
   }
 
   // invokePtr is of type

diff --git a/folly/Function.h b/folly/Function.h
index d33ec9ebb4f1e102c234908658adafc4529588f8..66cb96ba52ffada360e404f30822634c53d26ed4 100644 (file)
--- a/folly/Function.h
+++ b/folly/Function.h
@@ -308,10 +308,14 @@ class Function final
       typename OtherFunctionType,
       FunctionMoveCtor OtherNTM,
       size_t OtherEmbedFunctorSize>
-  Function(Function<OtherFunctionType, OtherNTM, OtherEmbedFunctorSize>&& other)
-  noexcept(
-      OtherNTM == FunctionMoveCtor::NO_THROW &&
-      EmbedFunctorSize >= OtherEmbedFunctorSize);
+  Function(
+      Function<OtherFunctionType, OtherNTM, OtherEmbedFunctorSize>&& other,
+      typename std::enable_if<std::is_same<
+          typename Traits::NonConstFunctionType,
+          typename detail::function::FunctionTypeTraits<
+              OtherFunctionType>::NonConstFunctionType>::value>::type* =
+          0) noexcept(OtherNTM == FunctionMoveCtor::NO_THROW &&
+          EmbedFunctorSize >= OtherEmbedFunctorSize);
 
   /**
    * Moves a `Function` with different template parameters with regards

diff --git a/folly/test/FunctionTest.cpp b/folly/test/FunctionTest.cpp
index 8019b59b62f899e5adf15c6a9eed48972112818f..718c00fc12988bcb3651bfd2feabcc8f8c8fbcf2 100644 (file)
--- a/folly/test/FunctionTest.cpp
+++ b/folly/test/FunctionTest.cpp
@@ -1081,3 +1081,124 @@ TEST(Function, SafeCaptureByReference) {
 
   EXPECT_EQ(sum, 999);
 }
+
+// TEST =====================================================================
+// IgnoreReturnValue
+
+TEST(Function, IgnoreReturnValue) {
+  int x = 95;
+
+  // Assign a lambda that return int to a folly::Function that returns void.
+  Function<void()> f = [&]() -> int { return ++x; };
+
+  EXPECT_EQ(x, 95);
+  f();
+  EXPECT_EQ(x, 96);
+
+  Function<int()> g = [&]() -> int { return ++x; };
+  Function<void()> cg = std::move(g);
+
+  EXPECT_EQ(x, 96);
+  cg();
+  EXPECT_EQ(x, 97);
+}
+
+// TEST =====================================================================
+// ReturnConvertible, ConvertReturnType
+
+TEST(Function, ReturnConvertible) {
+  struct CBase {
+    int x;
+  };
+  struct CDerived : CBase {};
+
+  Function<double()> f1 = []() -> int { return 5; };
+  EXPECT_EQ(f1(), 5.0);
+
+  Function<int()> f2 = []() -> double { return 5.2; };
+  EXPECT_EQ(f2(), 5);
+
+  CDerived derived;
+  derived.x = 55;
+
+  Function<CBase const&()> f3 = [&]() -> CDerived const& { return derived; };
+  EXPECT_EQ(f3().x, 55);
+
+  Function<CBase const&()> f4 = [&]() -> CDerived& { return derived; };
+  EXPECT_EQ(f4().x, 55);
+
+  Function<CBase&()> f5 = [&]() -> CDerived& { return derived; };
+  EXPECT_EQ(f5().x, 55);
+
+  Function<CBase const*()> f6 = [&]() -> CDerived const* { return &derived; };
+  EXPECT_EQ(f6()->x, 55);
+
+  Function<CBase const*()> f7 = [&]() -> CDerived* { return &derived; };
+  EXPECT_EQ(f7()->x, 55);
+
+  Function<CBase*()> f8 = [&]() -> CDerived* { return &derived; };
+  EXPECT_EQ(f8()->x, 55);
+
+  Function<CBase()> f9 = [&]() -> CDerived {
+    auto d = derived;
+    d.x = 66;
+    return d;
+  };
+  EXPECT_EQ(f9().x, 66);
+}
+
+TEST(Function, ConvertReturnType) {
+  struct CBase {
+    int x;
+  };
+  struct CDerived : CBase {};
+
+  Function<int()> f1 = []() -> int { return 5; };
+  Function<double()> cf1 = std::move(f1);
+  EXPECT_EQ(cf1(), 5.0);
+  Function<int()> ccf1 = std::move(cf1);
+  EXPECT_EQ(ccf1(), 5);
+
+  Function<double()> f2 = []() -> double { return 5.2; };
+  Function<int()> cf2 = std::move(f2);
+  EXPECT_EQ(cf2(), 5);
+  Function<double()> ccf2 = std::move(cf2);
+  EXPECT_EQ(ccf2(), 5.0);
+
+  CDerived derived;
+  derived.x = 55;
+
+  Function<CDerived const&()> f3 = [&]() -> CDerived const& { return derived; };
+  Function<CBase const&()> cf3 = std::move(f3);
+  EXPECT_EQ(cf3().x, 55);
+
+  Function<CDerived&()> f4 = [&]() -> CDerived& { return derived; };
+  Function<CBase const&()> cf4 = std::move(f4);
+  EXPECT_EQ(cf4().x, 55);
+
+  Function<CDerived&()> f5 = [&]() -> CDerived& { return derived; };
+  Function<CBase&()> cf5 = std::move(f5);
+  EXPECT_EQ(cf5().x, 55);
+
+  Function<CDerived const*()> f6 = [&]() -> CDerived const* {
+    return &derived;
+  };
+  Function<CBase const*()> cf6 = std::move(f6);
+  EXPECT_EQ(cf6()->x, 55);
+
+  Function<CDerived const*()> f7 = [&]() -> CDerived* { return &derived; };
+  Function<CBase const*()> cf7 = std::move(f7);
+  EXPECT_EQ(cf7()->x, 55);
+
+  Function<CDerived*()> f8 = [&]() -> CDerived* { return &derived; };
+  Function<CBase*()> cf8 = std::move(f8);
+  EXPECT_EQ(cf8()->x, 55);
+
+  Function<CDerived()> f9 = [&]() -> CDerived {
+    auto d = derived;
+    d.x = 66;
+    return d;
+  };
+  Function<CBase()> cf9 = std::move(f9);
+  EXPECT_EQ(cf9().x, 66);
+}