Fix a bug that prevents constants from overwriting function values in object literals.

BUG=http://code.google.com/p/v8/issues/detail?id=907

Review URL: http://codereview.chromium.org/4004006

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

src/arm/codegen-arm.cc

@@ -3596,6 +3596,12 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
     frame_->CallRuntime(Runtime::kCreateObjectLiteralShallow, 4);
   }
   frame_->EmitPush(r0);  // save the result
+
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  node->CalculateEmitStore();
+
   for (int i = 0; i < node->properties()->length(); i++) {
     // At the start of each iteration, the top of stack contains
     // the newly created object literal.
@@ -3612,11 +3618,15 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
         if (key->handle()->IsSymbol()) {
           Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
           Load(value);
-          frame_->PopToR0();
-          // Fetch the object literal.
-          frame_->SpillAllButCopyTOSToR1();
-          __ mov(r2, Operand(key->handle()));
-          frame_->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
+          if (property->emit_store()) {
+            frame_->PopToR0();
+            // Fetch the object literal.
+            frame_->SpillAllButCopyTOSToR1();
+            __ mov(r2, Operand(key->handle()));
+            frame_->CallCodeObject(ic, RelocInfo::CODE_TARGET, 0);
+          } else {
+            frame_->Drop();
+          }
           break;
         }
         // else fall through
@@ -3624,7 +3634,11 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
         frame_->Dup();
         Load(key);
         Load(value);
-        frame_->CallRuntime(Runtime::kSetProperty, 3);
+        if (property->emit_store()) {
+          frame_->CallRuntime(Runtime::kSetProperty, 3);
+        } else {
+          frame_->Drop(3);
+        }
         break;
       }
       case ObjectLiteral::Property::SETTER: {

src/arm/full-codegen-arm.cc

@@ -1169,6 +1169,11 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
   // result_saved is false the result is in r0.
   bool result_saved = false;
 
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  expr->CalculateEmitStore();
+
   for (int i = 0; i < expr->properties()->length(); i++) {
     ObjectLiteral::Property* property = expr->properties()->at(i);
     if (property->IsCompileTimeValue()) continue;
@@ -1190,8 +1195,10 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
           VisitForAccumulatorValue(value);
           __ mov(r2, Operand(key->handle()));
           __ ldr(r1, MemOperand(sp));
-          Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
-          EmitCallIC(ic, RelocInfo::CODE_TARGET);
+          if (property->emit_store()) {
+            Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+            EmitCallIC(ic, RelocInfo::CODE_TARGET);
+          }
           break;
         }
         // Fall through.
@@ -1201,7 +1208,11 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ push(r0);
         VisitForStackValue(key);
         VisitForStackValue(value);
-        __ CallRuntime(Runtime::kSetProperty, 3);
+        if (property->emit_store()) {
+          __ CallRuntime(Runtime::kSetProperty, 3);
+        } else {
+          __ Drop(3);
+        }
         break;
       case ObjectLiteral::Property::GETTER:
       case ObjectLiteral::Property::SETTER:

src/ast.cc

@@ -140,6 +140,7 @@ bool FunctionLiteral::AllowsLazyCompilation() {
 
 
 ObjectLiteral::Property::Property(Literal* key, Expression* value) {
+  emit_store_ = true;
   key_ = key;
   value_ = value;
   Object* k = *key->handle();
@@ -156,6 +157,7 @@ ObjectLiteral::Property::Property(Literal* key, Expression* value) {
 
 
 ObjectLiteral::Property::Property(bool is_getter, FunctionLiteral* value) {
+  emit_store_ = true;
   key_ = new Literal(value->name());
   value_ = value;
   kind_ = is_getter ? GETTER : SETTER;
@@ -169,6 +171,78 @@ bool ObjectLiteral::Property::IsCompileTimeValue() {
 }
 
 
+void ObjectLiteral::Property::set_emit_store(bool emit_store) {
+  emit_store_ = emit_store;
+}
+
+
+bool ObjectLiteral::Property::emit_store() {
+  return emit_store_;
+}
+
+
+bool IsEqualString(void* first, void* second) {
+  Handle<String> h1(reinterpret_cast<String**>(first));
+  Handle<String> h2(reinterpret_cast<String**>(second));
+  return (*h1)->Equals(*h2);
+}
+
+bool IsEqualSmi(void* first, void* second) {
+  Handle<Smi> h1(reinterpret_cast<Smi**>(first));
+  Handle<Smi> h2(reinterpret_cast<Smi**>(second));
+  return (*h1)->value() == (*h2)->value();
+}
+
+void ObjectLiteral::CalculateEmitStore() {
+  HashMap properties(&IsEqualString);
+  HashMap elements(&IsEqualSmi);
+  for (int i = this->properties()->length() - 1; i >= 0; i--) {
+    ObjectLiteral::Property* property = this->properties()->at(i);
+    Literal* literal = property->key();
+    Handle<Object> handle = literal->handle();
+
+    if (handle->IsNull()) {
+      continue;
+    }
+
+    uint32_t hash;
+    HashMap* table;
+    void* key;
+    uint32_t index;
+    if (handle->IsSymbol()) {
+      Handle<String> name(String::cast(*handle));
+      ASSERT(!name->AsArrayIndex(&index));
+      key = name.location();
+      hash = name->Hash();
+      table = &properties;
+    } else if (handle->ToArrayIndex(&index)) {
+      key = handle.location();
+      hash = index;
+      table = &elements;
+    } else {
+      ASSERT(handle->IsNumber());
+      double num = handle->Number();
+      char arr[100];
+      Vector<char> buffer(arr, ARRAY_SIZE(arr));
+      const char* str = DoubleToCString(num, buffer);
+      Handle<String> name = Factory::NewStringFromAscii(CStrVector(str));
+      key = name.location();
+      hash = name->Hash();
+      table = &properties;
+    }
+    // If the key of a computed property is in the table, do not emit
+    // a store for the property later.
+    if (property->kind() == ObjectLiteral::Property::COMPUTED) {
+      if (table->Lookup(literal, hash, false) != NULL) {
+        property->set_emit_store(false);
+      }
+    }
+    // Add key to the table.
+    table->Lookup(literal, hash, true);
+  }
+}
+
+
 void TargetCollector::AddTarget(BreakTarget* target) {
   // Add the label to the collector, but discard duplicates.
   int length = targets_->length();

src/ast.h

@@ -832,10 +832,14 @@ class ObjectLiteral: public MaterializedLiteral {
 
     bool IsCompileTimeValue();
 
+    void set_emit_store(bool emit_store);
+    bool emit_store();
+
    private:
     Literal* key_;
     Expression* value_;
     Kind kind_;
+    bool emit_store_;
   };
 
   ObjectLiteral(Handle<FixedArray> constant_properties,
@@ -858,6 +862,12 @@ class ObjectLiteral: public MaterializedLiteral {
 
   bool fast_elements() const { return fast_elements_; }
 
+
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  void CalculateEmitStore();
+
  private:
   Handle<FixedArray> constant_properties_;
   ZoneList<Property*>* properties_;

src/ia32/codegen-ia32.cc

@@ -5559,6 +5559,11 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
   }
   frame_->Push(&clone);
 
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  node->CalculateEmitStore();
+
   for (int i = 0; i < node->properties()->length(); i++) {
     ObjectLiteral::Property* property = node->properties()->at(i);
     switch (property->kind()) {
@@ -5573,24 +5578,32 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
           // Duplicate the object as the IC receiver.
           frame_->Dup();
           Load(property->value());
-          Result ignored =
-              frame_->CallStoreIC(Handle<String>::cast(key), false);
-          // A test eax instruction following the store IC call would
-          // indicate the presence of an inlined version of the
-          // store. Add a nop to indicate that there is no such
-          // inlined version.
-          __ nop();
+          if (property->emit_store()) {
+            Result ignored =
+                frame_->CallStoreIC(Handle<String>::cast(key), false);
+            // A test eax instruction following the store IC call would
+            // indicate the presence of an inlined version of the
+            // store. Add a nop to indicate that there is no such
+            // inlined version.
+            __ nop();
+          } else {
+            frame_->Drop(2);
+          }
           break;
         }
         // Fall through
       }
       case ObjectLiteral::Property::PROTOTYPE: {
-        // Duplicate the object as an argument to the runtime call.
-        frame_->Dup();
-        Load(property->key());
-        Load(property->value());
-        Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
-        // Ignore the result.
+          // Duplicate the object as an argument to the runtime call.
+          frame_->Dup();
+          Load(property->key());
+          Load(property->value());
+          if (property->emit_store()) {
+            // Ignore the result.
+            Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
+          } else {
+            frame_->Drop(3);
+          }
         break;
       }
       case ObjectLiteral::Property::SETTER: {

src/ia32/full-codegen-ia32.cc

@@ -1202,6 +1202,11 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
   // result_saved is false the result is in eax.
   bool result_saved = false;
 
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  expr->CalculateEmitStore();
+
   for (int i = 0; i < expr->properties()->length(); i++) {
     ObjectLiteral::Property* property = expr->properties()->at(i);
     if (property->IsCompileTimeValue()) continue;
@@ -1221,8 +1226,10 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
           VisitForAccumulatorValue(value);
           __ mov(ecx, Immediate(key->handle()));
           __ mov(edx, Operand(esp, 0));
-          Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
-          EmitCallIC(ic,  RelocInfo::CODE_TARGET);
+          if (property->emit_store()) {
+            Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+            EmitCallIC(ic, RelocInfo::CODE_TARGET);
+          }
           break;
         }
         // Fall through.
@@ -1230,7 +1237,11 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ push(Operand(esp, 0));  // Duplicate receiver.
         VisitForStackValue(key);
         VisitForStackValue(value);
-        __ CallRuntime(Runtime::kSetProperty, 3);
+        if (property->emit_store()) {
+          __ CallRuntime(Runtime::kSetProperty, 3);
+        } else {
+          __ Drop(3);
+        }
         break;
       case ObjectLiteral::Property::SETTER:
       case ObjectLiteral::Property::GETTER:

src/x64/codegen-x64.cc

@@ -4866,6 +4866,11 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
   }
   frame_->Push(&clone);
 
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  node->CalculateEmitStore();
+
   for (int i = 0; i < node->properties()->length(); i++) {
     ObjectLiteral::Property* property = node->properties()->at(i);
     switch (property->kind()) {
@@ -4880,13 +4885,17 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
           // Duplicate the object as the IC receiver.
           frame_->Dup();
           Load(property->value());
-          Result ignored =
-              frame_->CallStoreIC(Handle<String>::cast(key), false);
-          // A test rax instruction following the store IC call would
-          // indicate the presence of an inlined version of the
-          // store. Add a nop to indicate that there is no such
-          // inlined version.
-          __ nop();
+          if (property->emit_store()) {
+            Result ignored =
+                frame_->CallStoreIC(Handle<String>::cast(key), false);
+            // A test rax instruction following the store IC call would
+            // indicate the presence of an inlined version of the
+            // store. Add a nop to indicate that there is no such
+            // inlined version.
+            __ nop();
+          } else {
+            frame_->Drop(2);
+          }
           break;
         }
         // Fall through
@@ -4896,8 +4905,12 @@ void CodeGenerator::VisitObjectLiteral(ObjectLiteral* node) {
         frame_->Dup();
         Load(property->key());
         Load(property->value());
-        Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
-        // Ignore the result.
+        if (property->emit_store()) {
+          // Ignore the result.
+          Result ignored = frame_->CallRuntime(Runtime::kSetProperty, 3);
+        } else {
+          frame_->Drop(3);
+        }
         break;
       }
       case ObjectLiteral::Property::SETTER: {

src/x64/full-codegen-x64.cc

@@ -1158,6 +1158,11 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
   // result_saved is false the result is in rax.
   bool result_saved = false;
 
+  // Mark all computed expressions that are bound to a key that
+  // is shadowed by a later occurrence of the same key. For the
+  // marked expressions, no store code is emitted.
+  expr->CalculateEmitStore();
+
   for (int i = 0; i < expr->properties()->length(); i++) {
     ObjectLiteral::Property* property = expr->properties()->at(i);
     if (property->IsCompileTimeValue()) continue;
@@ -1179,8 +1184,10 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
           VisitForAccumulatorValue(value);
           __ Move(rcx, key->handle());
           __ movq(rdx, Operand(rsp, 0));
-          Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
-          EmitCallIC(ic, RelocInfo::CODE_TARGET);
+          if (property->emit_store()) {
+            Handle<Code> ic(Builtins::builtin(Builtins::StoreIC_Initialize));
+            EmitCallIC(ic, RelocInfo::CODE_TARGET);
+          }
           break;
         }
         // Fall through.
@@ -1188,7 +1195,11 @@ void FullCodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         __ push(Operand(rsp, 0));  // Duplicate receiver.
         VisitForStackValue(key);
         VisitForStackValue(value);
-        __ CallRuntime(Runtime::kSetProperty, 3);
+        if (property->emit_store()) {
+          __ CallRuntime(Runtime::kSetProperty, 3);
+        } else {
+          __ Drop(3);
+        }
         break;
       case ObjectLiteral::Property::SETTER:
       case ObjectLiteral::Property::GETTER:

test/mjsunit/object-literal-conversions.js

+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test that the various conversions between property names are correctly
+// used when overwriting initializers.
+
+var test1 = { 13: 6, "13": 7 };
+var test2 = { 13: 7, "13.0": 6 };
+var test3 = { "13": 6, 13.0000000000000000: 7 };
+var test4 = { 13.213000: 6, "13.213": 7 };
+
+assertEquals(7, test1[13]);
+assertEquals(7, test2[13]);
+assertEquals(7, test3[13]);
+assertEquals(7, test4[13.213]);
+
+var test5 = { 13: function() {}, "13": 7 };
+var test6 = { 17.31: function() {}, "17.31": 7 };
+
+assertEquals(7, test5[13]);
+assertEquals(7, test6[17.31]);
+  
\ No newline at end of file

test/mjsunit/object-literal-overwrite.js

+// Copyright 2010 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Check that constants and computed properties are overwriting each other
+// correctly, i.e., the last initializer for any name is stored in the object.
+
+
+// Tests for the full code generator (if active).
+
+var foo1 = {
+  bar: 6,
+  bar: 7
+};
+
+var foo2 = {
+  bar: function(a){},
+  bar: 7
+};
+
+var foo3 = {
+  bar: function(a){},
+  bar: function(b){},
+  bar: 7
+};
+
+var foo4 = {
+  bar: function(b){},
+  bar: 7,
+  bar: function(){return 7},
+};
+
+var foo5 = {
+  13: function(a){},
+  13: 7
+}
+
+var foo6 = {
+  14.31: function(a){},
+  14.31: 7
+}
+
+var foo7 = {
+  15: 6,
+  15: 7
+}
+
+assertEquals(7, foo1.bar);
+assertEquals(7, foo2.bar);
+assertEquals(7, foo3.bar);
+assertEquals(7, foo4.bar());
+assertEquals(7, foo5[13]);
+assertEquals(7, foo6[14.31]);
+assertEquals(7, foo7[15]);
+
+// Test for the classic code generator.
+
+function fun(x) {
+  var inner = { j: function(x) { return x; }, j: 7 }; 
+  return inner.j;
+}
+
+assertEquals(7, fun(7) );
+
+// Check that the initializers of computed properties are executed, even if
+// no store instructions are generated for the literals.
+
+var glob1 = 0;
+
+var bar1 = { x: glob1++, x: glob1++, x: glob1++, x: 7};
+
+assertEquals(3, glob1);
+
+
+var glob2 = 0;
+
+function fun2() {
+  var r = { y: glob2++, y: glob2++, y: glob2++, y: 7};
+  return r.y;
+}
+
+var y = fun2();
+assertEquals(7, y);
+assertEquals(3, glob2);
+
+var glob3 = 0;
+
+function fun3() {
+  var r = { 113: glob3++, 113: glob3++, 113: glob3++, 113: 7};
+  return r[113];
+}
+
+var y = fun3();
+assertEquals(7, y);
+assertEquals(3, glob3);
\ No newline at end of file