[pattern rewriter] Only desugar to call %ToName on computed properties

Convert strings to numbers if possible in the runtime call and store
in excluded property list.

BUG=v8:5549

Review-Url: https://codereview.chromium.org/2639333004
Cr-Commit-Position: refs/heads/master@{#42581}

src/objects.cc

@@ -2003,8 +2003,9 @@ Maybe<bool> JSReceiver::HasInPrototypeChain(Isolate* isolate,
 
 namespace {
 
-bool HasExcludedProperty(const ScopedVector<Handle<Name>>* excluded_properties,
-                         Handle<Object> search_element) {
+bool HasExcludedProperty(
+    const ScopedVector<Handle<Object>>* excluded_properties,
+    Handle<Object> search_element) {
   // TODO(gsathya): Change this to be a hashtable.
   for (int i = 0; i < excluded_properties->length(); i++) {
     if (search_element->SameValue(*excluded_properties->at(i))) {
@@ -2017,7 +2018,7 @@ bool HasExcludedProperty(const ScopedVector<Handle<Name>>* excluded_properties,
 
 MUST_USE_RESULT Maybe<bool> FastAssign(
     Handle<JSReceiver> target, Handle<Object> source,
-    const ScopedVector<Handle<Name>>* excluded_properties, bool use_set) {
+    const ScopedVector<Handle<Object>>* excluded_properties, bool use_set) {
   // Non-empty strings are the only non-JSReceivers that need to be handled
   // explicitly by Object.assign.
   if (!source->IsJSReceiver()) {
@@ -2113,7 +2114,7 @@ MUST_USE_RESULT Maybe<bool> FastAssign(
 // static
 Maybe<bool> JSReceiver::SetOrCopyDataProperties(
     Isolate* isolate, Handle<JSReceiver> target, Handle<Object> source,
-    const ScopedVector<Handle<Name>>* excluded_properties, bool use_set) {
+    const ScopedVector<Handle<Object>>* excluded_properties, bool use_set) {
   Maybe<bool> fast_assign =
       FastAssign(target, source, excluded_properties, use_set);
   if (fast_assign.IsNothing()) return Nothing<bool>();
@@ -2125,7 +2126,7 @@ Maybe<bool> JSReceiver::SetOrCopyDataProperties(
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
       isolate, keys,
       KeyAccumulator::GetKeys(from, KeyCollectionMode::kOwnOnly, ALL_PROPERTIES,
-                              GetKeysConversion::kConvertToString),
+                              GetKeysConversion::kKeepNumbers),
       Nothing<bool>());
 
   // 4. Repeat for each element nextKey of keys in List order,
@@ -6676,7 +6677,6 @@ bool PropertyKeyToArrayLength(Handle<Object> value, uint32_t* length) {
   return false;
 }
 
-
 bool PropertyKeyToArrayIndex(Handle<Object> index_obj, uint32_t* output) {
   return PropertyKeyToArrayLength(index_obj, output) && *output != kMaxUInt32;
 }

src/objects.h

@@ -2011,7 +2011,7 @@ class JSReceiver: public HeapObject {
   // maybe_excluded_properties list.
   MUST_USE_RESULT static Maybe<bool> SetOrCopyDataProperties(
       Isolate* isolate, Handle<JSReceiver> target, Handle<Object> source,
-      const ScopedVector<Handle<Name>>* excluded_properties = nullptr,
+      const ScopedVector<Handle<Object>>* excluded_properties = nullptr,
       bool use_set = true);
 
   // Implementation of [[HasProperty]], ECMA-262 5th edition, section 8.12.6.

src/parsing/pattern-rewriter.cc

@@ -361,12 +361,11 @@ void Parser::PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern,
     if (property->kind() == ObjectLiteralProperty::Kind::SPREAD) {
       // var { y, [x++]: a, ...c } = temp
       //     becomes
-      // var temp1 = %ToName('y');
-      // var y = temp[temp1]
-      // var temp2 = %ToName(x++);
-      // var a = temp[temp2];
+      // var y = temp.y;
+      // var temp1 = %ToName(x++);
+      // var a = temp[temp1];
       // var c;
-      // c = %CopyDataPropertiesWithExcludedProperties(temp, temp1, temp2);
+      // c = %CopyDataPropertiesWithExcludedProperties(temp, "y", temp1);
       value = factory()->NewCallRuntime(
           Runtime::kCopyDataPropertiesWithExcludedProperties,
           rest_runtime_callargs, kNoSourcePosition);
@@ -379,17 +378,23 @@ void Parser::PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern,
         RewriteParameterScopes(key);
       }
 
-      // TODO(gsathya): Skip %ToName runtime call for literals.
       if (pattern->has_rest_property()) {
-        auto args = new (zone()) ZoneList<Expression*>(1, zone());
-        args->Add(key, zone());
-        auto to_name_key = CreateTempVar(factory()->NewCallRuntime(
-            Runtime::kToName, args, kNoSourcePosition));
-        key = factory()->NewVariableProxy(to_name_key);
+        Expression* excluded_property = key;
+
+        if (property->is_computed_name()) {
+          DCHECK(!key->IsPropertyName() || !key->IsNumberLiteral());
+          auto args = new (zone()) ZoneList<Expression*>(1, zone());
+          args->Add(key, zone());
+          auto to_name_key = CreateTempVar(factory()->NewCallRuntime(
+              Runtime::kToName, args, kNoSourcePosition));
+          key = factory()->NewVariableProxy(to_name_key);
+          excluded_property = factory()->NewVariableProxy(to_name_key);
+        } else {
+          DCHECK(key->IsPropertyName() || key->IsNumberLiteral());
+        }
 
         DCHECK(rest_runtime_callargs != nullptr);
-        rest_runtime_callargs->Add(factory()->NewVariableProxy(to_name_key),
-                                   zone());
+        rest_runtime_callargs->Add(excluded_property, zone());
       }
 
       value = factory()->NewProperty(factory()->NewVariableProxy(temp), key,

src/runtime/runtime-object.cc

@@ -779,9 +779,20 @@ RUNTIME_FUNCTION(Runtime_CopyDataPropertiesWithExcludedProperties) {
     return isolate->heap()->undefined_value();
   }
 
-  ScopedVector<Handle<Name>> excluded_properties(args.length() - 1);
+  ScopedVector<Handle<Object>> excluded_properties(args.length() - 1);
   for (int i = 1; i < args.length(); i++) {
-    excluded_properties[i - 1] = args.at<Name>(i);
+    Handle<Object> property = args.at(i);
+    uint32_t property_num;
+    // We convert string to number if possible, in cases of computed
+    // properties resolving to numbers, which would've been strings
+    // instead because of our call to %ToName() in the desugaring for
+    // computed properties.
+    if (property->IsString() &&
+        String::cast(*property)->AsArrayIndex(&property_num)) {
+      property = isolate->factory()->NewNumberFromUint(property_num);
+    }
+
+    excluded_properties[i - 1] = property;
   }
 
   Handle<JSObject> target =

test/mjsunit/harmony/object-rest-basic.js

@@ -42,6 +42,11 @@ assertEquals({ a: 1 }, x);
 assertEquals(key, 2);
 assertEquals(1, y);
 
+var key = '1';
+var {[key]: y, ...x} = {1: 1, a: 1};
+assertEquals({a: 1}, x);
+assertEquals(1, y);
+
 function example({a, ...rest}, { b = rest }) {
     assertEquals(1, a);
     assertEquals({ b: 2, c: 3}, rest);
@@ -56,6 +61,13 @@ var y = {
 };
 assertEquals(y.a, 3);
 
+var {...y} = {
+  get a() {
+    return 1
+  }
+};
+assertEquals({a: 1}, y);
+
 var x = {
   get a() { throw new Error(); },
 };
@@ -120,3 +132,28 @@ assertEquals({ 0: { x: 1} }, z);
 
 var {...{x}} = { x: 1};
 assertEquals(1, x);
+
+var {4294967297: y, ...x} = {4294967297: 1, x: 1};
+assertEquals(1, y);
+assertEquals({x: 1}, x);
+
+var obj = {
+  [Symbol.toPrimitive]() {
+    return 1;
+  }
+};
+var {[obj]: y, ...x} = {1: 1, x: 1};
+assertEquals(1, y);
+assertEquals({x: 1}, x);
+
+var {[null]: y, ...x} = {null: 1, x: 1};
+assertEquals(1, y);
+assertEquals({x: 1}, x);
+
+var {[true]: y, ...x} = {true: 1, x: 1};
+assertEquals(1, y);
+assertEquals({x: 1}, x);
+
+var {[false]: y, ...x} = {false: 1, x: 1};
+assertEquals(1, y);
+assertEquals({x: 1}, x);