ES6 symbols: filter symbols form for-in loops and Object.keys

R=verwaest@chromium.org
BUG=v8:2158

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13838 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/handles.cc b/src/handles.cc
index 6584bdbaaec3159d120e44910bf7a2d8d470b636..2958d2cc0a8cb985dcb6407a04b83b2969d1d55a 100644 (file)
--- 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;
   }
 }

diff --git a/src/objects.cc b/src/objects.cc
index dc645d328e3d0d17c216766935f07eacd7f2f73a..381961be9a0dbc57a47e96a592e89aeb3ef18c68 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -3513,7 +3513,9 @@ MaybeObject* NormalizedMapCache::Get(JSObject* obj,
         ASSERT(memcmp(Map::cast(fresh)->address(),
                       Map::cast(result)->address(),
                       Map::kCodeCacheOffset) == 0);
-        int offset = Map::kCodeCacheOffset + kPointerSize;
+        STATIC_ASSERT(Map::kDependentCodeOffset ==
+                      Map::kCodeCacheOffset + kPointerSize);
+        int offset = Map::kDependentCodeOffset + kPointerSize;
         ASSERT(memcmp(Map::cast(fresh)->address() + offset,
                       Map::cast(result)->address() + offset,
                       Map::kSize - offset) == 0);
@@ -4570,7 +4572,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 +11295,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 +13329,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 +13385,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++;

diff --git a/src/property-details.h b/src/property-details.h
index 510e9852a987e0c175ac9121d6da77cbd6458b43..2aa6dcfa959421d8c6e285d3b731fccb6dbc4b31 100644 (file)
--- 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

diff --git a/src/proxy.js b/src/proxy.js
index 799792304579e85bfb7606c97ee5f241b8dc59e8..b16de016a7daf2627053a5621a436638b537e875 100644 (file)
--- a/src/proxy.js
+++ b/src/proxy.js
@@ -162,6 +162,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]
@@ -175,6 +176,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]
@@ -188,6 +190,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)
   }
 }

diff --git a/src/runtime.cc b/src/runtime.cc
index f15e9fa883a1b0852d6e2c9568203037a6582f29..5bbca4dba1bd35d504808c224e5f47dc50d6eba3 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -4890,7 +4890,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());

diff --git a/src/v8natives.js b/src/v8natives.js
index fbebbe7ca3466203708490aa8fa9c4b5e93f9bbe..f295c3a6de37d0f4fdc4d3aab65a854784d1e035 100644 (file)
--- 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.

diff --git a/test/mjsunit/harmony/symbols.js b/test/mjsunit/harmony/symbols.js
index b0aebd345ba34a298be3db07d837cf0ac4b1d4b4..13a1010879d25a521da1a17faadef3a0704c3c7d 100644 (file)
--- a/test/mjsunit/harmony/symbols.js
+++ b/test/mjsunit/harmony/symbols.js
@@ -178,26 +178,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)
 }