Get rid of ConvertFieldToDescriptor and simplify related code.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index daa5a4a5bdad825e7d73fc42531df9e23ac848b6..403a8ab340d8389d5b3303af75540bba6ae2de5c 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -1916,7 +1916,8 @@ MaybeObject* JSObject::AddFastProperty(Name* name,
                                        Object* value,
                                        PropertyAttributes attributes,
                                        StoreFromKeyed store_mode,
-                                       ValueType value_type) {
+                                       ValueType value_type,
+                                       TransitionFlag flag) {
   ASSERT(!IsJSGlobalProxy());
   ASSERT(DescriptorArray::kNotFound ==
          map()->instance_descriptors()->Search(
@@ -1926,15 +1927,13 @@ MaybeObject* JSObject::AddFastProperty(Name* name,
   // hidden strings) and is not a real identifier.
   // Normalize the object if it will have too many fast properties.
   Isolate* isolate = GetHeap()->isolate();
-  if ((!name->IsSymbol() && !IsIdentifier(isolate->unicode_cache(), name)
-       && name != isolate->heap()->hidden_string()) ||
-      (map()->unused_property_fields() == 0 &&
-       TooManyFastProperties(properties()->length(), store_mode))) {
-    Object* obj;
-    MaybeObject* maybe_obj =
+  if ((!name->IsSymbol() &&
+       !IsIdentifier(isolate->unicode_cache(), name) &&
+       name != isolate->heap()->hidden_string()) ||
+      TooManyFastProperties(store_mode)) {
+    MaybeObject* maybe_failure =
         NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
-    if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-
+    if (maybe_failure->IsFailure()) return maybe_failure;
     return AddSlowProperty(name, value, attributes);
   }
 
@@ -1961,8 +1960,6 @@ MaybeObject* JSObject::AddFastProperty(Name* name,
     if (!maybe_values->To(&values)) return maybe_values;
   }
 
-  TransitionFlag flag = INSERT_TRANSITION;
-
   Heap* heap = isolate->heap();
 
   Object* storage;
@@ -1995,7 +1992,8 @@ MaybeObject* JSObject::AddFastProperty(Name* name,
 MaybeObject* JSObject::AddConstantProperty(
     Name* name,
     Object* constant,
-    PropertyAttributes attributes) {
+    PropertyAttributes attributes,
+    TransitionFlag initial_flag) {
   // Allocate new instance descriptors with (name, constant) added
   ConstantDescriptor d(name, constant, attributes);
 
@@ -2006,7 +2004,7 @@ MaybeObject* JSObject::AddConstantProperty(
       // attributes.
        attributes != NONE)
       ? OMIT_TRANSITION
-      : INSERT_TRANSITION;
+      : initial_flag;
 
   Map* new_map;
   MaybeObject* maybe_new_map = map()->CopyAddDescriptor(&d, flag);
@@ -2066,7 +2064,8 @@ MaybeObject* JSObject::AddProperty(Name* name,
                                    JSReceiver::StoreFromKeyed store_mode,
                                    ExtensibilityCheck extensibility_check,
                                    ValueType value_type,
-                                   StoreMode mode) {
+                                   StoreMode mode,
+                                   TransitionFlag transition_flag) {
   ASSERT(!IsJSGlobalProxy());
   Map* map_of_this = map();
   Heap* heap = GetHeap();
@@ -2093,10 +2092,10 @@ MaybeObject* JSObject::AddProperty(Name* name,
       //     !value->IsTheHole() &&
       //     !value->IsConsString()) {
       if (value->IsJSFunction()) {
-        result = AddConstantProperty(name, value, attributes);
+        result = AddConstantProperty(name, value, attributes, transition_flag);
       } else {
         result = AddFastProperty(
-            name, value, attributes, store_mode, value_type);
+            name, value, attributes, store_mode, value_type, transition_flag);
       }
     } else {
       // Normalize the object to prevent very large instance descriptors.
@@ -2200,56 +2199,6 @@ MaybeObject* JSObject::ReplaceSlowProperty(Name* name,
 }
 
 
-MaybeObject* JSObject::ConvertDescriptorToField(Name* name,
-                                                Object* new_value,
-                                                PropertyAttributes attributes,
-                                                TransitionFlag flag) {
-  if (map()->unused_property_fields() == 0 &&
-      TooManyFastProperties(properties()->length(), MAY_BE_STORE_FROM_KEYED)) {
-    Object* obj;
-    MaybeObject* maybe_obj = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
-    if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-    return ReplaceSlowProperty(name, new_value, attributes);
-  }
-
-  Representation representation = IsJSContextExtensionObject()
-      ? Representation::Tagged() : new_value->OptimalRepresentation();
-  int index = map()->NextFreePropertyIndex();
-  FieldDescriptor new_field(name, index, attributes, representation);
-
-  // Make a new map for the object.
-  Map* new_map;
-  MaybeObject* maybe_new_map = map()->CopyInsertDescriptor(&new_field, flag);
-  if (!maybe_new_map->To(&new_map)) return maybe_new_map;
-
-  // Make new properties array if necessary.
-  FixedArray* new_properties = NULL;
-  int new_unused_property_fields = map()->unused_property_fields() - 1;
-  if (map()->unused_property_fields() == 0) {
-    new_unused_property_fields = kFieldsAdded - 1;
-    MaybeObject* maybe_new_properties =
-        properties()->CopySize(properties()->length() + kFieldsAdded);
-    if (!maybe_new_properties->To(&new_properties)) return maybe_new_properties;
-  }
-
-  Heap* heap = GetHeap();
-  Object* storage;
-  MaybeObject* maybe_storage =
-      new_value->AllocateNewStorageFor(heap, representation);
-  if (!maybe_storage->To(&storage)) return maybe_storage;
-
-  // Update pointers to commit changes.
-  // Object points to the new map.
-  new_map->set_unused_property_fields(new_unused_property_fields);
-  set_map(new_map);
-  if (new_properties != NULL) {
-    set_properties(new_properties);
-  }
-  FastPropertyAtPut(index, storage);
-  return new_value;
-}
-
-
 const char* Representation::Mnemonic() const {
   switch (kind_) {
     case kNone: return "v";
@@ -2396,6 +2345,7 @@ MaybeObject* JSObject::MigrateToMap(Map* new_map) {
     PropertyDetails details = new_descriptors->GetDetails(i);
     if (details.type() != FIELD) continue;
     PropertyDetails old_details = old_descriptors->GetDetails(i);
+    if (old_details.type() == CALLBACKS) continue;
     ASSERT(old_details.type() == CONSTANT ||
            old_details.type() == FIELD);
     Object* value = old_details.type() == CONSTANT
@@ -3796,8 +3746,14 @@ static MaybeObject* SetPropertyUsingTransition(LookupResult* lookup,
   PropertyDetails details = descriptors->GetDetails(descriptor);
 
   if (details.type() == CALLBACKS || attributes != details.attributes()) {
-    return lookup->holder()->ConvertDescriptorToField(
-        *name, *value, attributes);
+    // AddProperty will either normalize the object, or create a new fast copy
+    // of the map. If we get a fast copy of the map, all field representations
+    // will be tagged since the transition is omitted.
+    return lookup->holder()->AddProperty(
+        *name, *value, attributes, kNonStrictMode,
+        JSReceiver::CERTAINLY_NOT_STORE_FROM_KEYED,
+        JSReceiver::OMIT_EXTENSIBILITY_CHECK,
+        JSObject::FORCE_TAGGED, FORCE_FIELD, OMIT_TRANSITION);
   }
 
   // Keep the target CONSTANT if the same value is stored.
@@ -4100,11 +4056,33 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
       if (value->IsUninitialized()) break;
       result = SetPropertyToField(&lookup, name, value);
       break;
-    case CALLBACKS:
     case INTERCEPTOR:
-      // Override callback in clone
-      result = self->ConvertDescriptorToField(*name, *value, attributes);
+      result = self->AddProperty(
+          *name, *value, attributes, kNonStrictMode,
+          MAY_BE_STORE_FROM_KEYED, extensibility_check, value_type, mode);
       break;
+    case CALLBACKS: {
+      if (TooManyFastProperties()) {
+        MaybeObject* maybe_failure =
+            NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
+        if (maybe_failure->IsFailure()) return maybe_failure;
+      }
+
+      if (!self->HasFastProperties()) {
+        result = ReplaceSlowProperty(*name, *value, attributes);
+        break;

[rossberg, 2013/08/23 09:24:14]

Nit: I'd prefer an if-else instead of a goto.

+      }
+
+      int descriptor_index = lookup.GetDescriptorIndex();
+      MaybeObject* maybe_failure = self->GeneralizeFieldRepresentation(
+          descriptor_index, Representation::Tagged(), FORCE_FIELD);
+      if (maybe_failure->IsFailure()) return maybe_failure;
+      DescriptorArray* descriptors = self->map()->instance_descriptors();
+      int index = descriptors->GetDetails(descriptor_index).field_index();
+      self->FastPropertyAtPut(index, *value);
+      result = *value;
+      break;
+    }
     case TRANSITION:
       result = SetPropertyUsingTransition(&lookup, name, value, attributes);
       break;