Add static_assert on mismatches of base::Bind args and the target params

This CL adds a static_assert to detect the Bind earlier and to emit an
readable error message.

Bug: 746955
Change-Id: Id8081be63f789c7896a9a4229fe3978a7fdedc41
Reviewed-on: https://chromium-review.googlesource.com/583976Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490741}

base/bind.h

@@ -24,10 +24,129 @@
 
 namespace base {
 
+namespace internal {
+
+// IsOnceCallback<T> is a std::true_type if |T| is a OnceCallback.
+template <typename T>
+struct IsOnceCallback : std::false_type {};
+
+template <typename Signature>
+struct IsOnceCallback<OnceCallback<Signature>> : std::true_type {};
+
+// Asserts |Param| is constructible from |Unwrapped|. |Arg| is here just to
+// show it in the compile error message as a hint to fix the error.
+template <size_t i, typename Arg, typename Unwrapped, typename Param>
+struct AssertConstructible {
+  static_assert(std::is_constructible<Param, Unwrapped>::value,
+                "|Param| needs to be constructible from |Unwrapped| type. "
+                "The failing argument is passed as the |i|th parameter, whose "
+                "type is |Arg|, and delivered as |Unwrapped| into |Param|.");
+};
+
+// Takes three same-length TypeLists, and applies AssertConstructible for each
+// triples.
+template <typename Index,
+          typename ArgsList,
+          typename UnwrappedTypeList,
+          typename ParamsList>
+struct AssertBindArgsValidity;
+
+template <size_t... Ns,
+          typename... Args,
+          typename... Unwrapped,
+          typename... Params>
+struct AssertBindArgsValidity<IndexSequence<Ns...>,
+                              TypeList<Args...>,
+                              TypeList<Unwrapped...>,
+                              TypeList<Params...>>
+    : AssertConstructible<Ns, Args, Unwrapped, Params>... {
+  static constexpr bool ok = true;
+};
+
+// The implementation of TransformToUnwrappedType below.
+template <RepeatMode, typename T>
+struct TransformToUnwrappedTypeImpl;
+
+template <typename T>
+struct TransformToUnwrappedTypeImpl<RepeatMode::Once, T> {
+  using StoredType = typename std::decay<T>::type;
+  using ForwardType = StoredType&&;
+  using Unwrapped = decltype(Unwrap(std::declval<ForwardType>()));
+};
+
+template <typename T>
+struct TransformToUnwrappedTypeImpl<RepeatMode::Repeating, T> {
+  using StoredType = typename std::decay<T>::type;
+  using ForwardType = const StoredType&;
+  using Unwrapped = decltype(Unwrap(std::declval<ForwardType>()));
+};
+
+// Transform |T| into `Unwrapped` type, which is passed to the target function.
+// Example:
+//   In repeat_mode == RepeatMode::Once case,
+//     `int&&` -> `int&&`,
+//     `const int&` -> `int&&`,
+//     `OwnedWrapper<int>&` -> `int*&&`.
+//   In repeat_mode == RepeatMode::Repeating case,
+//     `int&&` -> `const int&`,
+//     `const int&` -> `const int&`,
+//     `OwnedWrapper<int>&` -> `int* const &`.
+template <RepeatMode repeat_mode, typename T>
+using TransformToUnwrappedType =
+    typename TransformToUnwrappedTypeImpl<repeat_mode, T>::Unwrapped;
+
+// Transforms |Args| into `Unwrapped` types, and packs them into a TypeList.
+// If |is_method| is true, tries to dereference the first argument to support
+// smart pointers.
+template <RepeatMode repeat_mode, bool is_method, typename... Args>
+struct MakeUnwrappedTypeListImpl {
+  using Type = TypeList<TransformToUnwrappedType<repeat_mode, Args>...>;
+};
+
+// Performs special handling for this pointers.
+// Example:
+//   int* -> int*,
+//   std::unique_ptr<int> -> int*.
+template <RepeatMode repeat_mode, typename Receiver, typename... Args>
+struct MakeUnwrappedTypeListImpl<repeat_mode, true, Receiver, Args...> {
+  using UnwrappedReceiver = TransformToUnwrappedType<repeat_mode, Receiver>;
+  using Type = TypeList<decltype(&*std::declval<UnwrappedReceiver>()),
+                        TransformToUnwrappedType<repeat_mode, Args>...>;
+};
+
+template <RepeatMode repeat_mode, bool is_method, typename... Args>
+using MakeUnwrappedTypeList =
+    typename MakeUnwrappedTypeListImpl<repeat_mode, is_method, Args...>::Type;
+
+}  // namespace internal
+
 // Bind as OnceCallback.
 template <typename Functor, typename... Args>
 inline OnceCallback<MakeUnboundRunType<Functor, Args...>>
 BindOnce(Functor&& functor, Args&&... args) {
+  static_assert(
+      !internal::IsOnceCallback<typename std::decay<Functor>::type>() ||
+          (std::is_rvalue_reference<Functor&&>() &&
+           !std::is_const<typename std::remove_reference<Functor>::type>()),
+      "BindOnce requires non-const rvalue for OnceCallback binding."
+      " I.e.: base::BindOnce(std::move(callback)).");
+
+  // This block checks if each |args| matches to the corresponding params of the
+  // target function. This check does not affect the behavior of Bind, but its
+  // error message should be more readable.
+  using Helper = internal::BindTypeHelper<Functor, Args...>;
+  using FunctorTraits = typename Helper::FunctorTraits;
+  using BoundArgsList = typename Helper::BoundArgsList;
+  using UnwrappedArgsList =
+      internal::MakeUnwrappedTypeList<internal::RepeatMode::Once,
+                                      FunctorTraits::is_method, Args&&...>;
+  using BoundParamsList = typename Helper::BoundParamsList;
+  static_assert(
+      internal::AssertBindArgsValidity<MakeIndexSequence<Helper::num_bounds>,
+                                       BoundArgsList, UnwrappedArgsList,
+                                       BoundParamsList>::ok,
+      "The bound args need to be convertible to the target params.");
+
   using BindState = internal::MakeBindStateType<Functor, Args...>;
   using UnboundRunType = MakeUnboundRunType<Functor, Args...>;
   using Invoker = internal::Invoker<BindState, UnboundRunType>;
@@ -50,6 +169,26 @@ BindOnce(Functor&& functor, Args&&... args) {
 template <typename Functor, typename... Args>
 inline RepeatingCallback<MakeUnboundRunType<Functor, Args...>>
 BindRepeating(Functor&& functor, Args&&... args) {
+  static_assert(
+      !internal::IsOnceCallback<typename std::decay<Functor>::type>(),
+      "BindRepeating cannot bind OnceCallback. Use BindOnce with std::move().");
+
+  // This block checks if each |args| matches to the corresponding params of the
+  // target function. This check does not affect the behavior of Bind, but its
+  // error message should be more readable.
+  using Helper = internal::BindTypeHelper<Functor, Args...>;
+  using FunctorTraits = typename Helper::FunctorTraits;
+  using BoundArgsList = typename Helper::BoundArgsList;
+  using UnwrappedArgsList =
+      internal::MakeUnwrappedTypeList<internal::RepeatMode::Repeating,
+                                      FunctorTraits::is_method, Args&&...>;
+  using BoundParamsList = typename Helper::BoundParamsList;
+  static_assert(
+      internal::AssertBindArgsValidity<MakeIndexSequence<Helper::num_bounds>,
+                                       BoundArgsList, UnwrappedArgsList,
+                                       BoundParamsList>::ok,
+      "The bound args need to be convertible to the target params.");
+
   using BindState = internal::MakeBindStateType<Functor, Args...>;
   using UnboundRunType = MakeUnboundRunType<Functor, Args...>;
   using Invoker = internal::Invoker<BindState, UnboundRunType>;

base/bind_internal.h

@@ -243,6 +243,9 @@ struct FunctorTraits<Callback<R(Args...), copy_mode, repeat_mode>> {
   }
 };
 
+template <typename Functor>
+using MakeFunctorTraits = FunctorTraits<typename std::decay<Functor>::type>;
+
 // InvokeHelper<>
 //
 // There are 2 logical InvokeHelper<> specializations: normal, WeakCalls.
@@ -258,7 +261,7 @@ template <typename ReturnType>
 struct InvokeHelper<false, ReturnType> {
   template <typename Functor, typename... RunArgs>
   static inline ReturnType MakeItSo(Functor&& functor, RunArgs&&... args) {
-    using Traits = FunctorTraits<typename std::decay<Functor>::type>;
+    using Traits = MakeFunctorTraits<Functor>;
     return Traits::Invoke(std::forward<Functor>(functor),
                           std::forward<RunArgs>(args)...);
   }
@@ -278,7 +281,7 @@ struct InvokeHelper<true, ReturnType> {
                               RunArgs&&... args) {
     if (!weak_ptr)
       return;
-    using Traits = FunctorTraits<typename std::decay<Functor>::type>;
+    using Traits = MakeFunctorTraits<Functor>;
     Traits::Invoke(std::forward<Functor>(functor),
                    std::forward<BoundWeakPtr>(weak_ptr),
                    std::forward<RunArgs>(args)...);
@@ -325,8 +328,7 @@ struct Invoker<StorageType, R(UnboundArgs...)> {
                           BoundArgsTuple&& bound,
                           IndexSequence<indices...>,
                           UnboundArgs&&... unbound_args) {
-    static constexpr bool is_method =
-        FunctorTraits<typename std::decay<Functor>::type>::is_method;
+    static constexpr bool is_method = MakeFunctorTraits<Functor>::is_method;
 
     using DecayedArgsTuple = typename std::decay<BoundArgsTuple>::type;
     static constexpr bool is_weak_call =
@@ -342,16 +344,35 @@ struct Invoker<StorageType, R(UnboundArgs...)> {
   }
 };
 
-// Used to implement MakeUnboundRunType.
+// Extracts necessary type info from Functor and BoundArgs.
+// Used to implement MakeUnboundRunType, BindOnce and BindRepeating.
 template <typename Functor, typename... BoundArgs>
-struct MakeUnboundRunTypeImpl {
-  using RunType =
-      typename FunctorTraits<typename std::decay<Functor>::type>::RunType;
+struct BindTypeHelper {
+  static constexpr size_t num_bounds = sizeof...(BoundArgs);
+  using FunctorTraits = MakeFunctorTraits<Functor>;
+
+  // Example:
+  //   When Functor is `double (Foo::*)(int, const std::string&)`, and BoundArgs
+  //   is a template pack of `Foo*` and `int16_t`:
+  //    - RunType is `double(Foo*, int, const std::string&)`,
+  //    - ReturnType is `double`,
+  //    - RunParamsList is `TypeList<Foo*, int, const std::string&>`,
+  //    - BoundParamsList is `TypeList<Foo*, int>`,
+  //    - UnboundParamsList is `TypeList<const std::string&>`,
+  //    - BoundArgsList is `TypeList<Foo*, int16_t>`,
+  //    - UnboundRunType is `double(const std::string&)`.
+  using RunType = typename FunctorTraits::RunType;
   using ReturnType = ExtractReturnType<RunType>;
-  using Args = ExtractArgs<RunType>;
-  using UnboundArgs = DropTypeListItem<sizeof...(BoundArgs), Args>;
-  using Type = MakeFunctionType<ReturnType, UnboundArgs>;
+
+  using RunParamsList = ExtractArgs<RunType>;
+  using BoundParamsList = TakeTypeListItem<num_bounds, RunParamsList>;
+  using UnboundParamsList = DropTypeListItem<num_bounds, RunParamsList>;
+
+  using BoundArgsList = TypeList<BoundArgs...>;
+
+  using UnboundRunType = MakeFunctionType<ReturnType, UnboundParamsList>;
 };
+
 template <typename Functor>
 typename std::enable_if<FunctorTraits<Functor>::is_nullable, bool>::type
 IsNull(const Functor& functor) {
@@ -504,10 +525,10 @@ struct MakeBindStateTypeImpl<true, Functor, Receiver, BoundArgs...> {
 };
 
 template <typename Functor, typename... BoundArgs>
-using MakeBindStateType = typename MakeBindStateTypeImpl<
-    FunctorTraits<typename std::decay<Functor>::type>::is_method,
-    Functor,
-    BoundArgs...>::Type;
+using MakeBindStateType =
+    typename MakeBindStateTypeImpl<MakeFunctorTraits<Functor>::is_method,
+                                   Functor,
+                                   BoundArgs...>::Type;
 
 }  // namespace internal
 
@@ -515,7 +536,7 @@ using MakeBindStateType = typename MakeBindStateTypeImpl<
 // E.g. MakeUnboundRunType<R(A, B, C), A, B> is evaluated to R(C).
 template <typename Functor, typename... BoundArgs>
 using MakeUnboundRunType =
-    typename internal::MakeUnboundRunTypeImpl<Functor, BoundArgs...>::Type;
+    typename internal::BindTypeHelper<Functor, BoundArgs...>::UnboundRunType;
 
 }  // namespace base
 

base/bind_unittest.nc

@@ -70,7 +70,7 @@ template <typename T>
 void VoidPolymorphic1(T t) {
 }
 
-#if defined(NCTEST_METHOD_ON_CONST_OBJECT)  // [r"fatal error: call to pointer to member function of type 'void \(\)' drops 'const' qualifier"]
+#if defined(NCTEST_METHOD_ON_CONST_OBJECT)  // [r"fatal error: static_assert failed \"|Param| needs to be constructible from |Unwrapped| type\.\""]
 
 // Method bound to const-object.
 //
@@ -107,7 +107,7 @@ void WontCompile() {
   no_ref_const_cb.Run();
 }
 
-#elif defined(NCTEST_CONST_POINTER)  // [r"fatal error: cannot initialize a parameter of type 'base::NoRef \*' with an lvalue of type 'const base::NoRef \*const'"]
+#elif defined(NCTEST_CONST_POINTER)  // [r"fatal error: static_assert failed \"|Param| needs to be constructible from |Unwrapped| type\.\""]
 
 // Const argument used with non-const pointer parameter of same type.
 //
@@ -119,7 +119,7 @@ void WontCompile() {
   pointer_same_cb.Run();
 }
 
-#elif defined(NCTEST_CONST_POINTER_SUBTYPE)  // [r"fatal error: cannot initialize a parameter of type 'base::NoRefParent \*' with an lvalue of type 'const base::NoRefChild \*const'"]
+#elif defined(NCTEST_CONST_POINTER_SUBTYPE)  // [r"fatal error: static_assert failed \"|Param| needs to be constructible from |Unwrapped| type\.\""]
 
 // Const argument used with non-const pointer parameter of super type.
 //
@@ -148,7 +148,7 @@ void WontCompile() {
   ref_arg_cb.Run(p);
 }
 
-#elif defined(NCTEST_DISALLOW_BIND_TO_NON_CONST_REF_PARAM)  // [r"fatal error: binding value of type 'const base::Parent' to reference to type 'base::Parent' drops 'const' qualifier"]
+#elif defined(NCTEST_DISALLOW_BIND_TO_NON_CONST_REF_PARAM)  // [r"fatal error: static_assert failed \"|Param| needs to be constructible from |Unwrapped| type\.\""]
 
 // Binding functions with reference parameters, unsupported.
 //
@@ -272,6 +272,25 @@ void WontCompile() {
   std::move(cb).Run();
 }
 
+#elif defined(NCTEST_DISALLOW_BIND_ONCECALLBACK)  // [r"fatal error: static_assert failed \"BindRepeating cannot bind OnceCallback. Use BindOnce with std::move\(\)\.\""]
+
+void WontCompile() {
+  Bind(BindOnce([](int) {}), 42);
+}
+
+#elif defined(NCTEST_DISALLOW_BINDONCE_LVALUE_ONCECALLBACK)  // [r"fatal error: static_assert failed \"BindOnce requires non-const rvalue for OnceCallback binding\."]
+void WontCompile() {
+  auto cb = BindOnce([](int) {});
+  BindOnce(cb, 42);
+}
+
+#elif defined(NCTEST_DISALLOW_BINDONCE_RVALUE_CONST_ONCECALLBACK)  // [r"fatal error: static_assert failed \"BindOnce requires non-const rvalue for OnceCallback binding\."]
+
+void WontCompile() {
+  const auto cb = BindOnce([](int) {});
+  BindOnce(std::move(cb), 42);
+}
+
 #endif
 
 }  // namespace base