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

The main API change IgnoreResult() and fully currying.  See unittest for what the new API looks like.  The rest of the changes are done to support that.

Previously, IgnoreReturn could not be used with WeakPtr<> Bind()s as it was applied after the fact to the Callback object.  Now, IgnoreResult() wraps the function like Unretained().

As an incidental benefit, the new implementation gave us fully currying for free.

Also, the new implementation scales better when supporting higher arities of functions.  The new type growth is:

  (n^2 + 20n) / 2

as opposed to

  (3n^2 + 17n) / 2

where n == arity.

For n = 6 and n=10, the new implementation has 81 and 155 templates respectively.

The old implementation had 105 and 235 templates respectively.

BUG=35233,98919,98542
TEST=existing unittests

Review URL: http://codereview.chromium.org/8483003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110975 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 73 insertions, 42 deletions

diff --git a/base/bind.h.pump b/base/bind.h.pump
index eca00cd..11b4667 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,88 @@ $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 do not require
+  // a scoped_refptr because BindState<> itself takes care of AddRef() for
+  // methods. We also 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