Callback API Change: Reimplement Bind(); support IgnoreResult, full currying, and use less types

base/bind.h.pump

Index: base/bind.h.pump
diff --git a/base/bind.h.pump b/base/bind.h.pump
index eca00cdd0773edf9606a120f0fc431f7f9624887..ea526d3cc060daabf8db69c05c348ac9d95c6e7c 100644
--- a/base/bind.h.pump
+++ b/base/bind.h.pump
@@ -18,7 +18,10 @@ $var MAX_ARITY = 6
 #include "base/bind_internal.h"
 #include "base/callback_internal.h"
 
-// See base/callback.h for how to use these functions.
+// See base/callback.h for how to use these functions. If reading the
+// implementation, before proceeding further, you should read the top
+// comment of base/bind_internal.h for a definition of common terms and
+// concepts.
 //
 // IMPLEMENTATION NOTE
 // Though Bind()'s result is meant to be stored in a Callback<> type, it
@@ -31,60 +34,87 @@ $var MAX_ARITY = 6
 // Each unique combination of (arity, function_type, num_prebound) where
 // function_type is one of {function, method, const_method} would require
 // one specialization.  We eventually have to do a similar number of
-// specializations anyways in the implementation (see the FunctionTraitsN,
+// specializations anyways in the implementation (see the Invoker<>,
 // classes).  However, it is avoidable in Bind if we return the result
 // via an indirection like we do below.
+//
+// TODO(ajwong): We might be able to avoid this now, but need to test.
+//
+// It is possible to move most of the COMPILE_ASSERT asserts into BindState<>,
+// but it feels a little nicer to have the asserts here so people do not
+// need to crack open bind_internal.h.  On the other hand, it makes Bind()
+// harder to read.
 
 namespace base {
 
-$range BOUND 0..MAX_ARITY
-$for BOUND [[
-$range BOUND_ARG 1..BOUND
+$range ARITY 0..MAX_ARITY
+$for ARITY [[
+$range ARG 1..ARITY
+
+template <typename Functor[[]]
+$if ARITY > 0 [[, ]] $for ARG , [[typename P$(ARG)]]>
+internal::BindStateHolder<
+    internal::BindState<
+        typename internal::FunctorTraits<Functor>::RunnableType,
+        typename internal::FunctorTraits<Functor>::RunType,
+        void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])> >
+Bind(Functor functor
+$if ARITY > 0 [[, ]] $for ARG , [[const P$(ARG)& p$(ARG)]]) {
+  // Typedefs for how to store and run the functor.
+  typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
+  typedef typename internal::FunctorTraits<Functor>::RunType RunType;
+
+  // Use RunnableType::RunType instead of RunType above because our
+  // checks should below for bound references need to know what the actual
+  // functor is going to interpret the argument as.
+  typedef internal::FunctionTraits<typename RunnableType::RunType>
+      BoundFunctorTraits;
+
+$if ARITY > 0 [[
+
+  // Do not allow binding a non-const reference parameter. Non-const reference
+  // parameters are disallowed by the Google style guide.  Also, binding a
+  // non-const reference parameter can make for subtle bugs because the
+  // invoked function will receive a reference to the stored copy of the
+  // argument and not the original.
+  COMPILE_ASSERT(
+      !($for ARG || [[
+is_non_const_reference<typename BoundFunctorTraits::A$(ARG)Type>::value ]]),
+      do_not_bind_functions_with_nonconst_ref);
+
+]]
 
-$if BOUND == 0 [[
 
-template <typename Sig>
-internal::InvokerStorageHolder<internal::InvokerStorage0<Sig> >
-Bind(Sig f) {
-  return internal::MakeInvokerStorageHolder(
-      new internal::InvokerStorage0<Sig>(f));
-}
+$for ARG [[
+
 
+$if ARG == 1 [[
+  // For methods, we need to be careful for parameter 1.  We skip the
+  // scoped_refptr check because the binder itself takes care of this. We also

[willchan no longer on Chromium, 2011/11/11 01:30:25]

I'm not sure what skipping the scoped_refptr check means.

[awong, 2011/11/11 02:17:57]

Reworded.

+  // disallow binding of an array as the method's target object.
+  COMPILE_ASSERT(
+      internal::HasIsMethodTag<RunnableType>::value ||
+          !internal::NeedsScopedRefptrButGetsRawPtr<P$(ARG)>::value,
+      p$(ARG)_is_refcounted_type_and_needs_scoped_refptr);
+  COMPILE_ASSERT(!internal::HasIsMethodTag<RunnableType>::value ||
+                     !is_array<P$(ARG)>::value,
+                 first_bound_argument_to_method_cannot_be_array);
 ]] $else [[
+  COMPILE_ASSERT(!internal::NeedsScopedRefptrButGetsRawPtr<P$(ARG)>::value,
+                 p$(ARG)_is_refcounted_type_and_needs_scoped_refptr);
+]]  $$ $if ARG
 
-template <typename Sig, $for BOUND_ARG , [[typename P$(BOUND_ARG)]]>
-internal::InvokerStorageHolder<internal::InvokerStorage$(BOUND)<Sig,
-$for BOUND_ARG , [[P$(BOUND_ARG)]]> >
-Bind(Sig f, $for BOUND_ARG , [[const P$(BOUND_ARG)& p$(BOUND_ARG)]]) {
-  return internal::MakeInvokerStorageHolder(
-      new internal::InvokerStorage$(BOUND)<Sig, [[]]
-$for BOUND_ARG , [[P$(BOUND_ARG)]]>(
-          f, $for BOUND_ARG , [[p$(BOUND_ARG)]]));
-}
+]]  $$ $for ARG
 
-]]
-]]  $$ for BOUND
-
-// Specializations to allow binding all the free arguments in a
-// pre-existing base::Callback<>. This does not give full support for
-// currying, but is significantly simpler and addresses the use case
-// where a base::Callback<> needs to be invoked on another context/thread.
-$for BOUND [[
-$range BOUND_ARG 1..BOUND
-$if BOUND != 0 [[
-
-template <typename Sig, $for BOUND_ARG , [[typename P$(BOUND_ARG)]]>
-base::Closure Bind(const base::Callback<Sig>& callback, [[]]
-$for BOUND_ARG , [[const P$(BOUND_ARG)& p$(BOUND_ARG)]]) {
-  return base::Bind([[]]
-&internal::BindMoreFunc$(BOUND)<Sig, $for BOUND_ARG , [[P$(BOUND_ARG)]]>, [[]]
-callback, [[]]
-$for BOUND_ARG , [[p$(BOUND_ARG)]]);
-}
 
-]]
+  return internal::MakeBindStateHolder(
+      new internal::BindState<RunnableType, RunType, [[]]
+void($for ARG , [[typename internal::CallbackParamTraits<P$(ARG)>::StorageType]])>(
+          internal::MakeRunnable(functor)[[]]
+$if ARITY > 0 [[, ]] $for ARG , [[p$(ARG)]]));
+}
 
-]]  $$ for BOUND
+]]  $$ for ARITY
 
 }  // namespace base