diff options
Diffstat (limited to 'base')
| -rw-r--r-- | base/bind_helpers.h | 7 | ||||
| -rw-r--r-- | base/bind_internal.h | 24 | ||||
| -rw-r--r-- | base/bind_unittest.cc | 34 | ||||
| -rw-r--r-- | base/callback.h | 21 |
4 files changed, 48 insertions, 38 deletions
diff --git a/base/bind_helpers.h b/base/bind_helpers.h index 660348e..a551465 100644 --- a/base/bind_helpers.h +++ b/base/bind_helpers.h @@ -563,15 +563,12 @@ static inline internal::OwnedWrapper<T> Owned(T* o) { // Both versions of Passed() prevent T from being an lvalue reference. The first // via use of enable_if, and the second takes a T* which will not bind to T&. template <typename T, - typename std::enable_if<internal::IsMoveOnlyType<T>::value && - !std::is_lvalue_reference<T>::value>::type* = + typename std::enable_if<!std::is_lvalue_reference<T>::value>::type* = nullptr> static inline internal::PassedWrapper<T> Passed(T&& scoper) { return internal::PassedWrapper<T>(std::move(scoper)); } -template <typename T, - typename std::enable_if<internal::IsMoveOnlyType<T>::value>::type* = - nullptr> +template <typename T> static inline internal::PassedWrapper<T> Passed(T* scoper) { return internal::PassedWrapper<T>(std::move(*scoper)); } diff --git a/base/bind_internal.h b/base/bind_internal.h index f296bbc..38a5756 100644 --- a/base/bind_internal.h +++ b/base/bind_internal.h @@ -339,19 +339,19 @@ template <size_t... bound_indices, typename StorageType, typename InvokeHelperType, typename R, - typename... UnboundForwardArgs> -struct Invoker<IndexSequence<bound_indices...>, StorageType, - InvokeHelperType, R(UnboundForwardArgs...)> { - static R Run(BindStateBase* base, - UnboundForwardArgs... unbound_args) { + typename... UnboundArgs> +struct Invoker<IndexSequence<bound_indices...>, + StorageType, + InvokeHelperType, + R(UnboundArgs...)> { + static R Run(BindStateBase* base, UnboundArgs&&... unbound_args) { StorageType* storage = static_cast<StorageType*>(base); // Local references to make debugger stepping easier. If in a debugger, // you really want to warp ahead and step through the // InvokeHelper<>::MakeItSo() call below. return InvokeHelperType::MakeItSo( - storage->runnable_, - Unwrap(get<bound_indices>(storage->bound_args_))..., - CallbackForward(unbound_args)...); + storage->runnable_, Unwrap(get<bound_indices>(storage->bound_args_))..., + std::forward<UnboundArgs>(unbound_args)...); } }; @@ -406,18 +406,14 @@ struct BindState<Runnable, R(Args...), BoundArgs...> final IsWeakMethod<is_method, typename std::decay<BoundArgs>::type...>; using BoundIndices = MakeIndexSequence<sizeof...(BoundArgs)>; - using UnboundForwardArgs = DropTypeListItem< - sizeof...(BoundArgs), - TypeList<typename CallbackParamTraits<Args>::ForwardType...>>; - using UnboundForwardRunType = MakeFunctionType<R, UnboundForwardArgs>; using InvokeHelperType = InvokeHelper<IsWeakCall::value, R, Runnable>; using UnboundArgs = DropTypeListItem<sizeof...(BoundArgs), TypeList<Args...>>; public: - using InvokerType = Invoker<BoundIndices, StorageType, - InvokeHelperType, UnboundForwardRunType>; using UnboundRunType = MakeFunctionType<R, UnboundArgs>; + using InvokerType = + Invoker<BoundIndices, StorageType, InvokeHelperType, UnboundRunType>; template <typename... ForwardArgs> BindState(const Runnable& runnable, ForwardArgs&&... bound_args) diff --git a/base/bind_unittest.cc b/base/bind_unittest.cc index 0595ed3..daf8c12 100644 --- a/base/bind_unittest.cc +++ b/base/bind_unittest.cc @@ -928,7 +928,7 @@ TEST_F(BindTest, ArgumentCopies) { copies = 0; assigns = 0; Bind(&VoidPolymorphic<CopyCounter>::Run).Run(counter); - EXPECT_EQ(1, copies); + EXPECT_EQ(2, copies); EXPECT_EQ(0, assigns); copies = 0; @@ -967,7 +967,20 @@ TEST_F(BindTest, ArgumentMoves) { // TODO(tzik): Support binding move-only type into a non-reference parameter // of a variant of Callback. - // TODO(tzik): Support move-only type forwarding on Callback::Run. + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<MoveCounter>::Run) + .Run(MoveCounter(&move_constructs, &move_assigns)); + EXPECT_EQ(1, move_constructs); + EXPECT_EQ(0, move_assigns); + + move_constructs = 0; + move_assigns = 0; + Bind(&VoidPolymorphic<MoveCounter>::Run) + .Run(MoveCounter(DerivedCopyMoveCounter( + nullptr, nullptr, &move_constructs, &move_assigns))); + EXPECT_EQ(2, move_constructs); + EXPECT_EQ(0, move_assigns); } // Argument constructor usage for non-reference movable-copyable @@ -1006,22 +1019,20 @@ TEST_F(BindTest, ArgumentCopiesAndMoves) { Bind(&VoidPolymorphic<CopyMoveCounter>::Run).Run(counter); EXPECT_EQ(1, copies); EXPECT_EQ(0, assigns); - EXPECT_EQ(0, move_constructs); + EXPECT_EQ(1, move_constructs); EXPECT_EQ(0, move_assigns); - // TODO(tzik): This case should be done in no copy and one move. copies = 0; assigns = 0; move_constructs = 0; move_assigns = 0; Bind(&VoidPolymorphic<CopyMoveCounter>::Run) .Run(CopyMoveCounter(&copies, &assigns, &move_constructs, &move_assigns)); - EXPECT_EQ(1, copies); + EXPECT_EQ(0, copies); EXPECT_EQ(0, assigns); - EXPECT_EQ(0, move_constructs); + EXPECT_EQ(1, move_constructs); EXPECT_EQ(0, move_assigns); - // TODO(tzik): This case should be done in one copy and one move. DerivedCopyMoveCounter derived_counter(&copies, &assigns, &move_constructs, &move_assigns); copies = 0; @@ -1030,12 +1041,11 @@ TEST_F(BindTest, ArgumentCopiesAndMoves) { move_assigns = 0; Bind(&VoidPolymorphic<CopyMoveCounter>::Run) .Run(CopyMoveCounter(derived_counter)); - EXPECT_EQ(2, copies); + EXPECT_EQ(1, copies); EXPECT_EQ(0, assigns); - EXPECT_EQ(0, move_constructs); + EXPECT_EQ(1, move_constructs); EXPECT_EQ(0, move_assigns); - // TODO(tzik): This case should be done in no copy and two move. copies = 0; assigns = 0; move_constructs = 0; @@ -1043,9 +1053,9 @@ TEST_F(BindTest, ArgumentCopiesAndMoves) { Bind(&VoidPolymorphic<CopyMoveCounter>::Run) .Run(CopyMoveCounter(DerivedCopyMoveCounter( &copies, &assigns, &move_constructs, &move_assigns))); - EXPECT_EQ(1, copies); + EXPECT_EQ(0, copies); EXPECT_EQ(0, assigns); - EXPECT_EQ(1, move_constructs); + EXPECT_EQ(2, move_constructs); EXPECT_EQ(0, move_assigns); } diff --git a/base/callback.h b/base/callback.h index 3bf0008..bff1dc4 100644 --- a/base/callback.h +++ b/base/callback.h @@ -386,18 +386,25 @@ class Callback<R(Args...)> : public internal::CallbackBase { return CallbackBase::Equals(other); } - R Run(typename internal::CallbackParamTraits<Args>::ForwardType... args) - const { + // Run() makes an extra copy compared to directly calling the bound function + // if an argument is passed-by-value and is copyable-but-not-movable: + // i.e. below copies CopyableNonMovableType twice. + // void F(CopyableNonMovableType) {} + // Bind(&F).Run(CopyableNonMovableType()); + // + // We can not fully apply Perfect Forwarding idiom to the callchain from + // Callback::Run() to the target function. Perfect Forwarding requires + // knowing how the caller will pass the arguments. However, the signature of + // InvokerType::Run() needs to be fixed in the callback constructor, so Run() + // cannot template its arguments based on how it's called. + R Run(Args... args) const { PolymorphicInvoke f = reinterpret_cast<PolymorphicInvoke>(polymorphic_invoke_); - - return f(bind_state_.get(), internal::CallbackForward(args)...); + return f(bind_state_.get(), std::forward<Args>(args)...); } private: - using PolymorphicInvoke = - R(*)(internal::BindStateBase*, - typename internal::CallbackParamTraits<Args>::ForwardType...); + using PolymorphicInvoke = R (*)(internal::BindStateBase*, Args&&...); }; } // namespace base |