Remove `Runnable` concept from base/bind_internal.h

base/bind_internal.h

Index: base/bind_internal.h
diff --git a/base/bind_internal.h b/base/bind_internal.h
index c7d6c3b4adc5f52e05dabd3bfd2d31e13e2bae84..9267b52c8566de849f8b486ee75b6cf83918402b 100644
--- a/base/bind_internal.h
+++ b/base/bind_internal.h
@@ -18,10 +18,6 @@
 #include "base/tuple.h"
 #include "build/build_config.h"
 
-#if defined(OS_WIN)
-#include "base/bind_internal_win.h"
-#endif
-
 namespace base {
 namespace internal {
 
@@ -29,40 +25,26 @@ namespace internal {
 //
 //
 // CONCEPTS:
-//  Runnable -- A type (really a type class) that has a single Run() method
-//              and a RunType typedef that corresponds to the type of Run().
-//              A Runnable can declare that it should treated like a method
-//              call by including a typedef named IsMethod.  The value of
-//              this typedef is NOT inspected, only the existence.  When a
-//              Runnable declares itself a method, Bind() will enforce special
-//              refcounting + WeakPtr handling semantics for the first
-//              parameter which is expected to be an object.
-//  Functor -- A copyable type representing something that should be called.
-//             All function pointers, Callback<>, and Runnables are functors
-//             even if the invocation syntax differs.
+//  Functor -- A movable type representing something that should be called.
+//             All function pointers and Callback<> are functors even if the
+//             invocation syntax differs.
 //  RunType -- A function type (as opposed to function _pointer_ type) for
 //             a Run() function.  Usually just a convenience typedef.

[Yuta Kitamura, 2016/06/30 07:13:29]

Apparently Run() in this context is no more?

[tzik, 2016/07/01 04:11:00]

Changed this to Callback<>::Run().

 //  (Bound)Args -- A set of types that stores the arguments.
 //
 // Types:
-//  RunnableAdapter<> -- Wraps the various "function" pointer types into an
-//                       object that adheres to the Runnable interface.
 //  ForceVoidReturn<> -- Helper class for translating function signatures to
 //                       equivalent forms with a "void" return type.
-//  FunctorTraits<> -- Type traits used determine the correct RunType and
-//                     RunnableType for a Functor.  This is where function
+//  FunctorTraits<> -- Type traits used to determine the correct RunType and
+//                     invocation manner for a Functor.  This is where function
 //                     signature adapters are applied.
-//  MakeRunnable<> -- Takes a Functor and returns an object in the Runnable
-//                    type class that represents the underlying Functor.
-//  InvokeHelper<> -- Take a Runnable + arguments and actully invokes it.
+//  InvokeHelper<> -- Take a Functor + arguments and actully invokes it.
 //                    Handle the differing syntaxes needed for WeakPtr<>
-//                    support, and for ignoring return values.  This is separate
-//                    from Invoker to avoid creating multiple version of
-//                    Invoker<>.
-//  Invoker<> -- Unwraps the curried parameters and executes the Runnable.
+//                    support.  This is separate from Invoker to avoid creating
+//                    multiple version of Invoker<>.
+//  Invoker<> -- Unwraps the curried parameters and executes the Functor.
 //  BindState<> -- Stores the curried parameters, and is the main entry point
-//                 into the Bind() system, doing most of the type resolution.
-//                 There are ARITY BindState types.
+//                 into the Bind() system.
 
 // HasRefCountedTypeAsRawPtr selects true_type when any of the |Args| is a raw
 // pointer to a RefCounted type.
@@ -80,239 +62,143 @@ struct HasRefCountedTypeAsRawPtr<T, Args...>
                        std::true_type,
                        HasRefCountedTypeAsRawPtr<Args...>>::type {};
 
-// BindsArrayToFirstArg selects true_type when |is_method| is true and the first
-// item of |Args| is an array type.
-// Implementation note: This non-specialized case handles !is_method case and
-// zero-arity case only.  Other cases should be handled by the specialization
-// below.
-template <bool is_method, typename... Args>
-struct BindsArrayToFirstArg : std::false_type {};
-
-template <typename T, typename... Args>
-struct BindsArrayToFirstArg<true, T, Args...>
-    : std::is_array<typename std::remove_reference<T>::type> {};
-
-// HasRefCountedParamAsRawPtr is the same to HasRefCountedTypeAsRawPtr except
-// when |is_method| is true HasRefCountedParamAsRawPtr skips the first argument.
-// Implementation note: This non-specialized case handles !is_method case and
-// zero-arity case only.  Other cases should be handled by the specialization
-// below.
-template <bool is_method, typename... Args>
-struct HasRefCountedParamAsRawPtr : HasRefCountedTypeAsRawPtr<Args...> {};
+// ForceVoidReturn<>
+//
+// Set of templates that support forcing the function return type to void.
+template <typename Sig>
+struct ForceVoidReturn;
 
-template <typename T, typename... Args>
-struct HasRefCountedParamAsRawPtr<true, T, Args...>
-    : HasRefCountedTypeAsRawPtr<Args...> {};
+template <typename R, typename... Args>
+struct ForceVoidReturn<R(Args...)> {
+  using RunType = void(Args...);
+};
 
-// RunnableAdapter<>
-//
-// The RunnableAdapter<> templates provide a uniform interface for invoking
-// a function pointer, method pointer, or const method pointer. The adapter
-// exposes a Run() method with an appropriate signature. Using this wrapper
-// allows for writing code that supports all three pointer types without
-// undue repetition.  Without it, a lot of code would need to be repeated 3
-// times.
-//
-// For method pointers and const method pointers the first argument to Run()
-// is considered to be the received of the method.  This is similar to STL's
-// mem_fun().
-//
-// This class also exposes a RunType typedef that is the function type of the
-// Run() function.
+// FunctorTraits<>
 //
-// If and only if the wrapper contains a method or const method pointer, an
-// IsMethod typedef is exposed.  The existence of this typedef (NOT the value)
-// marks that the wrapper should be considered a method wrapper.
-
+// See description at top of file.
 template <typename Functor>
-class RunnableAdapter;
+struct FunctorTraits;
 
-// Function.
+// For functions.
 template <typename R, typename... Args>
-class RunnableAdapter<R(*)(Args...)> {
- public:
-  // MSVC 2013 doesn't support Type Alias of function types.
-  // Revisit this after we update it to newer version.
-  typedef R RunType(Args...);
-
-  explicit RunnableAdapter(R(*function)(Args...))
-      : function_(function) {
-  }
+struct FunctorTraits<R (*)(Args...)> {
+  using RunType = R(Args...);
+  static constexpr bool is_method = false;
 
   template <typename... RunArgs>
-  R Run(RunArgs&&... args) const {
-    return function_(std::forward<RunArgs>(args)...);
+  static R Invoke(R (*function)(Args...), RunArgs&&... args) {
+    return function(std::forward<RunArgs>(args)...);
   }
-
- private:
-  R (*function_)(Args...);
 };
 
-// Method.
-template <typename R, typename T, typename... Args>
-class RunnableAdapter<R(T::*)(Args...)> {
- public:
-  // MSVC 2013 doesn't support Type Alias of function types.
-  // Revisit this after we update it to newer version.
-  typedef R RunType(T*, Args...);
-  using IsMethod = std::true_type;
+#if defined(OS_WIN) && !defined(ARCH_CPU_X86_64)
 
-  explicit RunnableAdapter(R(T::*method)(Args...))
-      : method_(method) {
-  }
+// For functions.
+template <typename R, typename... Args>
+struct FunctorTraits<R(__stdcall*)(Args...)> {
+  using RunType = R(Args...);
+  static constexpr bool is_method = false;
 
-  template <typename Receiver, typename... RunArgs>
-  R Run(Receiver&& receiver_ptr, RunArgs&&... args) const {
-    // Clang skips CV qualifier check on a method pointer invocation when the
-    // receiver is a subclass. Store the receiver into a const reference to
-    // T to ensure the CV check works.
-    // https://llvm.org/bugs/show_bug.cgi?id=27037
-    T& receiver = *receiver_ptr;
-    return (receiver.*method_)(std::forward<RunArgs>(args)...);
+  template <typename... RunArgs>
+  static R Invoke(R(__stdcall* function)(Args...), RunArgs&&... args) {
+    return function(std::forward<RunArgs>(args)...);
   }
-
- private:
-  R (T::*method_)(Args...);
 };
 
-// Const Method.
-template <typename R, typename T, typename... Args>
-class RunnableAdapter<R(T::*)(Args...) const> {
- public:
-  using RunType = R(const T*, Args...);
-  using IsMethod = std::true_type;
-
-  explicit RunnableAdapter(R(T::*method)(Args...) const)
-      : method_(method) {
-  }
+// For functions.
+template <typename R, typename... Args>
+struct FunctorTraits<R(__fastcall*)(Args...)> {
+  using RunType = R(Args...);
+  static constexpr bool is_method = false;
 
-  template <typename Receiver, typename... RunArgs>
-  R Run(Receiver&& receiver_ptr, RunArgs&&... args) const {
-    // Clang skips CV qualifier check on a method pointer invocation when the
-    // receiver is a subclass. Store the receiver into a unqualified reference
-    // to T to ensure the CV check works.
-    // https://llvm.org/bugs/show_bug.cgi?id=27037
-    const T& receiver = *receiver_ptr;
-    return (receiver.*method_)(std::forward<RunArgs>(args)...);
+  template <typename... RunArgs>
+  static R Invoke(R(__fastcall* function)(Args...), RunArgs&&... args) {
+    return function(std::forward<RunArgs>(args)...);
   }
-
- private:
-  R (T::*method_)(Args...) const;
 };
 
+#endif  // defined(OS_WIN) && !defined(ARCH_CPU_X86_64)
 
-// ForceVoidReturn<>
-//
-// Set of templates that support forcing the function return type to void.
-template <typename Sig>
-struct ForceVoidReturn;
+// For methods.
+template <typename R, typename Receiver, typename... Args>
+struct FunctorTraits<R (Receiver::*)(Args...)> {
+  using RunType = R(Receiver*, Args...);
+  static constexpr bool is_method = true;
 
-template <typename R, typename... Args>
-struct ForceVoidReturn<R(Args...)> {
-  // MSVC 2013 doesn't support Type Alias of function types.
-  // Revisit this after we update it to newer version.
-  typedef void RunType(Args...);
+  template <typename ReceiverPtr, typename... RunArgs>
+  static R Invoke(R (Receiver::*method)(Args...),
+                  ReceiverPtr&& receiver_ptr,
+                  RunArgs&&... args) {

[dcheng, 2016/06/30 12:53:36]

Is the clang CV qualifier check comment in the old RunnableAdapter impl
obsolete?

[tzik, 2016/07/01 04:11:00]

Oh, I missed to move it. It's still valid, and NCTEST_METHOD_ON_CONST_OBJECT in
bind_unittest.nc catches it.
Updated the comment.

+    Receiver& receiver = *receiver_ptr;
+    return (receiver.*method)(std::forward<RunArgs>(args)...);
+  }
 };
 
-
-// FunctorTraits<>
-//
-// See description at top of file.
-template <typename T>
-struct FunctorTraits {
-  using RunnableType = RunnableAdapter<T>;
-  using RunType = typename RunnableType::RunType;
+// For const methods.
+template <typename R, typename Receiver, typename... Args>
+struct FunctorTraits<R (Receiver::*)(Args...) const> {
+  using RunType = R(const Receiver*, Args...);
+  static constexpr bool is_method = true;
+
+  template <typename ReceiverPtr, typename... RunArgs>
+  static R Invoke(R (Receiver::*method)(Args...) const,
+                  ReceiverPtr&& receiver_ptr,
+                  RunArgs&&... args) {
+    const Receiver& receiver = *receiver_ptr;
+    return (receiver.*method)(std::forward<RunArgs>(args)...);
+  }
 };
 
+// For IgnoreResults.
 template <typename T>
-struct FunctorTraits<IgnoreResultHelper<T>> {
-  using RunnableType = typename FunctorTraits<T>::RunnableType;
+struct FunctorTraits<IgnoreResultHelper<T>> : FunctorTraits<T> {
   using RunType =
-      typename ForceVoidReturn<typename RunnableType::RunType>::RunType;
-};
+      typename ForceVoidReturn<typename FunctorTraits<T>::RunType>::RunType;
 
-template <typename T>
-struct FunctorTraits<Callback<T>> {
-  using RunnableType = Callback<T> ;
-  using RunType = typename Callback<T>::RunType;
+  template <typename IgnoreResultType, typename... RunArgs>
+  static void Invoke(IgnoreResultType&& ignore_result_helper,
+                     RunArgs&&... args) {
+    FunctorTraits<T>::Invoke(ignore_result_helper.functor_,
+                             std::forward<RunArgs>(args)...);
+  }
 };
 
-
-// MakeRunnable<>
-//
-// Converts a passed in functor to a RunnableType using type inference.
-
-template <typename T>
-typename FunctorTraits<T>::RunnableType MakeRunnable(const T& t) {
-  return RunnableAdapter<T>(t);
-}
-
-template <typename T>
-typename FunctorTraits<T>::RunnableType
-MakeRunnable(const IgnoreResultHelper<T>& t) {
-  return MakeRunnable(t.functor_);
-}
-
-template <typename T>
-const typename FunctorTraits<Callback<T>>::RunnableType&
-MakeRunnable(const Callback<T>& t) {
-  DCHECK(!t.is_null());
-  return t;
-}
+// For Callbacks.
+template <typename R, typename... Args, CopyMode copy_mode>
+struct FunctorTraits<Callback<R(Args...), copy_mode>> {
+  using RunType = R(Args...);
+  static constexpr bool is_method = false;
+
+  template <typename CallbackType, typename... RunArgs>
+  static R Invoke(CallbackType&& callback, RunArgs&&... args) {
+    DCHECK(!callback.is_null());
+    return std::forward<CallbackType>(callback).Run(
+        std::forward<RunArgs>(args)...);
+  }
+};
 
 
 // InvokeHelper<>
 //
-// There are 3 logical InvokeHelper<> specializations: normal, void-return,
-// WeakCalls.
+// There are 2 logical InvokeHelper<> specializations: normal, WeakCalls.
 //
 // The normal type just calls the underlying runnable.
 //
-// We need a InvokeHelper to handle void return types in order to support
-// IgnoreResult().  Normally, if the Runnable's RunType had a void return,
-// the template system would just accept "return functor.Run()" ignoring
-// the fact that a void function is being used with return. This piece of
-// sugar breaks though when the Runnable's RunType is not void.  Thus, we
-// need a partial specialization to change the syntax to drop the "return"
-// from the invocation call.
-//
-// WeakCalls similarly need special syntax that is applied to the first
-// argument to check if they should no-op themselves.
+// WeakCalls need special syntax that is applied to the first argument to check
+// if they should no-op themselves.
 template <bool is_weak_call, typename ReturnType>
 struct InvokeHelper;
 
 template <typename ReturnType>
 struct InvokeHelper<false, ReturnType> {
-  template <typename Runnable, typename... RunArgs>
-  static inline ReturnType MakeItSo(Runnable&& runnable, RunArgs&&... args) {
-    return std::forward<Runnable>(runnable).Run(std::forward<RunArgs>(args)...);
-  }
-};
-
-template <>
-struct InvokeHelper<false, void> {
-  template <typename Runnable, typename... RunArgs>
-  static inline void MakeItSo(Runnable&& runnable, RunArgs&&... args) {
-    std::forward<Runnable>(runnable).Run(std::forward<RunArgs>(args)...);
+  template <typename Functor, typename... RunArgs>
+  static inline ReturnType MakeItSo(Functor&& functor, RunArgs&&... args) {
+    using Traits = FunctorTraits<typename std::decay<Functor>::type>;
+    return Traits::Invoke(std::forward<Functor>(functor),
+                          std::forward<RunArgs>(args)...);
   }
 };
 
-template <>
-struct InvokeHelper<true, void> {
-  template <typename Runnable, typename BoundWeakPtr, typename... RunArgs>
-  static void MakeItSo(Runnable&& runnable,
-                       BoundWeakPtr&& weak_ptr,
-                       RunArgs&&... args) {
-    if (!weak_ptr) {
-      return;
-    }
-    std::forward<Runnable>(runnable).Run(
-        std::forward<BoundWeakPtr>(weak_ptr), std::forward<RunArgs>(args)...);
-  }
-};
-
-#if !defined(_MSC_VER)
-
 template <typename ReturnType>
 struct InvokeHelper<true, ReturnType> {
   // WeakCalls are only supported for functions with a void return type.
@@ -320,9 +206,19 @@ struct InvokeHelper<true, ReturnType> {
   // is invalidated.
   static_assert(std::is_void<ReturnType>::value,
                 "weak_ptrs can only bind to methods without return values");
-};
 
-#endif
+  template <typename Functor, typename BoundWeakPtr, typename... RunArgs>
+  static inline void MakeItSo(Functor&& functor,
+                              BoundWeakPtr weak_ptr,
+                              RunArgs&&... args) {
+    if (!weak_ptr)
+      return;
+    using Traits = FunctorTraits<typename std::decay<Functor>::type>;
+    Traits::Invoke(std::forward<Functor>(functor),
+                   std::forward<BoundWeakPtr>(weak_ptr),
+                   std::forward<RunArgs>(args)...);
+  }
+};
 
 // Invoker<>
 //
@@ -339,19 +235,20 @@ struct Invoker<StorageType, R(UnboundArgs...)> {
     const StorageType* storage = static_cast<StorageType*>(base);
     static constexpr size_t num_bound_args =
         std::tuple_size<decltype(storage->bound_args_)>::value;
-    return RunImpl(storage->runnable_,
+    return RunImpl(storage->functor_,
                    storage->bound_args_,
                    MakeIndexSequence<num_bound_args>(),
                    std::forward<UnboundArgs>(unbound_args)...);
   }
 
-  template <typename Runnable, typename BoundArgsTuple, size_t... indices>
-  static inline R RunImpl(Runnable&& runnable,
+ private:
+  template <typename Functor, typename BoundArgsTuple, size_t... indices>
+  static inline R RunImpl(Functor&& functor,
                           BoundArgsTuple&& bound,

[Yuta Kitamura, 2016/06/30 07:13:29]

Both |functor| and |bound| are always lvalue in this case, aren't they?
Especially, std::forward() calls below seem just passing an lvalue
reference; if that's the case, std::forward() call is an excessive
decoration.

(This has existed before your change, so it's okay to ignore.)

[tzik, 2016/07/01 04:11:00]

Good point. But let me leave this as is for future use.
I'm going to add another variant of Invoker::Run in next CL,
Invoker::RunOneShot, that will pass rvalue references to RunImpl.

                           IndexSequence<indices...>,
                           UnboundArgs&&... unbound_args) {
     static constexpr bool is_method =
-        HasIsMethodTag<typename std::decay<Runnable>::type>::value;
+        FunctorTraits<typename std::decay<Functor>::type>::is_method;
 
     using DecayedArgsTuple = typename std::decay<BoundArgsTuple>::type;
     static constexpr bool is_weak_call =
@@ -361,65 +258,39 @@ struct Invoker<StorageType, R(UnboundArgs...)> {
                          DecayedArgsTuple>::type...>::value;
 
     return InvokeHelper<is_weak_call, R>::MakeItSo(
-        std::forward<Runnable>(runnable),
-        Unwrap(std::get<indices>(std::forward<BoundArgsTuple>(bound)))...,
+        std::forward<Functor>(functor),
+        Unwrap(base::get<indices>(std::forward<BoundArgsTuple>(bound)))...,
         std::forward<UnboundArgs>(unbound_args)...);
   }
 };
 
-// Used to implement MakeArgsStorage.
-template <bool is_method, typename... BoundArgs>
-struct MakeArgsStorageImpl {
-  using Type = std::tuple<BoundArgs...>;
-};
-
-template <typename Obj, typename... BoundArgs>
-struct MakeArgsStorageImpl<true, Obj*, BoundArgs...> {
-  using Type = std::tuple<scoped_refptr<Obj>, BoundArgs...>;
-};
-
-// Constructs a tuple type to store BoundArgs into BindState.
-// This wraps the first argument into a scoped_refptr if |is_method| is true and
-// the first argument is a raw pointer.
-// Other arguments are adjusted for store and packed into a tuple.
-template <bool is_method, typename... BoundArgs>
-using MakeArgsStorage = typename MakeArgsStorageImpl<
-  is_method, typename std::decay<BoundArgs>::type...>::Type;
-
 // Used to implement MakeUnboundRunType.
 template <typename Functor, typename... BoundArgs>
 struct MakeUnboundRunTypeImpl {
-  using RunType = typename FunctorTraits<Functor>::RunType;
+  using RunType =
+      typename FunctorTraits<typename std::decay<Functor>::type>::RunType;
   using ReturnType = ExtractReturnType<RunType>;
   using Args = ExtractArgs<RunType>;
   using UnboundArgs = DropTypeListItem<sizeof...(BoundArgs), Args>;
   using Type = MakeFunctionType<ReturnType, UnboundArgs>;
 };
 
+

[dcheng, 2016/06/30 12:53:36]

Nit: I think the other top-level definitions in this file are separated by only
one newline, so let's remove this for consistency?

[tzik, 2016/07/01 04:11:00]

Done.

 // BindState<>
 //
-// This stores all the state passed into Bind() and is also where most
-// of the template resolution magic occurs.
-//
-// Runnable is the functor we are binding arguments to.
-//
-// BoundArgs contains the storage type for all the bound arguments.
-template <typename Runnable, typename... BoundArgs>
-struct BindState final : public BindStateBase {
- private:
-  using RunnableType = typename std::decay<Runnable>::type;
-
-  static constexpr bool is_method = HasIsMethodTag<RunnableType>::value;
-
- public:
-  template <typename... ForwardArgs>
-  explicit BindState(RunnableType runnable, ForwardArgs&&... bound_args)
+// This stores all the state passed into Bind().
+template <typename Functor, typename... BoundArgs>
+struct BindState final : BindStateBase {
+  template <typename... ForwardBoundArgs>
+  explicit BindState(Functor functor, ForwardBoundArgs&&... bound_args)
       : BindStateBase(&Destroy),
-        runnable_(std::move(runnable)),
-        bound_args_(std::forward<ForwardArgs>(bound_args)...) {}
+        functor_(std::move(functor)),
+        bound_args_(std::forward<ForwardBoundArgs>(bound_args)...) {
+    DCHECK(functor_);
+  }
 
-  RunnableType runnable_;
-  MakeArgsStorage<is_method, BoundArgs...> bound_args_;
+  Functor functor_;
+  std::tuple<BoundArgs...> bound_args_;
 
  private:
   ~BindState() {}
@@ -429,6 +300,50 @@ struct BindState final : public BindStateBase {
   }
 };
 
+// Used to implement MakeBindStateType.
+template <bool is_method, typename Functor, typename... BoundArgs>
+struct MakeBindStateTypeImpl;
+
+template <typename Functor, typename... BoundArgs>
+struct MakeBindStateTypeImpl<false, Functor, BoundArgs...> {
+  static_assert(!HasRefCountedTypeAsRawPtr<BoundArgs...>::value,
+                "A parameter is a refcounted type and needs scoped_refptr.");
+  using Type = BindState<typename std::decay<Functor>::type,
+                         typename std::decay<BoundArgs>::type...>;
+};
+
+template <typename Functor>
+struct MakeBindStateTypeImpl<true, Functor> {
+  using Type = BindState<typename std::decay<Functor>::type>;
+};
+
+template <typename Functor, typename Receiver, typename... BoundArgs>
+struct MakeBindStateTypeImpl<true, Functor, Receiver, BoundArgs...> {
+  static_assert(
+      !std::is_array<typename std::remove_reference<Receiver>::type>::value,
+      "First bound argument to a method cannot be an array.");
+  static_assert(!HasRefCountedTypeAsRawPtr<BoundArgs...>::value,
+                "A parameter is a refcounted type and needs scoped_refptr.");
+
+ private:
+  using DecayedReceiver = typename std::decay<Receiver>::type;
+
+ public:
+  using Type = BindState<
+      typename std::decay<Functor>::type,
+      typename std::conditional<
+          std::is_pointer<DecayedReceiver>::value,
+          scoped_refptr<typename std::remove_pointer<DecayedReceiver>::type>,
+          DecayedReceiver>::type,
+      typename std::decay<BoundArgs>::type...>;
+};
+
+template <typename Functor, typename... BoundArgs>
+using MakeBindStateType = typename MakeBindStateTypeImpl<
+    FunctorTraits<typename std::decay<Functor>::type>::is_method,
+    Functor,
+    BoundArgs...>::Type;
+
 }  // namespace internal
 
 // Returns a RunType of bound functor.