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

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 "

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

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

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);
+}