Reviewers: Sven Panne, jamesr,
Message:
it turns out this is anyway disallowed by the styleguide without permission
of a
styleguide owner, so I added one.
jamesr@ this is a feature we would really like for a single class:
UniquePersistent, which is effectively a unique_ptr a lot of internal state
that
needs to be managed. Not being able to use move semantics causes a lot
jumping
through hoops to have containers of them. Do you know if there is a
upcoming
discussion about move semantics or is it okay to sneak it in this
exceptional
case?
Description:
give UniquePersistent full move semantics
BUG=v8:3669
LOG=Y
Please review this at https://codereview.chromium.org/978783002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+45, -25 lines):
M build/standalone.gypi
M include/v8.h
M test/cctest/test-api.cc
Index: build/standalone.gypi
diff --git a/build/standalone.gypi b/build/standalone.gypi
index
5c31fb5d272733e239bb56ee4984e640b9f77c38..66892ed7340c79083161791ce6f6e070c053417e
100644
--- a/build/standalone.gypi
+++ b/build/standalone.gypi
@@ -325,7 +325,7 @@
# Don't warn about the "struct foo f = {0};" initialization
pattern.
'-Wno-missing-field-initializers',
],
- 'cflags_cc': [ '-Wnon-virtual-dtor', '-fno-rtti', '-std=gnu++0x' ],
+ 'cflags_cc': [ '-Wnon-virtual-dtor', '-fno-rtti', '-std=gnu++11' ],
'ldflags': [ '-pthread', ],
'conditions': [
[ 'host_arch=="ppc64"', {
@@ -372,7 +372,7 @@
# Don't warn about the "struct foo f = {0};" initialization
pattern.
'-Wno-missing-field-initializers',
],
- 'cflags_cc': [ '-Wnon-virtual-dtor', '-fno-rtti', '-std=gnu++0x' ],
+ 'cflags_cc': [ '-Wnon-virtual-dtor', '-fno-rtti', '-std=gnu++11' ],
'conditions': [
[ 'visibility=="hidden"', {
'cflags': [ '-fvisibility=hidden' ],
@@ -523,8 +523,9 @@
['clang==1', {
'xcode_settings': {
'GCC_VERSION': 'com.apple.compilers.llvm.clang.1_0',
- 'CLANG_CXX_LANGUAGE_STANDARD': 'gnu++0x', # -std=gnu++0x
- },
+ 'CLANG_CXX_LANGUAGE_STANDARD': 'c++11', # -std=c++11
+ 'CLANG_CXX_LIBRARY': 'libc++', # -stdlib=libc++
+ },
}],
],
'target_conditions': [
Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index
69a9c9c8247d1919d55fba58f39d8c0a35a8bf4e..84ef170de0e4773e71c48f2df345e993b637ca83
100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -665,8 +665,8 @@ template <class T> class PersistentBase {
friend class Object;
explicit V8_INLINE PersistentBase(T* val) : val_(val) {}
- PersistentBase(PersistentBase& other); // NOLINT
- void operator=(PersistentBase&);
+ PersistentBase(PersistentBase& other) = delete; // NOLINT
+ void operator=(PersistentBase&) = delete;
V8_INLINE static T* New(Isolate* isolate, T* that);
T* val_;
@@ -814,16 +814,11 @@ template <class T, class M> class Persistent : public
PersistentBase<T> {
*/
template<class T>
class UniquePersistent : public PersistentBase<T> {
- struct RValue {
- V8_INLINE explicit RValue(UniquePersistent* obj) : object(obj) {}
- UniquePersistent* object;
- };
-
public:
/**
* A UniquePersistent with no storage cell.
*/
- V8_INLINE UniquePersistent() : PersistentBase<T>(0) { }
+ V8_INLINE UniquePersistent() : PersistentBase<T>(nullptr) {}
/**
* Construct a UniquePersistent from a Handle.
* When the Handle is non-empty, a new storage cell is created
@@ -847,34 +842,31 @@ class UniquePersistent : public PersistentBase<T> {
/**
* Move constructor.
*/
- V8_INLINE UniquePersistent(RValue rvalue)
- : PersistentBase<T>(rvalue.object->val_) {
- rvalue.object->val_ = 0;
+ V8_INLINE UniquePersistent(UniquePersistent&& other)
+ : PersistentBase<T>(other.val_) {
+ other.val_ = nullptr;
}
V8_INLINE ~UniquePersistent() { this->Reset(); }
/**
* Move via assignment.
*/
- template<class S>
- V8_INLINE UniquePersistent& operator=(UniquePersistent<S> rhs) {
+ template <class S>
+ V8_INLINE UniquePersistent& operator=(UniquePersistent<S>&& rhs) {
TYPE_CHECK(T, S);
this->Reset();
this->val_ = rhs.val_;
- rhs.val_ = 0;
+ rhs.val_ = nullptr;
return *this;
}
/**
- * Cast operator for moves.
- */
- V8_INLINE operator RValue() { return RValue(this); }
- /**
* Pass allows returning uniques from functions, etc.
*/
- UniquePersistent Pass() { return UniquePersistent(RValue(this)); }
+ // TODO(dcarney): deprecate, this is just std::move
+ UniquePersistent Pass() { return static_cast<UniquePersistent&&>(*this);
}
private:
- UniquePersistent(UniquePersistent&);
- void operator=(UniquePersistent&);
+ UniquePersistent(UniquePersistent&) = delete;
+ void operator=(UniquePersistent&) = delete;
};
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
44270382d4e76e9f03e04d629066efd59d72b7d6..606411a1b97c258b3a7f29ce817c42e92068f00b
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -28,6 +28,7 @@
#include <climits>
#include <csignal>
#include <map>
+#include <memory>
#include <string>
#include "test/cctest/test-api.h"
@@ -3243,6 +3244,32 @@ TEST(PersistentValueVector) {
}
+TEST(PersistentStdContainerCompatible) {
+ LocalContext env;
+ v8::Isolate* isolate = env->GetIsolate();
+ v8::internal::GlobalHandles* global_handles =
+ reinterpret_cast<v8::internal::Isolate*>(isolate)->global_handles();
+ int handle_count = global_handles->global_handles_count();
+ HandleScope scope(isolate);
+
+ std::vector<v8::UniquePersistent<v8::Object>> vector;
+
+ auto obj = v8::Object::New(isolate);
+ v8::UniquePersistent<v8::Object> global(isolate, obj);
+
+ vector.push_back(std::move(global));
+
+ CHECK_EQ(1, static_cast<int>(vector.size()));
+ CHECK(global.IsEmpty());
+ CHECK(obj == vector[0]);
+
+ CHECK_EQ(1 + handle_count, global_handles->global_handles_count());
+
+ vector.clear();
+ CHECK_EQ(handle_count, global_handles->global_handles_count());
+}
+
+
THREADED_TEST(GlobalHandleUpcast) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
