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

src/parsing/pattern-rewriter.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/ast/ast.h"
 #include "src/messages.h"
 #include "src/objects-inl.h"
 #include "src/parsing/parameter-initializer-rewriter.h"
 #include "src/parsing/parser.h"
 
@@ skipping 343 matching lines @@
 
   block_->statements()->Add(parser_->BuildAssertIsCoercible(temp), zone());
 
   for (ObjectLiteralProperty* property : *pattern->properties()) {
     PatternContext context = SetInitializerContextIfNeeded(property->value());
     Expression* value;
 
     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];

[adamk, 2017/01/19 02:04:13]

Isn't this temp.y?

[gsathya, 2017/01/21 00:27:33]

Done.

+      // 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);
     } else {
       Expression* key = property->key();
 
       if (!key->IsLiteral()) {
         // Computed property names contain expressions which might require
         // scope rewriting.
         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 (!key->IsLiteral()) {
+          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);
+        }

[adamk, 2017/01/19 02:04:13]

Can you add a DCHECK in the else branch about the type of this literal?
Something like

DCHECK(key->IsPropertyName() || key->raw_value()->IsNumber())

which means we couldn't end up with, say, a literal null or a literal true here?
Actually I'm not sure that we don't. Maybe add some tests with computed
properties that are literals?

{ [null]: x, ...y } // should exclude the key "null"

{ [true]: x, ...y } // should exclude "true"

{ [false]: x ] } // should exclude "false"

[gsathya, 2017/01/21 00:27:32]

Done.

 
         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,
                                      kNoSourcePosition);
     }
 
     RecurseIntoSubpattern(property->value(), value);
     set_context(context);
   }
 }
@@ skipping 341 matching lines @@
 NOT_A_PATTERN(TryFinallyStatement)
 NOT_A_PATTERN(UnaryOperation)
 NOT_A_PATTERN(VariableDeclaration)
 NOT_A_PATTERN(WhileStatement)
 NOT_A_PATTERN(WithStatement)
 NOT_A_PATTERN(Yield)
 
 #undef NOT_A_PATTERN
 }  // namespace internal
 }  // namespace v8