Fix issue with multiple properties and emit store.

We used to have a parse error for conflicting property keys. This check
was removed to match ES6 (SpiderMonkey & Chakra already made this change).
Since this check was removed we ended up with a few new cases when
generating code. For example, accessors always generated code even if
those should have been shadowed by a data property.

BUG=v8:3856
LOG=Y
R=adamk, dslomov@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#26441}

diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc
index 2b3656e03627c742f5fbd572fcd8ba4650643a80..9bf4d8e3efb6c6496507baffe7ea725d9bae1b74 100644 (file)
--- a/src/arm/full-codegen-arm.cc
+++ b/src/arm/full-codegen-arm.cc
@@ -1758,10 +1758,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         break;
 
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc
index 0bfd9cbd6b0a86e9b58b0a654afc33e0195345ad..9ffbf8862c8fbc439b4b8badeea14367fa09a3a0 100644 (file)
--- a/src/arm64/full-codegen-arm64.cc
+++ b/src/arm64/full-codegen-arm64.cc
@@ -1739,10 +1739,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/ast.cc b/src/ast.cc
index 584bf548c56ba9456a8ca9cd4d981406b9098166..057d0d50e6ffd653f3699c6eae02798a9a27ba9a 100644 (file)
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -235,6 +235,9 @@ bool ObjectLiteral::Property::emit_store() {
 
 
 void ObjectLiteral::CalculateEmitStore(Zone* zone) {
+  const auto GETTER = ObjectLiteral::Property::GETTER;
+  const auto SETTER = ObjectLiteral::Property::SETTER;
+
   ZoneAllocationPolicy allocator(zone);
 
   ZoneHashMap table(Literal::Match, ZoneHashMap::kDefaultHashMapCapacity,
@@ -242,19 +245,23 @@ void ObjectLiteral::CalculateEmitStore(Zone* zone) {
   for (int i = properties()->length() - 1; i >= 0; i--) {
     ObjectLiteral::Property* property = properties()->at(i);
     if (property->is_computed_name()) continue;
+    if (property->kind() == ObjectLiteral::Property::PROTOTYPE) continue;
     Literal* literal = property->key()->AsLiteral();
-    if (literal->value()->IsNull()) continue;
+    DCHECK(!literal->value()->IsNull());
+
+    // If there is an existing entry do not emit a store unless the previous
+    // entry was also an accessor.
     uint32_t hash = literal->Hash();
-    // If the key of a computed property value is in the table, do not emit
-    // a store for the property later.
-    if ((property->kind() == ObjectLiteral::Property::MATERIALIZED_LITERAL ||
-         property->kind() == ObjectLiteral::Property::COMPUTED) &&
-        table.Lookup(literal, hash, false, allocator) != NULL) {
-      property->set_emit_store(false);
-    } else if (property->kind() != ObjectLiteral::Property::PROTOTYPE) {
-      // Add key to the table.
-      table.Lookup(literal, hash, true, allocator);
+    ZoneHashMap::Entry* entry = table.Lookup(literal, hash, true, allocator);
+    if (entry->value != NULL) {
+      auto previous_kind =
+          static_cast<ObjectLiteral::Property*>(entry->value)->kind();
+      if (!((property->kind() == GETTER && previous_kind == SETTER) ||
+            (property->kind() == SETTER && previous_kind == GETTER))) {
+        property->set_emit_store(false);
+      }
     }
+    entry->value = property;
   }
 }
 

diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc
index 7ea1bbed792b42f836d2717684b561cac3c16960..b3c40b3e2e25d7ca7bcd4ff9dcb63508f37078cf 100644 (file)
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -1520,10 +1520,14 @@ void AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
         break;
       }
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = property->value();
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = property->value();
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = property->value();
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = property->value();
+        }
         break;
     }
   }

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index 9a070ce994035ed3ac0347bfe266a5c75a76c28b..7a499151b50790c1606161cbedf9c6cc4f31a332 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -1683,10 +1683,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc
index 51d21da98d9d9ee66006402f9b9905fb1676a720..585e36283fad29c6f730acce2e6393753716202e 100644 (file)
--- a/src/mips/full-codegen-mips.cc
+++ b/src/mips/full-codegen-mips.cc
@@ -1743,10 +1743,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc
index 025c4ae52e5f23aa2ce8b0110fa3bb3787b24a1c..eb143f8deecb4a8ca9940a65f8a157ce0df14f02 100644 (file)
--- a/src/mips64/full-codegen-mips64.cc
+++ b/src/mips64/full-codegen-mips64.cc
@@ -1740,10 +1740,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/ppc/full-codegen-ppc.cc b/src/ppc/full-codegen-ppc.cc
index 5b89437369dc7abb4ebc3ef00d2abb2f4309f9b6..6f43313026952d4da836a8b210b373f5c1fcd429 100644 (file)
--- a/src/ppc/full-codegen-ppc.cc
+++ b/src/ppc/full-codegen-ppc.cc
@@ -1718,10 +1718,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc
index ba4d1b534154844743571c59267ceb50b5d8a8ef..6815838e2b0feac3a7df6c03e9c1b3eb134015ec 100644 (file)
--- a/src/x64/full-codegen-x64.cc
+++ b/src/x64/full-codegen-x64.cc
@@ -1717,10 +1717,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/src/x87/full-codegen-x87.cc b/src/x87/full-codegen-x87.cc
index a017b12ba10c02d825fa868a268516948df091c4..84df83b1bcb82269dc21b33678fa6390c20ca608 100644 (file)
--- a/src/x87/full-codegen-x87.cc
+++ b/src/x87/full-codegen-x87.cc
@@ -1667,10 +1667,14 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ CallRuntime(Runtime::kInternalSetPrototype, 2);
         break;
       case ObjectLiteral::Property::GETTER:
-        accessor_table.lookup(key)->second->getter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->getter = value;
+        }
         break;
       case ObjectLiteral::Property::SETTER:
-        accessor_table.lookup(key)->second->setter = value;
+        if (property->emit_store()) {
+          accessor_table.lookup(key)->second->setter = value;
+        }
         break;
     }
   }

diff --git a/test/mjsunit/object-literal-multiple-fields.js b/test/mjsunit/object-literal-multiple-fields.js
new file mode 100644 (file)
index 0000000..f36d14d
--- /dev/null
+++ b/test/mjsunit/object-literal-multiple-fields.js
@@ -0,0 +1,96 @@
+// 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.
+
+(function TestConstants() {
+  var o = {
+    p: 1,
+    p: 2,
+  };
+  assertEquals(2, o.p);
+})();
+
+
+(function TestMaterialized() {
+  var o = {
+    p: [1],
+    p: [2],
+  };
+  assertEquals(2, o.p[0]);
+})();
+
+
+(function TestMaterialize2() {
+  var o = {
+    p: function() {},
+    p: 2,
+  };
+  assertEquals(2, o.p);
+})();
+
+
+
+(function TestComputed() {
+  var o = {
+    p: (function() { return 1; })(),
+    p: (function() { return 2; })(),
+  };
+  assertEquals(2, o.p);
+})();
+
+
+(function TestComputed2() {
+  var o = {
+    p: (function() { return 1; })(),
+    p: 2,
+  };
+  assertEquals(2, o.p);
+})();
+
+
+
+(function TestGetter() {
+  var o = {
+    get p() { return 1; },
+    get p() { return 2; },
+  };
+  assertEquals(2, o.p);
+})();
+
+
+(function TestGetterSetter() {
+  var o = {
+    get p() { return 1; },
+    set p(_) {},
+  };
+  assertEquals(1, o.p);
+
+  o = {
+    set p(_) {},
+    get p() { return 2; },
+  };
+  assertEquals(2, o.p);
+})();
+
+
+(function TestCombined() {
+  var o = {
+    get p() { return 1; },
+    p: 2,
+  };
+  assertEquals(2, o.p);
+
+  o = {
+    get p() { return 1; },
+    p: 2,
+    get p() { return 3; },
+  };
+  assertEquals(3, o.p);
+
+  o = {
+    get p() { return 1; },
+    p: 2,
+    set p(_) {},
+  };
+  assertEquals(undefined, o.p);
+})();