Reviewers: Toon Verwaest,
Description:
ES6 symbols: filter symbols form for-in loops and Object.keys
[email protected]
BUG=v8:2158
Please review this at https://codereview.chromium.org/12455002/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/handles.cc
M src/objects.cc
M src/property-details.h
M src/proxy.js
M src/runtime.cc
M src/v8natives.js
M test/mjsunit/harmony/symbols.js
Index: src/handles.cc
diff --git a/src/handles.cc b/src/handles.cc
index
6584bdbaaec3159d120e44910bf7a2d8d470b636..2958d2cc0a8cb985dcb6407a04b83b2969d1d55a
100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -551,6 +551,7 @@ void CustomArguments::IterateInstance(ObjectVisitor* v)
{
// Compute the property keys from the interceptor.
+// TODO(rossberg): support symbols in API, and filter here if needed.
v8::Handle<v8::Array> GetKeysForNamedInterceptor(Handle<JSReceiver>
receiver,
Handle<JSObject> object) {
Isolate* isolate = receiver->GetIsolate();
@@ -625,7 +626,7 @@ static bool ContainsOnlyValidKeys(Handle<FixedArray>
array) {
int len = array->length();
for (int i = 0; i < len; i++) {
Object* e = array->get(i);
- if (!(e->IsName() || e->IsNumber())) return false;
+ if (!(e->IsString() || e->IsNumber())) return false;
}
return true;
}
@@ -754,10 +755,10 @@ Handle<FixedArray>
GetEnumPropertyKeys(Handle<JSObject> object,
// to kInvalidEnumCache, this means that the map itself has never
used the
// present enum cache. The first step to using the cache is to set
the
// enum length of the map by counting the number of own descriptors
that
- // are not DONT_ENUM.
+ // are not DONT_ENUM or SYMBOLIC.
if (own_property_count == Map::kInvalidEnumCache) {
own_property_count = object->map()->NumberOfDescribedProperties(
- OWN_DESCRIPTORS, DONT_ENUM);
+ OWN_DESCRIPTORS, DONT_SHOW);
if (cache_result) object->map()->SetEnumLength(own_property_count);
}
@@ -784,7 +785,7 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject>
object,
}
isolate->counters()->enum_cache_misses()->Increment();
- int num_enum = map->NumberOfDescribedProperties(ALL_DESCRIPTORS,
DONT_ENUM);
+ int num_enum = map->NumberOfDescribedProperties(ALL_DESCRIPTORS,
DONT_SHOW);
Handle<FixedArray> storage =
isolate->factory()->NewFixedArray(num_enum);
Handle<FixedArray> indices =
isolate->factory()->NewFixedArray(num_enum);
@@ -798,9 +799,10 @@ Handle<FixedArray>
GetEnumPropertyKeys(Handle<JSObject> object,
for (int i = 0; i < descs->number_of_descriptors(); i++) {
PropertyDetails details = descs->GetDetails(i);
- if (!details.IsDontEnum()) {
+ Object* key = descs->GetKey(i);
+ if (!(details.IsDontEnum() || key->IsSymbol())) {
if (i < real_size) ++enum_size;
- storage->set(index, descs->GetKey(i));
+ storage->set(index, key);
if (!indices.is_null()) {
if (details.type() != FIELD) {
indices = Handle<FixedArray>();
@@ -860,7 +862,7 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject>
object,
isolate->factory()->NewFixedArray(next_enumeration);
storage = Handle<FixedArray>(dictionary->CopyEnumKeysTo(*storage));
- ASSERT(storage->length() ==
object->NumberOfLocalProperties(DONT_ENUM));
+ ASSERT(storage->length() ==
object->NumberOfLocalProperties(DONT_SHOW));
return storage;
}
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
8b26185b11cf5b8d258d0b8f2a6b33dd8eabeb71..3f1c33f69d30af48d667e313644b0ab99ffe6453
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4570,7 +4570,10 @@ int Map::NumberOfDescribedProperties(DescriptorFlag
which,
? descs->number_of_descriptors()
: NumberOfOwnDescriptors();
for (int i = 0; i < limit; i++) {
- if ((descs->GetDetails(i).attributes() & filter) == 0) result++;
+ if ((descs->GetDetails(i).attributes() & filter) == 0 &&
+ ((filter & SYMBOLIC) == 0 || !descs->GetKey(i)->IsSymbol())) {
+ result++;
+ }
}
return result;
}
@@ -11290,7 +11293,7 @@ int
JSObject::NumberOfLocalProperties(PropertyAttributes filter) {
if (HasFastProperties()) {
Map* map = this->map();
if (filter == NONE) return map->NumberOfOwnDescriptors();
- if (filter == DONT_ENUM) {
+ if (filter & DONT_ENUM) {
int result = map->EnumLength();
if (result != Map::kInvalidEnumCache) return result;
}
@@ -13324,7 +13327,8 @@ int Dictionary<Shape,
Key>::NumberOfElementsFilterAttributes(
int result = 0;
for (int i = 0; i < capacity; i++) {
Object* k = HashTable<Shape, Key>::KeyAt(i);
- if (HashTable<Shape, Key>::IsKey(k)) {
+ if (HashTable<Shape, Key>::IsKey(k) &&
+ ((filter & SYMBOLIC) == 0 || !k->IsSymbol())) {
PropertyDetails details = DetailsAt(i);
if (details.IsDeleted()) continue;
PropertyAttributes attr = details.attributes();
@@ -13379,7 +13383,7 @@ FixedArray*
NameDictionary::CopyEnumKeysTo(FixedArray* storage) {
// that are deleted or not enumerable.
for (int i = 0; i < capacity; i++) {
Object* k = KeyAt(i);
- if (IsKey(k)) {
+ if (IsKey(k) && !k->IsSymbol()) {
PropertyDetails details = DetailsAt(i);
if (details.IsDeleted() || details.IsDontEnum()) continue;
properties++;
Index: src/property-details.h
diff --git a/src/property-details.h b/src/property-details.h
index
510e9852a987e0c175ac9121d6da77cbd6458b43..2aa6dcfa959421d8c6e285d3b731fccb6dbc4b31
100644
--- a/src/property-details.h
+++ b/src/property-details.h
@@ -42,6 +42,8 @@ enum PropertyAttributes {
SEALED = DONT_ENUM | DONT_DELETE,
FROZEN = SEALED | READ_ONLY,
+ SYMBOLIC = 8, // Used to filter symbol names
+ DONT_SHOW = DONT_ENUM | SYMBOLIC,
ABSENT = 16 // Used in runtime to indicate a property is
absent.
// ABSENT can never be stored in or returned from a descriptor's
attributes
// bitfield. It is only used as a return value meaning the attributes of
Index: src/proxy.js
diff --git a/src/proxy.js b/src/proxy.js
index
285d33c7fa1a208607d13698a48250b35129a76c..d402449145174afc9a82695bc03b0d11a6cb1e8c
100644
--- a/src/proxy.js
+++ b/src/proxy.js
@@ -163,6 +163,7 @@ function DerivedKeysTrap() {
var enumerableNames = []
for (var i = 0, count = 0; i < names.length; ++i) {
var name = names[i]
+ if (IS_SYMBOL(name)) continue
var desc = this.getOwnPropertyDescriptor(TO_STRING_INLINE(name))
if (!IS_UNDEFINED(desc) && desc.enumerable) {
enumerableNames[count++] = names[i]
@@ -176,6 +177,7 @@ function DerivedEnumerateTrap() {
var enumerableNames = []
for (var i = 0, count = 0; i < names.length; ++i) {
var name = names[i]
+ if (IS_SYMBOL(name)) continue
var desc = this.getPropertyDescriptor(TO_STRING_INLINE(name))
if (!IS_UNDEFINED(desc) && desc.enumerable) {
enumerableNames[count++] = names[i]
@@ -189,6 +191,6 @@ function ProxyEnumerate(proxy) {
if (IS_UNDEFINED(handler.enumerate)) {
return %Apply(DerivedEnumerateTrap, handler, [], 0, 0)
} else {
- return ToNameArray(handler.enumerate(), "enumerate")
+ return ToNameArray(handler.enumerate(), "enumerate", false)
}
}
Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index
bd18302e1a3a5539ea18ffb92ee28f31294c3157..bdfea164744344bac81e45b7ed0b076c948089bf
100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4873,7 +4873,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_LocalKeys) {
Handle<FixedArray> copy = isolate->factory()->NewFixedArray(length);
for (int i = 0; i < length; i++) {
Object* entry = contents->get(i);
- if (entry->IsName()) {
+ if (entry->IsString()) {
copy->set(i, entry);
} else {
ASSERT(entry->IsNumber());
Index: src/v8natives.js
diff --git a/src/v8natives.js b/src/v8natives.js
index
259856b283b66947fd6a1b2da3c7e6e304ba401c..a0986634985f3b410434479b91fc6ae5f4d3d893
100644
--- a/src/v8natives.js
+++ b/src/v8natives.js
@@ -346,8 +346,7 @@ function ObjectKeys(obj) {
if (%IsJSProxy(obj)) {
var handler = %GetHandler(obj);
var names = CallTrap0(handler, "keys", DerivedKeysTrap);
- // TODO(rossberg): filter non-string keys.
- return ToNameArray(names, "keys");
+ return ToNameArray(names, "keys", false);
}
return %LocalKeys(obj);
}
@@ -983,7 +982,7 @@ function ObjectGetOwnPropertyDescriptor(obj, p) {
// For Harmony proxies
-function ToNameArray(obj, trap) {
+function ToNameArray(obj, trap, includeSymbols) {
if (!IS_SPEC_OBJECT(obj)) {
throw MakeTypeError("proxy_non_object_prop_names", [obj, trap]);
}
@@ -992,6 +991,7 @@ function ToNameArray(obj, trap) {
var names = { __proto__: null }; // TODO(rossberg): use sets once ready.
for (var index = 0; index < n; index++) {
var s = ToName(obj[index]);
+ if (IS_SYMBOL(s) && !includeSymbols) continue;
if (%HasLocalProperty(names, s)) {
throw MakeTypeError("proxy_repeated_prop_name", [obj, trap, s]);
}
@@ -1011,7 +1011,7 @@ function ObjectGetOwnPropertyNames(obj) {
if (%IsJSProxy(obj)) {
var handler = %GetHandler(obj);
var names = CallTrap0(handler, "getOwnPropertyNames", void 0);
- return ToNameArray(names, "getOwnPropertyNames");
+ return ToNameArray(names, "getOwnPropertyNames", true);
}
// Find all the indexed properties.
Index: test/mjsunit/harmony/symbols.js
diff --git a/test/mjsunit/harmony/symbols.js
b/test/mjsunit/harmony/symbols.js
index
b35d950981ba1f119cbc71a88a8eb21eca9f9a38..4ed44abc8f10daedf1f8d4b91f4b62d80c64004c
100644
--- a/test/mjsunit/harmony/symbols.js
+++ b/test/mjsunit/harmony/symbols.js
@@ -162,26 +162,14 @@ function TestKeyHas() {
function TestKeyEnum(obj) {
- // TODO(rossberg): symbols should not show up at all in for-in.
- var found = [];
- names: for (var name in obj) {
- for (var i in symbols) {
- if (name === symbols[i]) {
- found[i] = true;
- continue names;
- }
- }
- }
- // All even symbols should have been enumerated.
- for (var i = 0; i < symbols.length; i += 2) {
- assertTrue(i in found)
+ for (var name in obj) {
+ assertFalse(%_IsSymbol(name))
}
}
function TestKeyKeys(obj) {
- // TODO(rossberg): symbols should not be returned by Object.keys.
- assertEquals(symbols.length / 2, Object.keys(obj).length)
+ assertEquals(0, Object.keys(obj).length)
assertTrue(symbols.length <= Object.getOwnPropertyNames(obj).length)
}
--
--
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/groups/opt_out.
