Fix bugs in rewriting combined with attributes and accessors

R=danno@chromium.org

Review URL: https://chromiumcodereview.appspot.com/14843023

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

diff --git a/src/objects.cc b/src/objects.cc
index ba7cdb1..ee0ff51 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2437,6 +2437,7 @@ Map* Map::FindRootMap() {
 }
 
 
+// Returns NULL if the updated map is incompatible.
 Map* Map::FindUpdatedMap(int verbatim,
                          int length,
                          DescriptorArray* descriptors) {
@@ -2452,6 +2453,17 @@ Map* Map::FindUpdatedMap(int verbatim,
     int transition = transitions->Search(name);
     if (transition == TransitionArray::kNotFound) break;
     current = transitions->GetTarget(transition);
+    PropertyDetails details = descriptors->GetDetails(i);
+    PropertyDetails target_details =
+        current->instance_descriptors()->GetDetails(i);
+    if (details.attributes() != target_details.attributes()) return NULL;
+    if (details.type() == CALLBACKS) {
+      if (target_details.type() != CALLBACKS) return NULL;
+      if (descriptors->GetValue(i) !=
+              current->instance_descriptors()->GetValue(i)) {
+        return NULL;
+      }
+    }
   }
 
   return current;
@@ -2534,6 +2546,8 @@ MaybeObject* Map::GeneralizeRepresentation(int modify_index,
 
   Map* updated = root_map->FindUpdatedMap(
       verbatim, descriptors, old_descriptors);
+  if (updated == NULL) return CopyGeneralizeAllRepresentations();
+
   // Check the state of the root map.
   DescriptorArray* updated_descriptors = updated->instance_descriptors();
 
@@ -7509,19 +7523,13 @@ bool DescriptorArray::IsMoreGeneralThan(int verbatim,
   for (int descriptor = verbatim; descriptor < valid; descriptor++) {
     PropertyDetails details = GetDetails(descriptor);
     PropertyDetails other_details = other->GetDetails(descriptor);
-    if (details.type() != other_details.type()) {
-      if (details.type() != FIELD ||
-          other_details.type() != CONSTANT_FUNCTION) {
-        return false;
-      }
-    } else if (details.type() == CONSTANT_FUNCTION) {
-      if (GetValue(descriptor) != other->GetValue(descriptor)) {
-        return false;
-      }
-    } else if (!other_details.representation().fits_into(
-                   details.representation())) {
+    if (!other_details.representation().fits_into(details.representation())) {
       return false;
     }
+    if (details.type() == CONSTANT_FUNCTION) {
+      if (other_details.type() != CONSTANT_FUNCTION) return false;
+      if (GetValue(descriptor) != other->GetValue(descriptor)) return false;
+    }
   }
 
   return true;

diff --git a/test/mjsunit/track-fields.js b/test/mjsunit/track-fields.js
index bcf37ae..74389d9 100644 (file)
--- a/test/mjsunit/track-fields.js
+++ b/test/mjsunit/track-fields.js
@@ -261,3 +261,48 @@ assertEquals(some_object20, obj20);
 assertEquals(100, o20.smi);
 assertEquals(100, o20.dbl);
 assertEquals(100, o20.dbl);
+
+function attr_mismatch_obj(v, writable) {
+  var o = {};
+  o.some_value = v;
+  Object.defineProperty(o, "second_value", {value:10, writable:writable});
+  return o;
+}
+
+function is_writable(o, p) {
+  return Object.getOwnPropertyDescriptor(o,p).writable;
+}
+
+var writable = attr_mismatch_obj(10, true);
+var non_writable1 = attr_mismatch_obj(10.5, false);
+assertTrue(is_writable(writable,"second_value"));
+assertFalse(is_writable(non_writable1,"second_value"));
+writable.some_value = 20.5;
+assertTrue(is_writable(writable,"second_value"));
+var non_writable2 = attr_mismatch_obj(10.5, false);
+assertTrue(%HaveSameMap(non_writable1, non_writable2));
+
+function test_f(v) {
+  var o = {};
+  o.vbf = v;
+  o.func = test_f;
+  return o;
+}
+
+function test_fic(o) {
+  return o.vbf;
+}
+
+var ftest1 = test_f(10);
+var ftest2 = test_f(10);
+var ftest3 = test_f(10.5);
+var ftest4 = test_f(10);
+assertFalse(%HaveSameMap(ftest1, ftest3));
+assertTrue(%HaveSameMap(ftest3, ftest4));
+ftest2.func = is_writable;
+test_fic(ftest1);
+test_fic(ftest2);
+test_fic(ftest3);
+test_fic(ftest4);
+assertTrue(%HaveSameMap(ftest1, ftest3));
+assertTrue(%HaveSameMap(ftest3, ftest4));