ES6: Add Object.getOwnPropertySymbols

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index e763d435cc985e80a21f0fe43b4e11439c534e6b..58e9449abc3200bf8cff6942e424f8b1ce93864f 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -5741,17 +5741,47 @@ static int LocalPrototypeChainLength(JSObject* obj) {
 }
 
 
+static Handle<FixedArray> FilterPublicSymbols(
+    Isolate* isolate, Handle<FixedArray> names) {
+  int length = names->length();
+  int symbol_count = 0;
+  for (int i = 0; i < length; i++) {
+    Object* name = names->get(i);
+    if (name->IsSymbol() && !static_cast<Symbol*>(name)->is_private()) {

[rossberg, 2013/12/12 15:37:59]

Use Symbol::cast(name) here (and below).

But I had actually hoped that this whole function would be unnecessary. See
below.

[arv (Not doing code reviews), 2013/12/16 21:51:21]

Done.

+      symbol_count++;
+    }
+  }
+
+  Handle<FixedArray> symbols = isolate->factory()->NewFixedArray(symbol_count);
+
+  if (symbol_count) {
+    int dest_pos = 0;
+    for (int i = 0; i < length; i++) {
+      Object* name = names->get(i);
+      if (name->IsSymbol() && !static_cast<Symbol*>(name)->is_private()) {
+        symbols->set(dest_pos++, name);
+      }
+    }
+  }
+
+  return symbols;
+}
+
+
 // Return the names of the local named properties.
 // args[0]: object
+// args[1]: int
 RUNTIME_FUNCTION(MaybeObject*, Runtime_GetLocalPropertyNames) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 2);
   if (!args[0]->IsJSObject()) {
     return isolate->heap()->undefined_value();
   }
+
   CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
-  CONVERT_BOOLEAN_ARG_CHECKED(include_symbols, 1);
-  PropertyAttributes filter = include_symbols ? NONE : SYMBOLIC;
+  CONVERT_SMI_ARG_CHECKED(key_type, 1);
+  PropertyAttributes filter =
+      (key_type & Runtime::PROPERTY_KEY_SYMBOL) ? NONE : SYMBOLIC;
 
   // Skip the global proxy as it has no properties and always delegates to the
   // real global object.
@@ -5828,6 +5858,11 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetLocalPropertyNames) {
     }
   }
 
+  if ((key_type & Runtime::PROPERTY_KEY_SYMBOL) &&

[rossberg, 2013/12/12 15:37:59]

How about splitting PropertyAttributes SYMBOLIC filter into SYMBOLIC_PUBLIC and
SYMBOLIC_PRIVATE, so that you can avoid the filter post-processing altogether?
(Except for interceptors of course, which you have to post-process anyway, but
see below.)

You would only need to refine a couple of places in objects.cc accordingly,
namely Map::NumberOfDescribedProperties and JSObject::GetLocalPropertyNames, and
Dictionary<Shape, Key>::NumberOfElementsFilterAttributes.

[arv (Not doing code reviews), 2013/12/12 16:06:16]

I considered going down this route to handle string vs symbol but we currently
have 3 cases: strings, symbols and symbols+strings. If we also want to filter
out private symbols we need to add 2 bits to the bitmask.

I guess that is fine. I'll make the change and see what it looks like in the
end.

+      !(key_type & Runtime::PROPERTY_KEY_STRING)) {
+    names = FilterPublicSymbols(isolate, names);
+  }
+
   return *isolate->factory()->NewJSArrayWithElements(names);
 }
 
@@ -5869,14 +5904,27 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetInterceptorInfo) {
 
 // Return property names from named interceptor.
 // args[0]: object
+// args[1]: int
 RUNTIME_FUNCTION(MaybeObject*, Runtime_GetNamedInterceptorPropertyNames) {
   HandleScope scope(isolate);
   ASSERT(args.length() == 1);
   CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0);
+  CONVERT_SMI_ARG_CHECKED(key_type, 1);
 
   if (obj->HasNamedInterceptor()) {
     v8::Handle<v8::Array> result = GetKeysForNamedInterceptor(obj, obj);
-    if (!result.IsEmpty()) return *v8::Utils::OpenHandle(*result);
+    if (!result.IsEmpty()) {
+      if ((key_type & Runtime::PROPERTY_KEY_SYMBOL) &&

[rossberg, 2013/12/12 15:37:59]

I'm not sure I understand this. If somebody passed type = PROPERTY_KEY_STRING,
wouldn't you also need to filter all symbols?

I think it's better to leave this function alone and for the interceptor case,
do the filtering in v8natives' ObjectGetOwnPropertyKeys -- for interceptors,
that has to do a copy anyway, where it fits in nicely.

+          !(key_type & Runtime::PROPERTY_KEY_STRING)) {
+        Handle<FixedArray> res = isolate->factory()->NewFixedArray(result->Length());

[rossberg, 2013/12/12 15:37:59]

Nit: line length

+        for (int i = 0; i < res->length(); i++) {

[arv (Not doing code reviews), 2013/12/11 23:51:12]

This is ugly. What is the correct way to do this?

[rossberg, 2013/12/12 15:37:59]

You mean the copying? Only if you allow the Filter function to operate on
different array types (e.g. by templatising it).

+          res->set(i, *v8::Utils::OpenHandle(*result->Get(i)));
+        }
+        return *isolate->factory()->NewJSArrayWithElements(
+            FilterPublicSymbols(isolate, res));
+      }
+      return *v8::Utils::OpenHandle(*result);
+    }
   }
   return isolate->heap()->undefined_value();
 }