Introduced PropertyType ACCESSOR_FIELD.

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

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

src/bootstrapper.cc

@@ -2581,6 +2581,8 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
           JSObject::AddProperty(to, key, constant, details.attributes());
           break;
         }
+        case ACCESSOR_FIELD:
+          UNREACHABLE();
         case CALLBACKS: {
           Handle<Name> key(descs->GetKey(i));
           LookupIterator it(to, key, LookupIterator::OWN_SKIP_INTERCEPTOR);
@@ -2619,6 +2621,7 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
                                  isolate());
         }
         PropertyDetails details = properties->DetailsAt(i);
+        DCHECK_EQ(DATA, details.kind());
         JSObject::AddProperty(to, key, value, details.attributes());
       }
     }

src/factory.cc

@@ -1592,7 +1592,7 @@ Handle<GlobalObject> Factory::NewGlobalObject(Handle<JSFunction> constructor) {
   for (int i = 0; i < map->NumberOfOwnDescriptors(); i++) {
     PropertyDetails details = descs->GetDetails(i);
     DCHECK(details.type() == CALLBACKS);  // Only accessors are expected.
-    PropertyDetails d = PropertyDetails(details.attributes(), CALLBACKS, i + 1);
+    PropertyDetails d(details.attributes(), CALLBACKS, i + 1);
     Handle<Name> name(descs->GetKey(i));
     Handle<Object> value(descs->GetCallbacksObject(i), isolate());
     Handle<PropertyCell> cell = NewPropertyCell(value);

src/heap-snapshot-generator.cc

@@ -1626,50 +1626,32 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, int entry) {
     DescriptorArray* descs = js_obj->map()->instance_descriptors();
     int real_size = js_obj->map()->NumberOfOwnDescriptors();
     for (int i = 0; i < real_size; i++) {
-      switch (descs->GetType(i)) {
-        case FIELD: {
-          Representation r = descs->GetDetails(i).representation();
+      PropertyDetails details = descs->GetDetails(i);
+      switch (details.location()) {
+        case IN_OBJECT: {
+          Representation r = details.representation();
           if (r.IsSmi() || r.IsDouble()) break;
-          int index = descs->GetFieldIndex(i);
 
           Name* k = descs->GetKey(i);
-          if (index < js_obj->map()->inobject_properties()) {
-            Object* value = js_obj->InObjectPropertyAt(index);
-            if (k != heap_->hidden_string()) {
-              SetPropertyReference(
-                  js_obj, entry,
-                  k, value,
-                  NULL,
-                  js_obj->GetInObjectPropertyOffset(index));
-            } else {
-              TagObject(value, "(hidden properties)");
-              SetInternalReference(
-                  js_obj, entry,
-                  "hidden_properties", value,
-                  js_obj->GetInObjectPropertyOffset(index));
-            }
+          FieldIndex field_index = FieldIndex::ForDescriptor(js_obj->map(), i);
+          Object* value = js_obj->RawFastPropertyAt(field_index);
+          int field_offset =
+              field_index.is_inobject() ? field_index.offset() : -1;
+
+          if (k != heap_->hidden_string()) {
+            SetDataOrAccessorPropertyReference(details.kind(), js_obj, entry, k,
+                                               value, NULL, field_offset);
           } else {
-            FieldIndex field_index =
-                FieldIndex::ForDescriptor(js_obj->map(), i);
-            Object* value = js_obj->RawFastPropertyAt(field_index);
-            if (k != heap_->hidden_string()) {
-              SetPropertyReference(js_obj, entry, k, value);
-            } else {
-              TagObject(value, "(hidden properties)");
-              SetInternalReference(js_obj, entry, "hidden_properties", value);
-            }
+            TagObject(value, "(hidden properties)");
+            SetInternalReference(js_obj, entry, "hidden_properties", value,
+                                 field_offset);
           }
           break;
         }
-        case CONSTANT:
-          SetPropertyReference(
-              js_obj, entry,
-              descs->GetKey(i), descs->GetConstant(i));
-          break;
-        case CALLBACKS:
-          ExtractAccessorPairProperty(
-              js_obj, entry,
-              descs->GetKey(i), descs->GetValue(i));
+        case IN_DESCRIPTOR:
+          SetDataOrAccessorPropertyReference(details.kind(), js_obj, entry,
+                                             descs->GetKey(i),
+                                             descs->GetValue(i));
           break;
       }
     }
@@ -1689,27 +1671,30 @@ void V8HeapExplorer::ExtractPropertyReferences(JSObject* js_obj, int entry) {
           SetInternalReference(js_obj, entry, "hidden_properties", value);
           continue;
         }
-        if (ExtractAccessorPairProperty(js_obj, entry, k, value)) continue;
-        SetPropertyReference(js_obj, entry, Name::cast(k), value);
+        PropertyDetails details = dictionary->DetailsAt(i);
+        SetDataOrAccessorPropertyReference(details.kind(), js_obj, entry,
+                                           Name::cast(k), value);
       }
     }
   }
 }
 
 
-bool V8HeapExplorer::ExtractAccessorPairProperty(
-    JSObject* js_obj, int entry, Object* key, Object* callback_obj) {
-  if (!callback_obj->IsAccessorPair()) return false;
+void V8HeapExplorer::ExtractAccessorPairProperty(JSObject* js_obj, int entry,
+                                                 Name* key,
+                                                 Object* callback_obj,
+                                                 int field_offset) {
+  if (!callback_obj->IsAccessorPair()) return;
   AccessorPair* accessors = AccessorPair::cast(callback_obj);
+  SetPropertyReference(js_obj, entry, key, accessors, NULL, field_offset);
   Object* getter = accessors->getter();
   if (!getter->IsOddball()) {
-    SetPropertyReference(js_obj, entry, Name::cast(key), getter, "get %s");
+    SetPropertyReference(js_obj, entry, key, getter, "get %s");
   }
   Object* setter = accessors->setter();
   if (!setter->IsOddball()) {
-    SetPropertyReference(js_obj, entry, Name::cast(key), setter, "set %s");
+    SetPropertyReference(js_obj, entry, key, setter, "set %s");
   }
-  return true;
 }
 
 
@@ -2048,6 +2033,20 @@ void V8HeapExplorer::SetWeakReference(HeapObject* parent_obj,
 }
 
 
+void V8HeapExplorer::SetDataOrAccessorPropertyReference(
+    PropertyKind kind, JSObject* parent_obj, int parent_entry,
+    Name* reference_name, Object* child_obj, const char* name_format_string,
+    int field_offset) {
+  if (kind == ACCESSOR) {
+    ExtractAccessorPairProperty(parent_obj, parent_entry, reference_name,
+                                child_obj, field_offset);
+  } else {
+    SetPropertyReference(parent_obj, parent_entry, reference_name, child_obj,
+                         name_format_string, field_offset);
+  }
+}
+
+
 void V8HeapExplorer::SetPropertyReference(HeapObject* parent_obj,
                                           int parent_entry,
                                           Name* reference_name,

src/heap-snapshot-generator.h

@@ -381,8 +381,8 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   void ExtractFixedArrayReferences(int entry, FixedArray* array);
   void ExtractClosureReferences(JSObject* js_obj, int entry);
   void ExtractPropertyReferences(JSObject* js_obj, int entry);
-  bool ExtractAccessorPairProperty(JSObject* js_obj, int entry,
-                                   Object* key, Object* callback_obj);
+  void ExtractAccessorPairProperty(JSObject* js_obj, int entry, Name* key,
+                                   Object* callback_obj, int field_offset = -1);
   void ExtractElementReferences(JSObject* js_obj, int entry);
   void ExtractInternalReferences(JSObject* js_obj, int entry);
 
@@ -430,6 +430,12 @@ class V8HeapExplorer : public HeapEntriesAllocator {
                             Object* child,
                             const char* name_format_string = NULL,
                             int field_offset = -1);
+  void SetDataOrAccessorPropertyReference(PropertyKind kind,
+                                          JSObject* parent_obj, int parent,
+                                          Name* reference_name, Object* child,
+                                          const char* name_format_string = NULL,
+                                          int field_offset = -1);
+
   void SetUserGlobalReference(Object* user_global);
   void SetRootGcRootsReference();
   void SetGcRootsReference(VisitorSynchronization::SyncTag tag);

src/lookup-inl.h

@@ -63,11 +63,10 @@ LookupIterator::State LookupIterator::LookupInHolder(Map* map,
         property_details_ = descriptors->GetDetails(number_);
       }
       has_property_ = true;
-      switch (property_details_.type()) {
-        case v8::internal::CONSTANT:
-        case v8::internal::FIELD:
+      switch (property_details_.kind()) {
+        case v8::internal::DATA:
           return DATA;
-        case v8::internal::CALLBACKS:
+        case v8::internal::ACCESSOR:
           return ACCESSOR;
       }
     case ACCESSOR:

src/objects-printer.cc

@@ -241,11 +241,16 @@ void JSObject::PrintProperties(std::ostream& os) {  // NOLINT
           os << " (field at offset " << index.property_index() << ")\n";
           break;
         }
+        case ACCESSOR_FIELD: {
+          FieldIndex index = FieldIndex::ForDescriptor(map(), i);
+          os << " (accessor at offset " << index.property_index() << ")\n";
+          break;
+        }
         case CONSTANT:
           os << Brief(descs->GetConstant(i)) << " (constant)\n";
           break;
         case CALLBACKS:
-          os << Brief(descs->GetCallbacksObject(i)) << " (callback)\n";
+          os << Brief(descs->GetCallbacksObject(i)) << " (callbacks)\n";
           break;
       }
     }
@@ -1189,19 +1194,15 @@ void TransitionArray::PrintTransitions(std::ostream& os,
       os << " (transition to Object.observe)";
     } else {
       PropertyDetails details = GetTargetDetails(key, target);
-      switch (details.type()) {
-        case FIELD: {
-          os << " (transition to field)";
-          break;
-        }
-        case CONSTANT:
-          os << " (transition to constant " << Brief(GetTargetValue(i)) << ")";
-          break;
-        case CALLBACKS:
-          os << " (transition to callback " << Brief(GetTargetValue(i)) << ")";
-          break;
+      os << " (transition to ";
+      if (details.location() == IN_DESCRIPTOR) {
+        os << "immutable ";
+      }
+      os << (details.kind() == DATA ? "data" : "accessor");
+      if (details.location() == IN_DESCRIPTOR) {
+        os << " " << Brief(GetTargetValue(i));
       }
-      os << ", attrs: " << details.attributes();
+      os << "), attrs: " << details.attributes();
     }
     os << " -> " << Brief(target) << "\n";
   }

src/objects.cc

@@ -2866,29 +2866,35 @@ MaybeHandle<Map> Map::TryUpdateInternal(Handle<Map> old_map) {
     DescriptorArray* new_descriptors = new_map->instance_descriptors();
 
     PropertyDetails new_details = new_descriptors->GetDetails(i);
-    if (old_details.attributes() != new_details.attributes() ||
-        !old_details.representation().fits_into(new_details.representation())) {
+    DCHECK_EQ(old_details.kind(), new_details.kind());
+    DCHECK_EQ(old_details.attributes(), new_details.attributes());
+    if (!old_details.representation().fits_into(new_details.representation())) {
       return MaybeHandle<Map>();
     }
-    PropertyType new_type = new_details.type();
-    PropertyType old_type = old_details.type();
     Object* new_value = new_descriptors->GetValue(i);
     Object* old_value = old_descriptors->GetValue(i);
-    switch (new_type) {
-      case FIELD:
-        if ((old_type == FIELD &&
-             !HeapType::cast(old_value)->NowIs(HeapType::cast(new_value))) ||
-            (old_type == CONSTANT &&
-             !HeapType::cast(new_value)->NowContains(old_value)) ||
-            (old_type == CALLBACKS &&
-             !HeapType::Any()->Is(HeapType::cast(new_value)))) {
-          return MaybeHandle<Map>();
+    switch (new_details.type()) {
+      case FIELD: {
+        PropertyType old_type = old_details.type();
+        if (old_type == FIELD) {
+          if (!HeapType::cast(old_value)->NowIs(HeapType::cast(new_value))) {
+            return MaybeHandle<Map>();
+          }
+        } else {
+          DCHECK(old_type == CONSTANT);
+          if (!HeapType::cast(new_value)->NowContains(old_value)) {
+            return MaybeHandle<Map>();
+          }
         }
         break;
+      }
+      case ACCESSOR_FIELD:
+        DCHECK(HeapType::Any()->Is(HeapType::cast(new_value)));
+        break;
 
       case CONSTANT:
       case CALLBACKS:
-        if (old_type != new_type || old_value != new_value) {
+        if (old_details.location() == IN_OBJECT || old_value != new_value) {
           return MaybeHandle<Map>();
         }
         break;
@@ -4364,16 +4370,15 @@ void JSObject::MigrateFastToSlow(Handle<JSObject> object,
   Handle<DescriptorArray> descs(map->instance_descriptors());
   for (int i = 0; i < real_size; i++) {
     PropertyDetails details = descs->GetDetails(i);
+    Handle<Name> key(descs->GetKey(i));
     switch (details.type()) {
       case CONSTANT: {
-        Handle<Name> key(descs->GetKey(i));
         Handle<Object> value(descs->GetConstant(i), isolate);
         PropertyDetails d(details.attributes(), FIELD, i + 1);
         dictionary = NameDictionary::Add(dictionary, key, value, d);
         break;
       }
       case FIELD: {
-        Handle<Name> key(descs->GetKey(i));
         FieldIndex index = FieldIndex::ForDescriptor(*map, i);
         Handle<Object> value;
         if (object->IsUnboxedDoubleField(index)) {
@@ -4391,8 +4396,14 @@ void JSObject::MigrateFastToSlow(Handle<JSObject> object,
         dictionary = NameDictionary::Add(dictionary, key, value, d);
         break;
       }
+      case ACCESSOR_FIELD: {
+        FieldIndex index = FieldIndex::ForDescriptor(*map, i);
+        Handle<Object> value(object->RawFastPropertyAt(index), isolate);
+        PropertyDetails d(details.attributes(), CALLBACKS, i + 1);
+        dictionary = NameDictionary::Add(dictionary, key, value, d);
+        break;
+      }
       case CALLBACKS: {
-        Handle<Name> key(descs->GetKey(i));
         Handle<Object> value(descs->GetCallbacksObject(i), isolate);
         PropertyDetails d(details.attributes(), CALLBACKS, i + 1);
         dictionary = NameDictionary::Add(dictionary, key, value, d);
@@ -7069,6 +7080,7 @@ bool DescriptorArray::CanHoldValue(int descriptor, Object* value) {
              value->FitsRepresentation(details.representation()));
       return GetConstant(descriptor) == value;
 
+    case ACCESSOR_FIELD:
     case CALLBACKS:
       return false;
   }
@@ -7194,7 +7206,7 @@ Handle<Map> Map::TransitionToAccessorProperty(Handle<Map> map,
     int descriptor = transition->LastAdded();
     DCHECK(descriptors->GetKey(descriptor)->Equals(*name));
 
-    DCHECK_EQ(CALLBACKS, descriptors->GetDetails(descriptor).type());
+    DCHECK_EQ(ACCESSOR, descriptors->GetDetails(descriptor).kind());
     DCHECK_EQ(attributes, descriptors->GetDetails(descriptor).attributes());
 
     Handle<Object> maybe_pair(descriptors->GetValue(descriptor), isolate);

src/property-details.h

@@ -57,6 +57,7 @@ enum PropertyLocation { IN_OBJECT = 0, IN_DESCRIPTOR = 1 };
 enum PropertyType {
   FIELD = (IN_OBJECT << 1) | DATA,
   CONSTANT = (IN_DESCRIPTOR << 1) | DATA,
+  ACCESSOR_FIELD = (IN_OBJECT << 1) | ACCESSOR,
   CALLBACKS = (IN_DESCRIPTOR << 1) | ACCESSOR
 };
 

src/property.cc

@@ -51,18 +51,11 @@ struct FastPropertyDetails {
 // Outputs PropertyDetails as a dictionary details.
 std::ostream& operator<<(std::ostream& os, const PropertyDetails& details) {
   os << "(";
-  switch (details.type()) {
-    case FIELD:
-      os << "normal: ";
-      break;
-    case CALLBACKS:
-      os << "callbacks: ";
-      break;
-    case CONSTANT:
-      UNREACHABLE();
-      break;
+  if (details.location() == IN_DESCRIPTOR) {
+    os << "immutable ";
   }
-  return os << " dictionary_index: " << details.dictionary_index()
+  os << (details.kind() == DATA ? "data" : "accessor");
+  return os << ", dictionary_index: " << details.dictionary_index()
             << ", attrs: " << details.attributes() << ")";
 }
 
@@ -72,20 +65,16 @@ std::ostream& operator<<(std::ostream& os,
                          const FastPropertyDetails& details_fast) {
   const PropertyDetails& details = details_fast.details;
   os << "(";
-  switch (details.type()) {
-    case CONSTANT:
-      os << "constant: p: " << details.pointer();
-      break;
-    case FIELD:
-      os << "field: " << details.representation().Mnemonic()
-         << ", field_index: " << details.field_index()
-         << ", p: " << details.pointer();
-      break;
-    case CALLBACKS:
-      os << "callbacks: p: " << details.pointer();
-      break;
+  if (details.location() == IN_DESCRIPTOR) {
+    os << "immutable ";
   }
-  return os << ", attrs: " << details.attributes() << ")";
+  os << (details.kind() == DATA ? "data" : "accessor");
+  if (details.location() == IN_OBJECT) {
+    os << ": " << details.representation().Mnemonic()
+       << ", field_index: " << details.field_index();
+  }
+  return os << ", p: " << details.pointer()
+            << ", attrs: " << details.attributes() << ")";
 }
 
 

test/cctest/test-heap-profiler.cc

@@ -1916,6 +1916,47 @@ TEST(FastCaseAccessors) {
 }
 
 
+TEST(FastCaseRedefinedAccessors) {
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+
+  CompileRun(
+      "var obj1 = {};\n"
+      "Object.defineProperty(obj1, 'prop', { "
+      "  get: function() { return 42; },\n"
+      "  set: function(value) { return this.prop_ = value; },\n"
+      "  configurable: true,\n"
+      "  enumerable: true,\n"
+      "});\n"
+      "Object.defineProperty(obj1, 'prop', { "
+      "  get: function() { return 153; },\n"
+      "  set: function(value) { return this.prop_ = value; },\n"
+      "  configurable: true,\n"
+      "  enumerable: true,\n"
+      "});\n");
+  v8::Local<v8::Object> js_global =
+      env->Global()->GetPrototype().As<v8::Object>();
+  i::Handle<i::JSObject> js_obj1 =
+      v8::Utils::OpenHandle(*js_global->Get(v8_str("obj1")).As<v8::Object>());
+  USE(js_obj1);
+
+  const v8::HeapSnapshot* snapshot =
+      heap_profiler->TakeHeapSnapshot(v8_str("fastCaseAccessors"));
+  CHECK(ValidateSnapshot(snapshot));
+  const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
+  CHECK_NE(NULL, global);
+  const v8::HeapGraphNode* obj1 =
+      GetProperty(global, v8::HeapGraphEdge::kProperty, "obj1");
+  CHECK_NE(NULL, obj1);
+  const v8::HeapGraphNode* func;
+  func = GetProperty(obj1, v8::HeapGraphEdge::kProperty, "get prop");
+  CHECK_NE(NULL, func);
+  func = GetProperty(obj1, v8::HeapGraphEdge::kProperty, "set prop");
+  CHECK_NE(NULL, func);
+}
+
+
 TEST(SlowCaseAccessors) {
   LocalContext env;
   v8::HandleScope scope(env->GetIsolate());

test/cctest/test-transitions.cc

@@ -30,6 +30,24 @@ static void ConnectTransition(Handle<Map> parent,
 }
 
 
+static void CheckPropertyDetailsFieldsConsistency(PropertyType type,
+                                                  PropertyKind kind,
+                                                  PropertyLocation location) {
+  int type_value = PropertyDetails::TypeField::encode(type);
+  int kind_location_value = PropertyDetails::KindField::encode(kind) |
+                            PropertyDetails::LocationField::encode(location);
+  CHECK_EQ(type_value, kind_location_value);
+}
+
+
+TEST(PropertyDetailsFieldsConsistency) {
+  CheckPropertyDetailsFieldsConsistency(FIELD, DATA, IN_OBJECT);
+  CheckPropertyDetailsFieldsConsistency(CONSTANT, DATA, IN_DESCRIPTOR);
+  CheckPropertyDetailsFieldsConsistency(ACCESSOR_FIELD, ACCESSOR, IN_OBJECT);
+  CheckPropertyDetailsFieldsConsistency(CALLBACKS, ACCESSOR, IN_DESCRIPTOR);
+}
+
+
 TEST(TransitionArray_SimpleFieldTransitions) {
   CcTest::InitializeVM();
   v8::HandleScope scope(CcTest::isolate());