Fix bug in object literals with large array indexes as strings.

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

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

diff --git a/src/ast.cc b/src/ast.cc
index 80927a8..ccfa2b4 100644 (file)
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -215,17 +215,28 @@ bool IsEqualString(void* first, void* second) {
   return (*h1)->Equals(*h2);
 }
 
-bool IsEqualSmi(void* first, void* second) {
-  ASSERT((*reinterpret_cast<Smi**>(first))->IsSmi());
-  ASSERT((*reinterpret_cast<Smi**>(second))->IsSmi());
-  Handle<Smi> h1(reinterpret_cast<Smi**>(first));
-  Handle<Smi> h2(reinterpret_cast<Smi**>(second));
-  return (*h1)->value() == (*h2)->value();
+
+bool IsEqualNumber(void* first, void* second) {
+  ASSERT((*reinterpret_cast<Object**>(first))->IsNumber());
+  ASSERT((*reinterpret_cast<Object**>(second))->IsNumber());
+
+  Handle<Object> h1(reinterpret_cast<Object**>(first));
+  Handle<Object> h2(reinterpret_cast<Object**>(second));
+  if (h1->IsSmi()) {
+    return h2->IsSmi() && *h1 == *h2;
+  }
+  if (h2->IsSmi()) return false;
+  Handle<HeapNumber> n1 = Handle<HeapNumber>::cast(h1);
+  Handle<HeapNumber> n2 = Handle<HeapNumber>::cast(h2);
+  ASSERT(isfinite(n1->value()));
+  ASSERT(isfinite(n2->value()));
+  return n1->value() == n2->value();
 }
 
+
 void ObjectLiteral::CalculateEmitStore() {
   HashMap properties(&IsEqualString);
-  HashMap elements(&IsEqualSmi);
+  HashMap elements(&IsEqualNumber);
   for (int i = this->properties()->length() - 1; i >= 0; i--) {
     ObjectLiteral::Property* property = this->properties()->at(i);
     Literal* literal = property->key();
@@ -238,23 +249,19 @@ void ObjectLiteral::CalculateEmitStore() {
     uint32_t hash;
     HashMap* table;
     void* key;
-    uint32_t index;
-    Smi* smi_key_location;
     if (handle->IsSymbol()) {
       Handle<String> name(String::cast(*handle));
-      if (name->AsArrayIndex(&index)) {
-        smi_key_location = Smi::FromInt(index);
-        key = &smi_key_location;
-        hash = index;
+      if (name->AsArrayIndex(&hash)) {
+        Handle<Object> key_handle = Factory::NewNumberFromUint(hash);
+        key = key_handle.location();
         table = &elements;
       } else {
         key = name.location();
         hash = name->Hash();
         table = &properties;
       }
-    } else if (handle->ToArrayIndex(&index)) {
+    } else if (handle->ToArrayIndex(&hash)) {
       key = handle.location();
-      hash = index;
       table = &elements;
     } else {
       ASSERT(handle->IsNumber());

diff --git a/src/hashmap.h b/src/hashmap.h
index 3b947be..2798988 100644 (file)
--- a/src/hashmap.h
+++ b/src/hashmap.h
@@ -49,7 +49,8 @@ class HashMap {
   typedef bool (*MatchFun) (void* key1, void* key2);
 
   // Dummy constructor.  This constructor doesn't set up the hash
-  // map properly so don't use it unless you have good reason.
+  // map properly so don't use it unless you have good reason (e.g.,
+  // you know that the HashMap will never be used).
   HashMap();
 
   // initial_capacity is the size of the initial hash map;

diff --git a/src/parser.cc b/src/parser.cc
index c097698..ccb3f64 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -3035,7 +3035,7 @@ Handle<Object> Parser::GetBoilerplateValue(Expression* expression) {
 
 // Defined in ast.cc
 bool IsEqualString(void* first, void* second);
-bool IsEqualSmi(void* first, void* second);
+bool IsEqualNumber(void* first, void* second);
 
 
 // Validation per 11.1.5 Object Initialiser
@@ -3043,7 +3043,7 @@ class ObjectLiteralPropertyChecker {
  public:
   ObjectLiteralPropertyChecker(Parser* parser, bool strict) :
     props(&IsEqualString),
-    elems(&IsEqualSmi),
+    elems(&IsEqualNumber),
     parser_(parser),
     strict_(strict) {
   }
@@ -3092,13 +3092,12 @@ void ObjectLiteralPropertyChecker::CheckProperty(
   uint32_t hash;
   HashMap* map;
   void* key;
-  Smi* smi_key_location;
 
   if (handle->IsSymbol()) {
     Handle<String> name(String::cast(*handle));
     if (name->AsArrayIndex(&hash)) {
-      smi_key_location = Smi::FromInt(hash);
-      key = &smi_key_location;
+      Handle<Object> key_handle = Factory::NewNumberFromUint(hash);
+      key = key_handle.location();
       map = &elems;
     } else {
       key = handle.location();

diff --git a/test/mjsunit/compiler/literals.js b/test/mjsunit/compiler/literals.js
index d846cf5..e910bb3 100644 (file)
--- a/test/mjsunit/compiler/literals.js
+++ b/test/mjsunit/compiler/literals.js
@@ -88,3 +88,6 @@ assertEquals(17, eval('[1,2,3,4]; 17'));
 assertEquals(19, eval('var a=1, b=2; [a,b,3,4]; 19'));
 assertEquals(23, eval('var a=1, b=2; c=23; [a,b,3,4]; c'));
 
+// Test that literals work for non-smi indices.
+// Ensure hash-map collision if using value as hash.
+var o = {"2345678901" : 42, "2345678901" : 30};