summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authordanakj <danakj@chromium.org>2015-12-07 16:44:41 -0800
committerCommit bot <commit-bot@chromium.org>2015-12-08 00:46:50 +0000
commit314d1f44144e9d2a12d41445c9e5a1287e14373c (patch)
treec4ff7b8d1eaa817112400516ede35bda691c5fcd
parentf981cb2a199f58143449eb64946b5d5e4d974187 (diff)
downloadchromium_src-314d1f44144e9d2a12d41445c9e5a1287e14373c.zip
chromium_src-314d1f44144e9d2a12d41445c9e5a1287e14373c.tar.gz
chromium_src-314d1f44144e9d2a12d41445c9e5a1287e14373c.tar.bz2
base: Stop using Pass() on move-only types in Bind and Callback.
This replaces calls to .Pass() with calls to std::move() instead, which will help allow us to remove the .Pass() method off of our move-only types. It does not dis/allow any new behaviour with Bind and Callback. Specifically: 1. PassedWrapper is created from base::Passed. Previously PassedWrapper called .Pass() which required a whitelisted type to compile. Now we get a much nicer error by using enable_if on base::Passed to check that the type is whitelisted: ../../base/callback_unittest.cc:221:66: error: no matching function for call to 'Passed' Callback<void(void)> callback = base::Bind(&TakeUniqueByValue, base::Passed(&i)); ^~~~~~~~~~~~ This prevents use such as base::Passed(&a_std_unique_ptr). 2. The CallbackParamTraits specialize on the whitelist, and for non- whitelisted types it will store a T and pass a const T&. The receiving method will take a T which will try to copy and fail for non-whitelisted move-only types. Secondly, the constructor of BindState will fail as Bind takes a const T& for each bound argument currently, and the storage of T will try to copy into the BindState. This prevents use such as base::Bind(&F, std::unique_ptr<T>()), which will also apply to scoped_ptr. R=Nico, dcheng BUG=557422 Review URL: https://codereview.chromium.org/1496403002 Cr-Commit-Position: refs/heads/master@{#363635}
-rw-r--r--base/bind_helpers.h47
-rw-r--r--base/callback_internal.h6
2 files changed, 33 insertions, 20 deletions
diff --git a/base/bind_helpers.h b/base/bind_helpers.h
index 24063ad..434520a 100644
--- a/base/bind_helpers.h
+++ b/base/bind_helpers.h
@@ -111,7 +111,7 @@
// scoped_ptr<Foo> f(new Foo());
//
// // |cb| is given ownership of Foo(). |f| is now NULL.
-// // You can use f.Pass() in place of &f, but it's more verbose.
+// // You can use std::move(f) in place of &f, but it's more verbose.
// Closure cb = Bind(&TakesOwnership, Passed(&f));
//
// // Run was never called so |cb| still owns Foo() and deletes
@@ -143,6 +143,9 @@
#ifndef BASE_BIND_HELPERS_H_
#define BASE_BIND_HELPERS_H_
+#include <type_traits>
+#include <utility>
+
#include "base/basictypes.h"
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
@@ -359,22 +362,24 @@ class OwnedWrapper {
// created when we are explicitly trying to do a destructive move.
//
// Two notes:
-// 1) PassedWrapper supports any type that has a "Pass()" function.
-// This is intentional. The whitelisting of which specific types we
-// support is maintained by CallbackParamTraits<>.
+// 1) PassedWrapper supports any type that has a move constructor, however
+// the type will need to be specifically whitelisted in order for it to be
+// bound to a Callback. We guard this explicitly at the call of Passed()
+// to make for clear errors. Things not given to Passed() will be forwarded
+// and stored by value which will not work for general move-only types.
// 2) is_valid_ is distinct from NULL because it is valid to bind a "NULL"
// scoper to a Callback and allow the Callback to execute once.
template <typename T>
class PassedWrapper {
public:
- explicit PassedWrapper(T scoper) : is_valid_(true), scoper_(scoper.Pass()) {}
+ explicit PassedWrapper(T&& scoper)
+ : is_valid_(true), scoper_(std::move(scoper)) {}
PassedWrapper(const PassedWrapper& other)
- : is_valid_(other.is_valid_), scoper_(other.scoper_.Pass()) {
- }
+ : is_valid_(other.is_valid_), scoper_(std::move(other.scoper_)) {}
T Pass() const {
CHECK(is_valid_);
is_valid_ = false;
- return scoper_.Pass();
+ return std::move(scoper_);
}
private:
@@ -566,17 +571,25 @@ static inline internal::OwnedWrapper<T> Owned(T* o) {
return internal::OwnedWrapper<T>(o);
}
-// We offer 2 syntaxes for calling Passed(). The first takes a temporary and
-// is best suited for use with the return value of a function. The second
-// takes a pointer to the scoper and is just syntactic sugar to avoid having
-// to write Passed(scoper.Pass()).
-template <typename T>
-static inline internal::PassedWrapper<T> Passed(T scoper) {
- return internal::PassedWrapper<T>(scoper.Pass());
+// We offer 2 syntaxes for calling Passed(). The first takes an rvalue and
+// is best suited for use with the return value of a function or other temporary
+// rvalues. The second takes a pointer to the scoper and is just syntactic sugar
+// to avoid having to write Passed(std::move(scoper)).
+//
+// 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* =
+ nullptr>
+static inline internal::PassedWrapper<T> Passed(T&& scoper) {
+ return internal::PassedWrapper<T>(std::move(scoper));
}
-template <typename T>
+template <typename T,
+ typename std::enable_if<internal::IsMoveOnlyType<T>::value>::type* =
+ nullptr>
static inline internal::PassedWrapper<T> Passed(T* scoper) {
- return internal::PassedWrapper<T>(scoper->Pass());
+ return internal::PassedWrapper<T>(std::move(*scoper));
}
template <typename T>
diff --git a/base/callback_internal.h b/base/callback_internal.h
index 8f0c2b3..aa8542c 100644
--- a/base/callback_internal.h
+++ b/base/callback_internal.h
@@ -93,7 +93,7 @@ class BASE_EXPORT CallbackBase {
};
// A helper template to determine if given type is non-const move-only-type,
-// i.e. if a value of the given type should be passed via .Pass() in a
+// i.e. if a value of the given type should be passed via std::move() in a
// destructive way.
template <typename T> struct IsMoveOnlyType {
template <typename U>
@@ -197,7 +197,7 @@ struct CallbackParamTraitsForMoveOnlyType {
// default template compiles out to be a no-op.
//
// In C++11, std::forward would replace all uses of this function. However, it
-// is impossible to implement a general std::forward with C++11 due to a lack
+// is impossible to implement a general std::forward without C++11 due to a lack
// of rvalue references.
//
// In addition to Callback/Bind, this is used by PostTaskAndReplyWithResult to
@@ -213,7 +213,7 @@ typename std::enable_if<!IsMoveOnlyType<T>::value, T>::type& CallbackForward(
template <typename T>
typename std::enable_if<IsMoveOnlyType<T>::value, T>::type CallbackForward(
T& t) {
- return t.Pass();
+ return std::move(t);
}
} // namespace internal