[runtime] Remove kIsEnumerable and move methods relying on it to BaseNameDictionary

Bug: 
Change-Id: Iab8fc855808b22a2786476ddc4568f3f474c73d8
Reviewed-on: https://chromium-review.googlesource.com/543079
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46125}

src/api-natives.cc

@@ -570,7 +570,7 @@ MaybeHandle<JSObject> ApiNatives::InstantiateRemoteObject(
 void ApiNatives::AddDataProperty(Isolate* isolate, Handle<TemplateInfo> info,
                                  Handle<Name> name, Handle<Object> value,
                                  PropertyAttributes attributes) {
-  PropertyDetails details(kData, attributes, 0, PropertyCellType::kNoCell);
+  PropertyDetails details(kData, attributes, PropertyCellType::kNoCell);
   auto details_handle = handle(details.AsSmi(), isolate);
   Handle<Object> data[] = {name, details_handle, value};
   AddPropertyToPropertyList(isolate, info, arraysize(data), data);
@@ -582,7 +582,7 @@ void ApiNatives::AddDataProperty(Isolate* isolate, Handle<TemplateInfo> info,
                                  PropertyAttributes attributes) {
   auto value = handle(Smi::FromInt(intrinsic), isolate);
   auto intrinsic_marker = isolate->factory()->true_value();
-  PropertyDetails details(kData, attributes, 0, PropertyCellType::kNoCell);
+  PropertyDetails details(kData, attributes, PropertyCellType::kNoCell);
   auto details_handle = handle(details.AsSmi(), isolate);
   Handle<Object> data[] = {name, intrinsic_marker, details_handle, value};
   AddPropertyToPropertyList(isolate, info, arraysize(data), data);
@@ -595,7 +595,7 @@ void ApiNatives::AddAccessorProperty(Isolate* isolate,
                                      Handle<FunctionTemplateInfo> getter,
                                      Handle<FunctionTemplateInfo> setter,
                                      PropertyAttributes attributes) {
-  PropertyDetails details(kAccessor, attributes, 0, PropertyCellType::kNoCell);
+  PropertyDetails details(kAccessor, attributes, PropertyCellType::kNoCell);
   auto details_handle = handle(details.AsSmi(), isolate);
   Handle<Object> data[] = {name, details_handle, getter, setter};
   AddPropertyToPropertyList(isolate, info, arraysize(data), data);

src/bootstrapper.cc

@@ -5004,7 +5004,7 @@ void Genesis::TransferNamedProperties(Handle<JSObject> from,
           DCHECK(!to->HasFastProperties());
           // Add to dictionary.
           Handle<Object> value(descs->GetValue(i), isolate());
-          PropertyDetails d(kAccessor, details.attributes(), i + 1,
+          PropertyDetails d(kAccessor, details.attributes(),
                             PropertyCellType::kMutable);
           JSObject::SetNormalizedProperty(to, key, value, d);
         }

src/code-stub-assembler.cc

@@ -5147,11 +5147,11 @@ void CodeStubAssembler::InsertEntry<NameDictionary>(Node* dictionary,
   StoreValueByKeyIndex<NameDictionary>(dictionary, index, value);
 
   // Prepare details of the new property.
-  const int kInitialIndex = 0;
-  PropertyDetails d(kData, NONE, kInitialIndex, PropertyCellType::kNoCell);
+  PropertyDetails d(kData, NONE, PropertyCellType::kNoCell);
   enum_index =
       SmiShl(enum_index, PropertyDetails::DictionaryStorageField::kShift);
-  STATIC_ASSERT(kInitialIndex == 0);
+  // We OR over the actual index below, so we expect the initial value to be 0.
+  DCHECK_EQ(0, d.dictionary_index());
   VARIABLE(var_details, MachineRepresentation::kTaggedSigned,
            SmiOr(SmiConstant(d.AsSmi()), enum_index));
 
@@ -5197,21 +5197,17 @@ void CodeStubAssembler::Add(Node* dictionary, Node* key, Node* value,
   CSA_ASSERT(this, SmiAbove(capacity, new_nof));
   Node* half_of_free_elements = SmiShr(SmiSub(capacity, new_nof), 1);
   GotoIf(SmiAbove(deleted, half_of_free_elements), bailout);
-  Node* enum_index = nullptr;
-  if (Dictionary::kIsEnumerable) {
-    enum_index = GetNextEnumerationIndex<Dictionary>(dictionary);
-    Node* new_enum_index = SmiAdd(enum_index, SmiConstant(1));
-    Node* max_enum_index =
-        SmiConstant(PropertyDetails::DictionaryStorageField::kMax);
-    GotoIf(SmiAbove(new_enum_index, max_enum_index), bailout);
-
-    // No more bailouts after this point.
-    // Operations from here on can have side effects.
-
-    SetNextEnumerationIndex<Dictionary>(dictionary, new_enum_index);
-  } else {
-    USE(enum_index);
-  }
+
+  Node* enum_index = GetNextEnumerationIndex<Dictionary>(dictionary);
+  Node* new_enum_index = SmiAdd(enum_index, SmiConstant(1));
+  Node* max_enum_index =
+      SmiConstant(PropertyDetails::DictionaryStorageField::kMax);
+  GotoIf(SmiAbove(new_enum_index, max_enum_index), bailout);
+
+  // No more bailouts after this point.
+  // Operations from here on can have side effects.
+
+  SetNextEnumerationIndex<Dictionary>(dictionary, new_enum_index);
   SetNumberOfElements<Dictionary>(dictionary, new_nof);
 
   VARIABLE(var_key_index, MachineType::PointerRepresentation());

src/elements.cc

@@ -1303,11 +1303,11 @@ class ElementsAccessorBase : public ElementsAccessor {
 
   static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store,
                                         uint32_t entry) {
-    return PropertyDetails(kData, NONE, 0, PropertyCellType::kNoCell);
+    return PropertyDetails(kData, NONE, PropertyCellType::kNoCell);
   }
 
   static PropertyDetails GetDetailsImpl(JSObject* holder, uint32_t entry) {
-    return PropertyDetails(kData, NONE, 0, PropertyCellType::kNoCell);
+    return PropertyDetails(kData, NONE, PropertyCellType::kNoCell);
   }
 
   PropertyDetails GetDetails(JSObject* holder, uint32_t entry) final {
@@ -1467,15 +1467,16 @@ class DictionaryElementsAccessor
     if (attributes != NONE) object->RequireSlowElements(dictionary);
     dictionary->ValueAtPut(entry, *value);
     PropertyDetails details = dictionary->DetailsAt(entry);
-    details = PropertyDetails(kData, attributes, details.dictionary_index(),
-                              PropertyCellType::kNoCell);
+    details = PropertyDetails(kData, attributes, PropertyCellType::kNoCell,
+                              details.dictionary_index());
+
     dictionary->DetailsAtPut(entry, details);
   }
 
   static void AddImpl(Handle<JSObject> object, uint32_t index,
                       Handle<Object> value, PropertyAttributes attributes,
                       uint32_t new_capacity) {
-    PropertyDetails details(kData, attributes, 0, PropertyCellType::kNoCell);
+    PropertyDetails details(kData, attributes, PropertyCellType::kNoCell);
     Handle<SeededNumberDictionary> dictionary =
         object->HasFastElements() || object->HasFastStringWrapperElements()
             ? JSObject::NormalizeElements(object)
@@ -2790,12 +2791,12 @@ class TypedElementsAccessor
   }
 
   static PropertyDetails GetDetailsImpl(JSObject* holder, uint32_t entry) {
-    return PropertyDetails(kData, DONT_DELETE, 0, PropertyCellType::kNoCell);
+    return PropertyDetails(kData, DONT_DELETE, PropertyCellType::kNoCell);
   }
 
   static PropertyDetails GetDetailsImpl(FixedArrayBase* backing_store,
                                         uint32_t entry) {
-    return PropertyDetails(kData, DONT_DELETE, 0, PropertyCellType::kNoCell);
+    return PropertyDetails(kData, DONT_DELETE, PropertyCellType::kNoCell);
   }
 
   static bool HasElementImpl(Isolate* isolate, JSObject* holder, uint32_t index,
@@ -3519,7 +3520,7 @@ class SloppyArgumentsElementsAccessor
         SloppyArgumentsElements::cast(holder->elements());
     uint32_t length = elements->parameter_map_length();
     if (entry < length) {
-      return PropertyDetails(kData, NONE, 0, PropertyCellType::kNoCell);
+      return PropertyDetails(kData, NONE, PropertyCellType::kNoCell);
     }
     FixedArray* arguments = elements->arguments();
     return ArgumentsAccessor::GetDetailsImpl(arguments, entry - length);
@@ -3730,7 +3731,7 @@ class SlowSloppyArgumentsElementsAccessor
         old_arguments->IsSeededNumberDictionary()
             ? Handle<SeededNumberDictionary>::cast(old_arguments)
             : JSObject::NormalizeElements(object);
-    PropertyDetails details(kData, attributes, 0, PropertyCellType::kNoCell);
+    PropertyDetails details(kData, attributes, PropertyCellType::kNoCell);
     Handle<SeededNumberDictionary> new_dictionary =
         SeededNumberDictionary::Add(dictionary, index, value, details);
     if (attributes != NONE) object->RequireSlowElements(*new_dictionary);
@@ -3762,7 +3763,7 @@ class SlowSloppyArgumentsElementsAccessor
         value = isolate->factory()->NewAliasedArgumentsEntry(context_entry);
       }
 
-      PropertyDetails details(kData, attributes, 0, PropertyCellType::kNoCell);
+      PropertyDetails details(kData, attributes, PropertyCellType::kNoCell);
       Handle<SeededNumberDictionary> arguments(
           SeededNumberDictionary::cast(elements->arguments()), isolate);
       arguments = SeededNumberDictionary::Add(arguments, entry, value, details);
@@ -3964,7 +3965,7 @@ class StringWrapperElementsAccessor
     if (entry < length) {
       PropertyAttributes attributes =
           static_cast<PropertyAttributes>(READ_ONLY | DONT_DELETE);
-      return PropertyDetails(kData, attributes, 0, PropertyCellType::kNoCell);
+      return PropertyDetails(kData, attributes, PropertyCellType::kNoCell);
     }
     return BackingStoreAccessor::GetDetailsImpl(holder, entry - length);
   }

src/factory.cc

@@ -1845,7 +1845,7 @@ Handle<JSGlobalObject> Factory::NewJSGlobalObject(
     PropertyDetails details = descs->GetDetails(i);
     // Only accessors are expected.
     DCHECK_EQ(kAccessor, details.kind());
-    PropertyDetails d(kAccessor, details.attributes(), i + 1,
+    PropertyDetails d(kAccessor, details.attributes(),
                       PropertyCellType::kMutable);
     Handle<Name> name(descs->GetKey(i));
     Handle<PropertyCell> cell = NewPropertyCell();

src/lookup.cc

@@ -289,7 +289,7 @@ void LookupIterator::ReconfigureDataProperty(Handle<Object> value,
   }
 
   if (!IsElement() && !holder->HasFastProperties()) {
-    PropertyDetails details(kData, attributes, 0, PropertyCellType::kMutable);
+    PropertyDetails details(kData, attributes, PropertyCellType::kMutable);
     if (holder->IsJSGlobalObject()) {
       Handle<GlobalDictionary> dictionary(holder->global_dictionary());
 
@@ -357,8 +357,8 @@ void LookupIterator::PrepareTransitionToDataProperty(
       // SetNextEnumerationIndex.
       int index = dictionary->NextEnumerationIndex();
       dictionary->SetNextEnumerationIndex(index + 1);
-      property_details_ = PropertyDetails(kData, attributes, index,
-                                          PropertyCellType::kUninitialized);
+      property_details_ = PropertyDetails(
+          kData, attributes, PropertyCellType::kUninitialized, index);
       PropertyCellType new_type =
           PropertyCell::UpdatedType(cell, value, property_details_);
       property_details_ = property_details_.set_cell_type(new_type);
@@ -368,7 +368,7 @@ void LookupIterator::PrepareTransitionToDataProperty(
     } else {
       // Don't set enumeration index (it will be set during value store).
       property_details_ =
-          PropertyDetails(kData, attributes, 0, PropertyCellType::kNoCell);
+          PropertyDetails(kData, attributes, PropertyCellType::kNoCell);
       transition_ = map;
     }
     return;
@@ -382,7 +382,7 @@ void LookupIterator::PrepareTransitionToDataProperty(
   if (transition->is_dictionary_map()) {
     // Don't set enumeration index (it will be set during value store).
     property_details_ =
-        PropertyDetails(kData, attributes, 0, PropertyCellType::kNoCell);
+        PropertyDetails(kData, attributes, PropertyCellType::kNoCell);
   } else {
     property_details_ = transition->GetLastDescriptorDetails();
     has_property_ = true;
@@ -530,7 +530,7 @@ void LookupIterator::TransitionToAccessorPair(Handle<Object> pair,
   Handle<JSObject> receiver = GetStoreTarget();
   holder_ = receiver;
 
-  PropertyDetails details(kAccessor, attributes, 0, PropertyCellType::kMutable);
+  PropertyDetails details(kAccessor, attributes, PropertyCellType::kMutable);
 
   if (IsElement()) {
     // TODO(verwaest): Move code into the element accessor.

src/objects/dictionary.h

@@ -62,20 +62,8 @@ class Dictionary : public HashTable<Derived, Shape> {
     return DerivedHashTable::Shrink(dictionary);
   }
 
-  int NextEnumerationIndex() { UNREACHABLE(); }
-  void SetNextEnumerationIndex(int index) { UNREACHABLE(); }
-  static Handle<FixedArray> IterationIndices(Handle<Derived> dictionary) {
-    UNREACHABLE();
-  }
-
   int NumberOfEnumerableProperties();
 
-  // Creates a new dictionary.
-  MUST_USE_RESULT static Handle<Derived> New(
-      Isolate* isolate, int at_least_space_for,
-      PretenureFlag pretenure = NOT_TENURED,
-      MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY);
-
   // Creates an dictionary with minimal possible capacity.
   MUST_USE_RESULT static Handle<Derived> NewEmpty(
       Isolate* isolate, PretenureFlag pretenure = NOT_TENURED);
@@ -83,9 +71,6 @@ class Dictionary : public HashTable<Derived, Shape> {
   // Ensures that a new dictionary is created when the capacity is checked.
   void SetRequiresCopyOnCapacityChange();
 
-  // Ensure enough space for n additional elements.
-  static Handle<Derived> EnsureCapacity(Handle<Derived> obj, int n);
-
 #ifdef OBJECT_PRINT
   // For our gdb macros, we should perhaps change these in the future.
   void Print();
@@ -105,8 +90,6 @@ class Dictionary : public HashTable<Derived, Shape> {
                                              PropertyDetails details,
                                              int* entry_out = nullptr);
 
-  static const bool kIsEnumerable = Shape::kIsEnumerable;
-
  protected:
   // Generic at put operation.
   MUST_USE_RESULT static Handle<Derived> AtPut(Handle<Derived> dictionary,
@@ -153,12 +136,13 @@ class NameDictionaryShape : public BaseDictionaryShape<Handle<Name>> {
   static const int kEntrySize = 3;
   static const int kEntryValueIndex = 1;
   static const int kEntryDetailsIndex = 2;
-  static const bool kIsEnumerable = true;
   static const bool kNeedsHoleCheck = false;
 };
 
 template <typename Derived, typename Shape>
 class BaseNameDictionary : public Dictionary<Derived, Shape> {
+  typedef typename Shape::Key Key;
+
  public:
   static const int kNextEnumerationIndexIndex =
       HashTableBase::kPrefixStartIndex;
@@ -174,6 +158,12 @@ class BaseNameDictionary : public Dictionary<Derived, Shape> {
     return Smi::cast(this->get(kNextEnumerationIndexIndex))->value();
   }
 
+  // Creates a new dictionary.
+  MUST_USE_RESULT static Handle<Derived> New(
+      Isolate* isolate, int at_least_space_for,
+      MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY,
+      PretenureFlag pretenure = NOT_TENURED);
+
   // Collect the keys into the given KeyAccumulator, in ascending chronological
   // order of property creation.
   static void CollectKeysTo(Handle<Derived> dictionary, KeyAccumulator* keys);
@@ -185,6 +175,14 @@ class BaseNameDictionary : public Dictionary<Derived, Shape> {
   static void CopyEnumKeysTo(Handle<Derived> dictionary,
                              Handle<FixedArray> storage, KeyCollectionMode mode,
                              KeyAccumulator* accumulator);
+
+  // Ensure enough space for n additional elements.
+  static Handle<Derived> EnsureCapacity(Handle<Derived> dictionary, int n);
+
+  MUST_USE_RESULT static Handle<Derived> Add(Handle<Derived> dictionary,
+                                             Key key, Handle<Object> value,
+                                             PropertyDetails details,
+                                             int* entry_out = nullptr);
 };
 
 class NameDictionary
@@ -227,7 +225,6 @@ class NumberDictionaryShape : public BaseDictionaryShape<uint32_t> {
  public:
   static inline bool IsMatch(uint32_t key, Object* other);
   static inline Handle<Object> AsHandle(Isolate* isolate, uint32_t key);
-  static const bool kIsEnumerable = false;
 };
 
 class SeededNumberDictionaryShape : public NumberDictionaryShape {

src/objects/hash-table.h

@@ -194,6 +194,10 @@ class HashTable : public HashTableBase {
     return (entry * kEntrySize) + kElementsStartIndex;
   }
 
+  // Ensure enough space for n additional elements.
+  MUST_USE_RESULT static Handle<Derived> EnsureCapacity(
+      Handle<Derived> table, int n, PretenureFlag pretenure = NOT_TENURED);
+
  protected:
   friend class ObjectHashTable;
 
@@ -207,10 +211,6 @@ class HashTable : public HashTableBase {
   // Attempt to shrink hash table after removal of key.
   MUST_USE_RESULT static Handle<Derived> Shrink(Handle<Derived> table);
 
-  // Ensure enough space for n additional elements.
-  MUST_USE_RESULT static Handle<Derived> EnsureCapacity(
-      Handle<Derived> table, int n, PretenureFlag pretenure = NOT_TENURED);
-
   // Returns true if this table has sufficient capacity for adding n elements.
   bool HasSufficientCapacityToAdd(int number_of_additional_elements);
 

src/property-details.h

@@ -231,11 +231,11 @@ enum class PropertyCellConstantType {
 class PropertyDetails BASE_EMBEDDED {
  public:
   // Property details for dictionary mode properties/elements.
-  PropertyDetails(PropertyKind kind, PropertyAttributes attributes, int index,
-                  PropertyCellType cell_type) {
+  PropertyDetails(PropertyKind kind, PropertyAttributes attributes,
+                  PropertyCellType cell_type, int dictionary_index = 0) {
     value_ = KindField::encode(kind) | LocationField::encode(kField) |
              AttributesField::encode(attributes) |
-             DictionaryStorageField::encode(index) |
+             DictionaryStorageField::encode(dictionary_index) |
              PropertyCellTypeField::encode(cell_type);
   }
 
@@ -252,7 +252,7 @@ class PropertyDetails BASE_EMBEDDED {
 
   static PropertyDetails Empty(
       PropertyCellType cell_type = PropertyCellType::kNoCell) {
-    return PropertyDetails(kData, NONE, 0, cell_type);
+    return PropertyDetails(kData, NONE, cell_type);
   }
 
   int pointer() const { return DescriptorPointer::decode(value_); }

src/runtime/runtime-object.cc

@@ -306,10 +306,8 @@ RUNTIME_FUNCTION(Runtime_AddDictionaryProperty) {
   DCHECK(name->IsUniqueName());
 
   Handle<NameDictionary> dictionary(receiver->property_dictionary(), isolate);
-  int entry;
-  PropertyDetails property_details(kData, NONE, 0, PropertyCellType::kNoCell);
-  dictionary =
-      NameDictionary::Add(dictionary, name, value, property_details, &entry);
+  PropertyDetails property_details(kData, NONE, PropertyCellType::kNoCell);
+  dictionary = NameDictionary::Add(dictionary, name, value, property_details);
   receiver->set_properties(*dictionary);
   return *value;
 }

test/cctest/test-dictionary.cc

@@ -302,7 +302,8 @@ TEST(SetRequiresCopyOnCapacityChange) {
   LocalContext context;
   v8::HandleScope scope(context->GetIsolate());
   Isolate* isolate = CcTest::i_isolate();
-  Handle<NameDictionary> dict = NameDictionary::New(isolate, 0, TENURED);
+  Handle<NameDictionary> dict =
+      NameDictionary::New(isolate, 0, USE_DEFAULT_MINIMUM_CAPACITY, TENURED);
   dict->SetRequiresCopyOnCapacityChange();
   Handle<Name> key = isolate->factory()->InternalizeString(
       v8::Utils::OpenHandle(*v8_str("key")));