Skip to content

Commit

Permalink
Remove unbound base::Bind overload
Browse files Browse the repository at this point in the history
This CL removes a base::Bind overload without bound arguments.
The difference to generic base::Bind is the type check for the bound
Runnable: the generic one ensures any argument of the Runnable
is not non-const reference, OTHT, the overload being removed checks
nothing.

Where, comments and tests asserts any *bound* argument should not be
non-const reference, so the generic one has been checked excessively.
This CL loosen the type check to match the requirement, and then
the overloaded base::Bind is no longer needed.

BUG=

Review URL: https://codereview.chromium.org/1512833002

Cr-Commit-Position: refs/heads/master@{#365988}
  • Loading branch information
tzik authored and Commit bot committed Dec 18, 2015
1 parent c1aac5a commit 7fe3a68
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 35 deletions.
22 changes: 5 additions & 17 deletions base/bind.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,6 @@

namespace base {

template <typename Functor>
base::Callback<
typename internal::BindState<
typename internal::FunctorTraits<Functor>::RunnableType,
typename internal::FunctorTraits<Functor>::RunType>::UnboundRunType>
Bind(Functor functor) {
// Typedefs for how to store and run the functor.
typedef typename internal::FunctorTraits<Functor>::RunnableType RunnableType;
typedef typename internal::FunctorTraits<Functor>::RunType RunType;

typedef internal::BindState<RunnableType, RunType> BindState;

return Callback<typename BindState::UnboundRunType>(
new BindState(internal::MakeRunnable(functor)));
}

template <typename Functor, typename... Args>
base::Callback<
typename internal::BindState<
Expand All @@ -80,12 +64,16 @@ Bind(Functor functor, const Args&... args) {
// functor is going to interpret the argument as.
typedef typename RunnableType::RunType BoundRunType;

using BoundArgs =
internal::TakeTypeListItem<sizeof...(Args),
internal::ExtractArgs<BoundRunType>>;

// 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.
static_assert(!internal::HasNonConstReferenceParam<BoundRunType>::value,
static_assert(!internal::HasNonConstReferenceItem<BoundArgs>::value,
"do not bind functions with nonconst ref");

const bool is_method = internal::HasIsMethodTag<RunnableType>::value;
Expand Down
39 changes: 39 additions & 0 deletions base/bind_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,31 @@ struct DropTypeListItemImpl<0, TypeList<>> {
template <size_t n, typename List>
using DropTypeListItem = typename DropTypeListItemImpl<n, List>::Type;

// Used for TakeTypeListItem implementation.
template <size_t n, typename List, typename... Accum>
struct TakeTypeListItemImpl;

// Do not use enable_if and SFINAE here to avoid MSVC2013 compile failure.
template <size_t n, typename T, typename... List, typename... Accum>
struct TakeTypeListItemImpl<n, TypeList<T, List...>, Accum...>
: TakeTypeListItemImpl<n - 1, TypeList<List...>, Accum..., T> {};

template <typename T, typename... List, typename... Accum>
struct TakeTypeListItemImpl<0, TypeList<T, List...>, Accum...> {
using Type = TypeList<Accum...>;
};

template <typename... Accum>
struct TakeTypeListItemImpl<0, TypeList<>, Accum...> {
using Type = TypeList<Accum...>;
};

// A type-level function that takes first |n| list item from given TypeList.
// E.g. TakeTypeListItem<3, TypeList<A, B, C, D>> is evaluated to
// TypeList<A, B, C>.
template <size_t n, typename List>
using TakeTypeListItem = typename TakeTypeListItemImpl<n, List>::Type;

// Used for ConcatTypeLists implementation.
template <typename List1, typename List2>
struct ConcatTypeListsImpl;
Expand Down Expand Up @@ -554,6 +579,20 @@ struct MakeFunctionTypeImpl<R, TypeList<Args...>> {
template <typename R, typename ArgList>
using MakeFunctionType = typename MakeFunctionTypeImpl<R, ArgList>::Type;

// Used for ExtractArgs.
template <typename Signature>
struct ExtractArgsImpl;

template <typename R, typename... Args>
struct ExtractArgsImpl<R(Args...)> {
using Type = TypeList<Args...>;
};

// A type-level function that extracts function arguments into a TypeList.
// E.g. ExtractArgs<R(A, B, C)> is evaluated to TypeList<A, B, C>.
template <typename Signature>
using ExtractArgs = typename ExtractArgsImpl<Signature>::Type;

} // namespace internal

template <typename T>
Expand Down
10 changes: 5 additions & 5 deletions base/bind_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ namespace internal {
// |Sig| is a non-const reference.
// Implementation note: This non-specialized case handles zero-arity case only.
// Non-zero-arity cases should be handled by the specialization below.
template <typename Sig>
struct HasNonConstReferenceParam : false_type {};
template <typename List>
struct HasNonConstReferenceItem : false_type {};

// Implementation note: Select true_type if the first parameter is a non-const
// reference. Otherwise, skip the first parameter and check rest of parameters
// recursively.
template <typename R, typename T, typename... Args>
struct HasNonConstReferenceParam<R(T, Args...)>
template <typename T, typename... Args>
struct HasNonConstReferenceItem<TypeList<T, Args...>>
: std::conditional<is_non_const_reference<T>::value,
true_type,
HasNonConstReferenceParam<R(Args...)>>::type {};
HasNonConstReferenceItem<TypeList<Args...>>>::type {};

// HasRefCountedTypeAsRawPtr selects true_type when any of the |Args| is a raw
// pointer to a RefCounted type.
Expand Down
30 changes: 17 additions & 13 deletions base/bind_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,10 @@ T PolymorphicIdentity(T t) {
return t;
}

template <typename T>
void VoidPolymorphic1(T t) {
}
template <typename... Ts>
struct VoidPolymorphic {
static void Run(Ts... t) {}
};

int Identity(int n) {
return n;
Expand Down Expand Up @@ -499,17 +500,20 @@ TEST_F(BindTest, ArgumentBinding) {
// - Unbound sized array.
// - Unbound array-of-arrays.
TEST_F(BindTest, UnboundArgumentTypeSupport) {
Callback<void(int)> unbound_value_cb = Bind(&VoidPolymorphic1<int>);
Callback<void(int*)> unbound_pointer_cb = Bind(&VoidPolymorphic1<int*>);
Callback<void(int&)> unbound_ref_cb = Bind(&VoidPolymorphic1<int&>);
Callback<void(int)> unbound_value_cb = Bind(&VoidPolymorphic<int>::Run);
Callback<void(int*)> unbound_pointer_cb = Bind(&VoidPolymorphic<int*>::Run);
Callback<void(int&)> unbound_ref_cb = Bind(&VoidPolymorphic<int&>::Run);
Callback<void(const int&)> unbound_const_ref_cb =
Bind(&VoidPolymorphic1<const int&>);
Bind(&VoidPolymorphic<const int&>::Run);
Callback<void(int[])> unbound_unsized_array_cb =
Bind(&VoidPolymorphic1<int[]>);
Bind(&VoidPolymorphic<int[]>::Run);
Callback<void(int[2])> unbound_sized_array_cb =
Bind(&VoidPolymorphic1<int[2]>);
Bind(&VoidPolymorphic<int[2]>::Run);
Callback<void(int[][2])> unbound_array_of_arrays_cb =
Bind(&VoidPolymorphic1<int[][2]>);
Bind(&VoidPolymorphic<int[][2]>::Run);

Callback<void(int&)> unbound_ref_with_bound_arg =
Bind(&VoidPolymorphic<int, int&>::Run, 1);
}

// Function with unbound reference parameter.
Expand Down Expand Up @@ -810,14 +814,14 @@ TEST_F(BindTest, ArgumentCopies) {
CopyCounter counter(&copies, &assigns);

Callback<void(void)> copy_cb =
Bind(&VoidPolymorphic1<CopyCounter>, counter);
Bind(&VoidPolymorphic<CopyCounter>::Run, counter);
EXPECT_GE(1, copies);
EXPECT_EQ(0, assigns);

copies = 0;
assigns = 0;
Callback<void(CopyCounter)> forward_cb =
Bind(&VoidPolymorphic1<CopyCounter>);
Bind(&VoidPolymorphic<CopyCounter>::Run);
forward_cb.Run(counter);
EXPECT_GE(1, copies);
EXPECT_EQ(0, assigns);
Expand All @@ -826,7 +830,7 @@ TEST_F(BindTest, ArgumentCopies) {
assigns = 0;
DerivedCopyCounter derived(&copies, &assigns);
Callback<void(CopyCounter)> coerce_cb =
Bind(&VoidPolymorphic1<CopyCounter>);
Bind(&VoidPolymorphic<CopyCounter>::Run);
coerce_cb.Run(CopyCounter(derived));
EXPECT_GE(2, copies);
EXPECT_EQ(0, assigns);
Expand Down

0 comments on commit 7fe3a68

Please sign in to comment.