[keys] support shadowing keys in the KeyAccumulator

This cl fixes the long-standing bug for for-in with shadowing properties.

BUG=v8:705

Review-Url: https://codereview.chromium.org/2081733002
Cr-Commit-Position: refs/heads/master@{#37333}

src/elements.cc

@@ -1323,10 +1323,14 @@ class DictionaryElementsAccessor
     int insertion_index = 0;
     PropertyFilter filter = keys->filter();
     for (int i = 0; i < capacity; i++) {
-      uint32_t key = GetKeyForEntryImpl(isolate, dictionary, i, filter);
-      if (key == kMaxUInt32) continue;
-      Handle<Object> key_handle = isolate->factory()->NewNumberFromUint(key);
-      elements->set(insertion_index, *key_handle);
+      Object* raw_key = dictionary->KeyAt(i);
+      if (!dictionary->IsKey(isolate, raw_key)) continue;
+      uint32_t key = FilterKey(dictionary, i, raw_key, filter);
+      if (key == kMaxUInt32) {
+        keys->AddShadowKey(raw_key);
+        continue;
+      }
+      elements->set(insertion_index, raw_key);
       insertion_index++;
     }
     SortIndices(elements, insertion_index);

src/keys.cc

@@ -69,6 +69,7 @@ void KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
   } else if (filter_ & SKIP_STRINGS) {
     return;
   }
+  if (IsShadowed(key)) return;
   if (keys_.is_null()) {
     keys_ = OrderedHashSet::Allocate(isolate_, 16);
   }
@@ -96,13 +97,15 @@ void KeyAccumulator::AddKeys(Handle<JSObject> array_like,
   accessor->AddElementsToKeyAccumulator(array_like, this, convert);
 }
 
-MaybeHandle<FixedArray> FilterProxyKeys(Isolate* isolate, Handle<JSProxy> owner,
+MaybeHandle<FixedArray> FilterProxyKeys(KeyAccumulator* accumulator,
+                                        Handle<JSProxy> owner,
                                         Handle<FixedArray> keys,
                                         PropertyFilter filter) {
   if (filter == ALL_PROPERTIES) {
     // Nothing to do.
     return keys;
   }
+  Isolate* isolate = accumulator->isolate();
   int store_position = 0;
   for (int i = 0; i < keys->length(); ++i) {
     Handle<Name> key(Name::cast(keys->get(i)), isolate);
@@ -112,7 +115,11 @@ MaybeHandle<FixedArray> FilterProxyKeys(Isolate* isolate, Handle<JSProxy> owner,
       Maybe<bool> found =
           JSProxy::GetOwnPropertyDescriptor(isolate, owner, key, &desc);
       MAYBE_RETURN(found, MaybeHandle<FixedArray>());
-      if (!found.FromJust() || !desc.enumerable()) continue;  // Skip this key.
+      if (!found.FromJust()) continue;
+      if (!desc.enumerable()) {
+        accumulator->AddShadowKey(key);
+        continue;
+      }
     }
     // Keep this key.
     if (store_position != i) {
@@ -131,7 +138,7 @@ Maybe<bool> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> proxy,
   if (filter_proxy_keys_) {
     DCHECK(!is_for_in_);
     ASSIGN_RETURN_ON_EXCEPTION_VALUE(
-        isolate_, keys, FilterProxyKeys(isolate_, proxy, keys, filter_),
+        isolate_, keys, FilterProxyKeys(this, proxy, keys, filter_),
         Nothing<bool>());
   }
   if (mode_ == KeyCollectionMode::kOwnOnly && !is_for_in_) {
@@ -183,6 +190,23 @@ Maybe<bool> KeyAccumulator::CollectKeys(Handle<JSReceiver> receiver,
   return Just(true);
 }
 
+bool KeyAccumulator::IsShadowed(Handle<Object> key) {
+  if (shadowed_keys_.is_null()) return false;
+  return shadowed_keys_->Has(isolate_, key);
+}
+
+void KeyAccumulator::AddShadowKey(Object* key) {
+  if (mode_ == KeyCollectionMode::kOwnOnly) return;
+  AddShadowKey(handle(key, isolate_));
+}
+void KeyAccumulator::AddShadowKey(Handle<Object> key) {
+  if (mode_ == KeyCollectionMode::kOwnOnly) return;
+  if (shadowed_keys_.is_null()) {
+    shadowed_keys_ = ObjectHashSet::New(isolate_, 16);
+  }
+  shadowed_keys_ = ObjectHashSet::Add(shadowed_keys_, key);
+}
+
 namespace {
 
 void TrySettingEmptyEnumCache(JSReceiver* object) {
@@ -329,7 +353,7 @@ Handle<FixedArray> GetOwnKeysWithElements(Isolate* isolate,
     keys = GetFastEnumPropertyKeys(isolate, object);
   } else {
     // TODO(cbruni): preallocate big enough array to also hold elements.
-    keys = KeyAccumulator::GetEnumPropertyKeys(isolate, object);
+    keys = KeyAccumulator::GetOwnEnumPropertyKeys(isolate, object);
   }
   Handle<FixedArray> result =
       accessor->PrependElementIndices(object, keys, convert, ONLY_ENUMERABLE);
@@ -495,33 +519,87 @@ int CollectOwnPropertyNamesInternal(Handle<JSObject> object,
                                     Handle<DescriptorArray> descs,
                                     int start_index, int limit) {
   int first_skipped = -1;
+  PropertyFilter filter = keys->filter();
+  KeyCollectionMode mode = keys->mode();
   for (int i = start_index; i < limit; i++) {
+    bool is_shadowing_key = false;
     PropertyDetails details = descs->GetDetails(i);
-    if ((details.attributes() & keys->filter()) != 0) continue;
-    if (keys->filter() & ONLY_ALL_CAN_READ) {
+
+    if ((details.attributes() & filter) != 0) {
+      if (mode == KeyCollectionMode::kIncludePrototypes) {
+        is_shadowing_key = true;
+      } else {
+        continue;
+      }
+    }
+
+    if (filter & ONLY_ALL_CAN_READ) {
       if (details.kind() != kAccessor) continue;
       Object* accessors = descs->GetValue(i);
       if (!accessors->IsAccessorInfo()) continue;
       if (!AccessorInfo::cast(accessors)->all_can_read()) continue;
     }
+
     Name* key = descs->GetKey(i);
     if (skip_symbols == key->IsSymbol()) {
       if (first_skipped == -1) first_skipped = i;
       continue;
     }
     if (key->FilterKey(keys->filter())) continue;
-    keys->AddKey(key, DO_NOT_CONVERT);
+
+    if (is_shadowing_key) {
+      keys->AddShadowKey(key);
+    } else {
+      keys->AddKey(key, DO_NOT_CONVERT);
+    }
   }
   return first_skipped;
 }
 
+template <class T>
+Handle<FixedArray> GetOwnEnumPropertyDictionaryKeys(Isolate* isolate,
+                                                    KeyCollectionMode mode,
+                                                    KeyAccumulator* accumulator,
+                                                    Handle<JSObject> object,
+                                                    T* raw_dictionary) {
+  Handle<T> dictionary(raw_dictionary, isolate);
+  int length = dictionary->NumberOfEnumElements();
+  if (length == 0) {
+    return isolate->factory()->empty_fixed_array();
+  }
+  Handle<FixedArray> storage = isolate->factory()->NewFixedArray(length);
+  T::CopyEnumKeysTo(dictionary, storage, mode, accumulator);
+  return storage;
+}
 }  // namespace
 
 Maybe<bool> KeyAccumulator::CollectOwnPropertyNames(Handle<JSReceiver> receiver,
                                                     Handle<JSObject> object) {
   if (filter_ == ENUMERABLE_STRINGS) {
-    Handle<FixedArray> enum_keys =
-        KeyAccumulator::GetEnumPropertyKeys(isolate_, object);
+    Handle<FixedArray> enum_keys;
+    if (object->HasFastProperties()) {
+      enum_keys = KeyAccumulator::GetOwnEnumPropertyKeys(isolate_, object);
+      // If the number of properties equals the length of enumerable properties
+      // we do not have to filter out non-enumerable ones
+      Map* map = object->map();
+      int nof_descriptors = map->NumberOfOwnDescriptors();
+      if (enum_keys->length() != nof_descriptors) {
+        Handle<DescriptorArray> descs =
+            Handle<DescriptorArray>(map->instance_descriptors(), isolate_);
+        for (int i = 0; i < nof_descriptors; i++) {
+          PropertyDetails details = descs->GetDetails(i);
+          if (!details.IsDontEnum()) continue;
+          Object* key = descs->GetKey(i);
+          this->AddShadowKey(key);
+        }
+      }
+    } else if (object->IsJSGlobalObject()) {
+      enum_keys = GetOwnEnumPropertyDictionaryKeys(
+          isolate_, mode_, this, object, object->global_dictionary());
+    } else {
+      enum_keys = GetOwnEnumPropertyDictionaryKeys(
+          isolate_, mode_, this, object, object->property_dictionary());
+    }
     AddKeys(enum_keys, DO_NOT_CONVERT);
   } else {
     if (object->HasFastProperties()) {
@@ -538,10 +616,10 @@ Maybe<bool> KeyAccumulator::CollectOwnPropertyNames(Handle<JSReceiver> receiver,
       }
     } else if (object->IsJSGlobalObject()) {
       GlobalDictionary::CollectKeysTo(
-          handle(object->global_dictionary(), isolate_), this, filter_);
+          handle(object->global_dictionary(), isolate_), this);
     } else {
       NameDictionary::CollectKeysTo(
-          handle(object->property_dictionary(), isolate_), this, filter_);
+          handle(object->property_dictionary(), isolate_), this);
     }
   }
   // Add the property keys from the interceptor.
@@ -608,28 +686,18 @@ Maybe<bool> KeyAccumulator::CollectOwnKeys(Handle<JSReceiver> receiver,
 }
 
 // static
-Handle<FixedArray> KeyAccumulator::GetEnumPropertyKeys(
+Handle<FixedArray> KeyAccumulator::GetOwnEnumPropertyKeys(
     Isolate* isolate, Handle<JSObject> object) {
   if (object->HasFastProperties()) {
     return GetFastEnumPropertyKeys(isolate, object);
   } else if (object->IsJSGlobalObject()) {
-    Handle<GlobalDictionary> dictionary(object->global_dictionary(), isolate);
-    int length = dictionary->NumberOfEnumElements();
-    if (length == 0) {
-      return isolate->factory()->empty_fixed_array();
-    }
-    Handle<FixedArray> storage = isolate->factory()->NewFixedArray(length);
-    dictionary->CopyEnumKeysTo(*storage);
-    return storage;
+    return GetOwnEnumPropertyDictionaryKeys(
+        isolate, KeyCollectionMode::kOwnOnly, nullptr, object,
+        object->global_dictionary());
   } else {
-    Handle<NameDictionary> dictionary(object->property_dictionary(), isolate);
-    int length = dictionary->NumberOfEnumElements();
-    if (length == 0) {
-      return isolate->factory()->empty_fixed_array();
-    }
-    Handle<FixedArray> storage = isolate->factory()->NewFixedArray(length);
-    dictionary->CopyEnumKeysTo(*storage);
-    return storage;
+    return GetOwnEnumPropertyDictionaryKeys(
+        isolate, KeyCollectionMode::kOwnOnly, nullptr, object,
+        object->property_dictionary());
   }
 }
 

src/keys.h

@@ -53,8 +53,8 @@ class KeyAccumulator final BASE_EMBEDDED {
       Handle<AccessCheckInfo> access_check_info, Handle<JSReceiver> receiver,
       Handle<JSObject> object);
 
-  static Handle<FixedArray> GetEnumPropertyKeys(Isolate* isolate,
-                                                Handle<JSObject> object);
+  static Handle<FixedArray> GetOwnEnumPropertyKeys(Isolate* isolate,
+                                                   Handle<JSObject> object);
 
   void AddKey(Object* key, AddKeyConversion convert = DO_NOT_CONVERT);
   void AddKey(Handle<Object> key, AddKeyConversion convert = DO_NOT_CONVERT);
@@ -64,13 +64,27 @@ class KeyAccumulator final BASE_EMBEDDED {
   // Jump to the next level, pushing the current |levelLength_| to
   // |levelLengths_| and adding a new list to |elements_|.
   Isolate* isolate() { return isolate_; }
+  // Filter keys based on their property descriptors.
   PropertyFilter filter() { return filter_; }
+  // The collection mode defines whether we collect the keys from the prototype
+  // chain or only look at the receiver.
+  KeyCollectionMode mode() { return mode_; }
   void set_filter_proxy_keys(bool filter) { filter_proxy_keys_ = filter; }
+  // In case of for-in loops we have to treat JSProxy keys differently and
+  // deduplicate them. Additionally we convert JSProxy keys back to array
+  // indices.
   void set_is_for_in(bool value) { is_for_in_ = value; }
   void set_skip_indices(bool value) { skip_indices_ = value; }
+  // The last_non_empty_prototype is used to limit the prototypes for which
+  // we have to keep track of non-enumerable keys that can shadow keys
+  // repeated on the prototype chain.
   void set_last_non_empty_prototype(Handle<JSReceiver> object) {
     last_non_empty_prototype_ = object;
   }
+  // Shadowing keys are used to filter keys. This happens when non-enumerable
+  // keys appear again on the prototype chain.
+  void AddShadowKey(Object* key);
+  void AddShadowKey(Handle<Object> key);
 
  private:
   Maybe<bool> CollectOwnKeys(Handle<JSReceiver> receiver,
@@ -79,17 +93,18 @@ class KeyAccumulator final BASE_EMBEDDED {
                                     Handle<JSProxy> proxy);
   Maybe<bool> CollectOwnJSProxyTargetKeys(Handle<JSProxy> proxy,
                                           Handle<JSReceiver> target);
-
   Maybe<bool> AddKeysFromJSProxy(Handle<JSProxy> proxy,
                                  Handle<FixedArray> keys);
-
+  bool IsShadowed(Handle<Object> key);
   Handle<OrderedHashSet> keys() { return Handle<OrderedHashSet>::cast(keys_); }
 
   Isolate* isolate_;
   // keys_ is either an Handle<OrderedHashSet> or in the case of own JSProxy
-  // keys a Handle<FixedArray>.
+  // keys a Handle<FixedArray>. The OrderedHashSet is in-place converted to the
+  // result list, a FixedArray containing all collected keys.
   Handle<FixedArray> keys_;
   Handle<JSReceiver> last_non_empty_prototype_;
+  Handle<ObjectHashSet> shadowed_keys_;
   KeyCollectionMode mode_;
   PropertyFilter filter_;
   bool filter_proxy_keys_ = true;
@@ -101,7 +116,8 @@ class KeyAccumulator final BASE_EMBEDDED {
 
 // The FastKeyAccumulator handles the cases where there are no elements on the
 // prototype chain and forwords the complex/slow cases to the normal
-// KeyAccumulator.
+// KeyAccumulator. This significantly speeds up the cases where the OWN_ONLY
+// case where we do not have to walk the prototype chain.
 class FastKeyAccumulator {
  public:
   FastKeyAccumulator(Isolate* isolate, Handle<JSReceiver> receiver,

src/messages.cc

@@ -286,7 +286,7 @@ Handle<Object> CallSite::GetMethodName() {
     Handle<JSObject> current_obj = Handle<JSObject>::cast(current);
     if (current_obj->IsAccessCheckNeeded()) break;
     Handle<FixedArray> keys =
-        KeyAccumulator::GetEnumPropertyKeys(isolate_, current_obj);
+        KeyAccumulator::GetOwnEnumPropertyKeys(isolate_, current_obj);
     for (int i = 0; i < keys->length(); i++) {
       HandleScope inner_scope(isolate_);
       if (!keys->get(i)->IsName()) continue;

src/objects-inl.h

@@ -859,6 +859,8 @@ bool HeapObject::IsStringTable() const { return IsHashTable(); }
 
 bool HeapObject::IsStringSet() const { return IsHashTable(); }
 
+bool HeapObject::IsObjectHashSet() const { return IsHashTable(); }
+
 bool HeapObject::IsNormalizedMapCache() const {
   return NormalizedMapCache::IsNormalizedMapCache(this);
 }
@@ -3073,7 +3075,6 @@ int HashTable<Derived, Shape, Key>::FindEntry(Isolate* isolate, Key key) {
   return FindEntry(isolate, key, HashTable::Hash(key));
 }
 
-
 // Find entry for key otherwise return kNotFound.
 template <typename Derived, typename Shape, typename Key>
 int HashTable<Derived, Shape, Key>::FindEntry(Isolate* isolate, Key key,
@@ -3095,6 +3096,26 @@ int HashTable<Derived, Shape, Key>::FindEntry(Isolate* isolate, Key key,
   return kNotFound;
 }
 
+template <typename Derived, typename Shape, typename Key>
+bool HashTable<Derived, Shape, Key>::Has(Key key) {
+  return FindEntry(key) != kNotFound;
+}
+
+template <typename Derived, typename Shape, typename Key>
+bool HashTable<Derived, Shape, Key>::Has(Isolate* isolate, Key key) {
+  return FindEntry(isolate, key) != kNotFound;
+}
+
+bool ObjectHashSet::Has(Isolate* isolate, Handle<Object> key, int32_t hash) {
+  return FindEntry(isolate, key, hash) != kNotFound;
+}
+
+bool ObjectHashSet::Has(Isolate* isolate, Handle<Object> key) {
+  Object* hash = key->GetHash();
+  if (!hash->IsSmi()) return false;
+  return FindEntry(isolate, key, Smi::cast(hash)->value()) != kNotFound;
+}
+
 bool StringSetShape::IsMatch(String* key, Object* value) {
   return value->IsString() && key->Equals(String::cast(value));
 }
@@ -3191,6 +3212,7 @@ CAST_ACCESSOR(NameDictionary)
 CAST_ACCESSOR(NormalizedMapCache)
 CAST_ACCESSOR(Object)
 CAST_ACCESSOR(ObjectHashTable)
+CAST_ACCESSOR(ObjectHashSet)
 CAST_ACCESSOR(Oddball)
 CAST_ACCESSOR(OrderedHashMap)
 CAST_ACCESSOR(OrderedHashSet)

src/objects.cc

@@ -16515,6 +16515,11 @@ template Handle<NameDictionary>
 HashTable<NameDictionary, NameDictionaryShape, Handle<Name> >::
     New(Isolate*, int, MinimumCapacity, PretenureFlag);
 
+template Handle<ObjectHashSet> HashTable<ObjectHashSet, ObjectHashSetShape,
+                                         Handle<Object>>::New(Isolate*, int n,
+                                                              MinimumCapacity,
+                                                              PretenureFlag);
+
 template Handle<NameDictionary>
 HashTable<NameDictionary, NameDictionaryShape, Handle<Name> >::
     Shrink(Handle<NameDictionary>, Handle<Name>);
@@ -16585,24 +16590,33 @@ template int Dictionary<GlobalDictionary, GlobalDictionaryShape, Handle<Name>>::
 template int Dictionary<NameDictionary, NameDictionaryShape, Handle<Name>>::
     NumberOfElementsFilterAttributes(PropertyFilter filter);
 
-template void Dictionary<GlobalDictionary, GlobalDictionaryShape,
-                         Handle<Name>>::CopyEnumKeysTo(FixedArray* storage);
+template void
+Dictionary<GlobalDictionary, GlobalDictionaryShape, Handle<Name>>::
+    CopyEnumKeysTo(Handle<Dictionary<GlobalDictionary, GlobalDictionaryShape,
+                                     Handle<Name>>>
+                       dictionary,
+                   Handle<FixedArray> storage, KeyCollectionMode mode,
+                   KeyAccumulator* accumulator);
 
-template void Dictionary<NameDictionary, NameDictionaryShape,
-                         Handle<Name>>::CopyEnumKeysTo(FixedArray* storage);
+template void
+Dictionary<NameDictionary, NameDictionaryShape, Handle<Name>>::CopyEnumKeysTo(
+    Handle<Dictionary<NameDictionary, NameDictionaryShape, Handle<Name>>>
+        dictionary,
+    Handle<FixedArray> storage, KeyCollectionMode mode,
+    KeyAccumulator* accumulator);
 
 template void
 Dictionary<GlobalDictionary, GlobalDictionaryShape, Handle<Name>>::
     CollectKeysTo(Handle<Dictionary<GlobalDictionary, GlobalDictionaryShape,
                                     Handle<Name>>>
                       dictionary,
-                  KeyAccumulator* keys, PropertyFilter filter);
+                  KeyAccumulator* keys);
 
 template void
 Dictionary<NameDictionary, NameDictionaryShape, Handle<Name>>::CollectKeysTo(
     Handle<Dictionary<NameDictionary, NameDictionaryShape, Handle<Name>>>
         dictionary,
-    KeyAccumulator* keys, PropertyFilter filter);
+    KeyAccumulator* keys);
 
 Handle<Object> JSObject::PrepareSlowElementsForSort(
     Handle<JSObject> object, uint32_t limit) {
@@ -17099,6 +17113,20 @@ bool StringSet::Has(Handle<String> name) {
   return FindEntry(*name) != kNotFound;
 }
 
+Handle<ObjectHashSet> ObjectHashSet::Add(Handle<ObjectHashSet> set,
+                                         Handle<Object> key) {
+  Isolate* isolate = set->GetIsolate();
+  int32_t hash = Object::GetOrCreateHash(isolate, key)->value();
+
+  if (!set->Has(isolate, key, hash)) {
+    set = EnsureCapacity(set, 1, key);
+    int entry = set->FindInsertionEntry(hash);
+    set->set(EntryToIndex(entry), *key);
+    set->ElementAdded();
+  }
+  return set;
+}
+
 Handle<Object> CompilationCacheTable::Lookup(Handle<String> src,
                                              Handle<Context> context,
                                              LanguageMode language_mode) {
@@ -17554,43 +17582,61 @@ struct EnumIndexComparator {
   Dictionary* dict;
 };
 
-
 template <typename Derived, typename Shape, typename Key>
-void Dictionary<Derived, Shape, Key>::CopyEnumKeysTo(FixedArray* storage) {
-  Isolate* isolate = this->GetIsolate();
+void Dictionary<Derived, Shape, Key>::CopyEnumKeysTo(
+    Handle<Dictionary<Derived, Shape, Key>> dictionary,
+    Handle<FixedArray> storage, KeyCollectionMode mode,
+    KeyAccumulator* accumulator) {
+  Isolate* isolate = dictionary->GetIsolate();
   int length = storage->length();
-  int capacity = this->Capacity();
+  int capacity = dictionary->Capacity();
   int properties = 0;
   for (int i = 0; i < capacity; i++) {
-    Object* k = this->KeyAt(i);
-    if (this->IsKey(isolate, k) && !k->IsSymbol()) {
-      PropertyDetails details = this->DetailsAt(i);
-      if (details.IsDontEnum() || this->IsDeleted(i)) continue;
+    Object* key = dictionary->KeyAt(i);
+    bool is_shadowing_key = false;
+    if (!dictionary->IsKey(isolate, key)) continue;
+    if (key->IsSymbol()) continue;
+    PropertyDetails details = dictionary->DetailsAt(i);
+    if (details.IsDontEnum()) {
+      if (mode == KeyCollectionMode::kIncludePrototypes) {
+        is_shadowing_key = true;
+      } else {
+        continue;
+      }
+    }
+    if (dictionary->IsDeleted(i)) continue;
+    if (is_shadowing_key) {
+      accumulator->AddShadowKey(key);
+      continue;
+    } else {
       storage->set(properties, Smi::FromInt(i));
-      properties++;
-      if (properties == length) break;
     }
+    properties++;
+    if (properties == length) break;
   }
+
   CHECK_EQ(length, properties);
-  EnumIndexComparator<Derived> cmp(static_cast<Derived*>(this));
+  DisallowHeapAllocation no_gc;
+  Dictionary<Derived, Shape, Key>* raw_dictionary = *dictionary;
+  FixedArray* raw_storage = *storage;
+  EnumIndexComparator<Derived> cmp(static_cast<Derived*>(*dictionary));
   Smi** start = reinterpret_cast<Smi**>(storage->GetFirstElementAddress());
   std::sort(start, start + length, cmp);
   for (int i = 0; i < length; i++) {
-    int index = Smi::cast(storage->get(i))->value();
-    storage->set(i, this->KeyAt(index));
+    int index = Smi::cast(raw_storage->get(i))->value();
+    raw_storage->set(i, raw_dictionary->KeyAt(index));
   }
 }
 
 template <typename Derived, typename Shape, typename Key>
 void Dictionary<Derived, Shape, Key>::CollectKeysTo(
-    Handle<Dictionary<Derived, Shape, Key> > dictionary, KeyAccumulator* keys,
-    PropertyFilter filter) {
+    Handle<Dictionary<Derived, Shape, Key>> dictionary, KeyAccumulator* keys) {
   Isolate* isolate = keys->isolate();
   int capacity = dictionary->Capacity();
   Handle<FixedArray> array =
       isolate->factory()->NewFixedArray(dictionary->NumberOfElements());
   int array_size = 0;
-
+  PropertyFilter filter = keys->filter();
   {
     DisallowHeapAllocation no_gc;
     Dictionary<Derived, Shape, Key>* raw_dict = *dictionary;
@@ -17599,7 +17645,10 @@ void Dictionary<Derived, Shape, Key>::CollectKeysTo(
       if (!raw_dict->IsKey(isolate, k) || k->FilterKey(filter)) continue;
       if (raw_dict->IsDeleted(i)) continue;
       PropertyDetails details = raw_dict->DetailsAt(i);
-      if ((details.attributes() & filter) != 0) continue;
+      if ((details.attributes() & filter) != 0) {
+        keys->AddShadowKey(k);
+        continue;
+      }
       if (filter & ONLY_ALL_CAN_READ) {
         if (details.kind() != kAccessor) continue;
         Object* accessors = raw_dict->ValueAt(i);

src/objects.h

@@ -1002,6 +1002,7 @@ template <class C> inline bool Is(Object* obj);
   V(PropertyCell)                \
   V(WeakCell)                    \
   V(ObjectHashTable)             \
+  V(ObjectHashSet)               \
   V(WeakHashTable)               \
   V(OrderedHashTable)
 
@@ -3230,6 +3231,8 @@ class HashTable : public HashTableBase {
   inline int FindEntry(Key key);
   inline int FindEntry(Isolate* isolate, Key key, int32_t hash);
   int FindEntry(Isolate* isolate, Key key);
+  inline bool Has(Isolate* isolate, Key key);
+  inline bool Has(Key key);
 
   // Rehashes the table in-place.
   void Rehash(Key key);
@@ -3456,11 +3459,13 @@ class Dictionary: public HashTable<Derived, Shape, Key> {
 
   // Collect the keys into the given KeyAccumulator, in ascending chronological
   // order of property creation.
-  static void CollectKeysTo(Handle<Dictionary<Derived, Shape, Key> > dictionary,
-                            KeyAccumulator* keys, PropertyFilter filter);
+  static void CollectKeysTo(Handle<Dictionary<Derived, Shape, Key>> dictionary,
+                            KeyAccumulator* keys);
 
   // Copies enumerable keys to preallocated fixed array.
-  void CopyEnumKeysTo(FixedArray* storage);
+  static void CopyEnumKeysTo(Handle<Dictionary<Derived, Shape, Key>> dictionary,
+                             Handle<FixedArray> storage, KeyCollectionMode mode,
+                             KeyAccumulator* accumulator);
 
   // Accessors for next enumeration index.
   void SetNextEnumerationIndex(int index) {
@@ -3806,6 +3811,23 @@ class ObjectHashTable: public HashTable<ObjectHashTable,
   }
 };
 
+class ObjectHashSetShape : public ObjectHashTableShape {
+ public:
+  static const int kPrefixSize = 0;
+  static const int kEntrySize = 1;
+};
+
+class ObjectHashSet
+    : public HashTable<ObjectHashSet, ObjectHashSetShape, Handle<Object>> {
+ public:
+  static Handle<ObjectHashSet> Add(Handle<ObjectHashSet> set,
+                                   Handle<Object> key);
+
+  inline bool Has(Isolate* isolate, Handle<Object> key, int32_t hash);
+  inline bool Has(Isolate* isolate, Handle<Object> key);
+
+  DECLARE_CAST(ObjectHashSet)
+};
 
 // OrderedHashTable is a HashTable with Object keys that preserves
 // insertion order. There are Map and Set interfaces (OrderedHashMap

test/cctest/test-dictionary.cc

@@ -113,6 +113,74 @@ TEST(HashMap) {
   TestHashMap(ObjectHashTable::New(isolate, 23));
 }
 
+template <typename HashSet>
+static void TestHashSet(Handle<HashSet> table) {
+  Isolate* isolate = CcTest::i_isolate();
+  Factory* factory = isolate->factory();
+
+  Handle<JSObject> a = factory->NewJSArray(7);
+  Handle<JSObject> b = factory->NewJSArray(11);
+  table = HashSet::Add(table, a);
+  CHECK_EQ(table->NumberOfElements(), 1);
+  CHECK(table->Has(isolate, a));
+  CHECK(!table->Has(isolate, b));
+
+  // Keys still have to be valid after objects were moved.
+  CcTest::heap()->CollectGarbage(NEW_SPACE);
+  CHECK_EQ(table->NumberOfElements(), 1);
+  CHECK(table->Has(isolate, a));
+  CHECK(!table->Has(isolate, b));
+
+  // Keys that are overwritten should not change number of elements.
+  table = HashSet::Add(table, a);
+  CHECK_EQ(table->NumberOfElements(), 1);
+  CHECK(table->Has(isolate, a));
+  CHECK(!table->Has(isolate, b));
+
+  // Keys that have been removed are mapped to the hole.
+  // TODO(cbruni): not implemented yet.
+  // bool was_present = false;
+  // table = HashSet::Remove(table, a, &was_present);
+  // CHECK(was_present);
+  // CHECK_EQ(table->NumberOfElements(), 0);
+  // CHECK(!table->Has(a));
+  // CHECK(!table->Has(b));
+
+  // Keys should map back to their respective values and also should get
+  // an identity hash code generated.
+  for (int i = 0; i < 100; i++) {
+    Handle<JSReceiver> key = factory->NewJSArray(7);
+    table = HashSet::Add(table, key);
+    CHECK_EQ(table->NumberOfElements(), i + 2);
+    CHECK(table->Has(isolate, key));
+    CHECK(JSReceiver::GetIdentityHash(isolate, key)->IsSmi());
+  }
+
+  // Keys never added to the map which already have an identity hash
+  // code should not be found.
+  for (int i = 0; i < 100; i++) {
+    Handle<JSReceiver> key = factory->NewJSArray(7);
+    CHECK(JSReceiver::GetOrCreateIdentityHash(isolate, key)->IsSmi());
+    CHECK(!table->Has(isolate, key));
+    CHECK(JSReceiver::GetIdentityHash(isolate, key)->IsSmi());
+  }
+
+  // Keys that don't have an identity hash should not be found and also
+  // should not get an identity hash code generated.
+  for (int i = 0; i < 100; i++) {
+    Handle<JSReceiver> key = factory->NewJSArray(7);
+    CHECK(!table->Has(isolate, key));
+    Object* identity_hash = JSReceiver::GetIdentityHash(isolate, key);
+    CHECK_EQ(CcTest::heap()->undefined_value(), identity_hash);
+  }
+}
+
+TEST(HashSet) {
+  LocalContext context;
+  v8::HandleScope scope(context->GetIsolate());
+  Isolate* isolate = CcTest::i_isolate();
+  TestHashSet(ObjectHashSet::New(isolate, 23));
+}
 
 class ObjectHashTableTest: public ObjectHashTable {
  public:

test/mjsunit/for-in.js

@@ -30,61 +30,136 @@
 function props(x) {
   var array = [];
   for (var p in x) array.push(p);
-  return array.sort();
+  return array;
 }
 
-assertEquals(0, props({}).length, "olen0");
-assertEquals(1, props({x:1}).length, "olen1");
-assertEquals(2, props({x:1, y:2}).length, "olen2");
+(function forInBasic() {
+  assertEquals(0, props({}).length, "olen0");
+  assertEquals(1, props({x:1}).length, "olen1");
+  assertEquals(2, props({x:1, y:2}).length, "olen2");
 
-assertArrayEquals(["x"], props({x:1}), "x");
-assertArrayEquals(["x", "y"], props({x:1, y:2}), "xy");
-assertArrayEquals(["x", "y", "zoom"], props({x:1, y:2, zoom:3}), "xyzoom");
+  assertArrayEquals(["x"], props({x:1}), "x");
+  assertArrayEquals(["x", "y"], props({x:1, y:2}), "xy");
+  assertArrayEquals(["x", "y", "zoom"], props({x:1, y:2, zoom:3}), "xyzoom");
 
-assertEquals(0, props([]).length, "alen0");
-assertEquals(1, props([1]).length, "alen1");
-assertEquals(2, props([1,2]).length, "alen2");
+  assertEquals(0, props([]).length, "alen0");
+  assertEquals(1, props([1]).length, "alen1");
+  assertEquals(2, props([1,2]).length, "alen2");
 
-assertArrayEquals(["0"], props([1]), "0");
-assertArrayEquals(["0", "1"], props([1,2]), "01");
-assertArrayEquals(["0", "1", "2"], props([1,2,3]), "012");
+  assertArrayEquals(["0"], props([1]), "0");
+  assertArrayEquals(["0", "1"], props([1,2]), "01");
+  assertArrayEquals(["0", "1", "2"], props([1,2,3]), "012");
+})();
 
-var o = {};
-var a = [];
-for (var i = 0x0020; i < 0x01ff; i+=2) {
-  var s = 'char:' + String.fromCharCode(i);
-  a.push(s);
-  o[s] = i;
-}
-assertArrayEquals(a, props(o), "charcodes");
-
-var a = [];
-assertEquals(0, props(a).length, "proplen0");
-a[Math.pow(2,30)-1] = 0;
-assertEquals(1, props(a).length, "proplen1");
-a[Math.pow(2,31)-1] = 0;
-assertEquals(2, props(a).length, "proplen2");
-a[1] = 0;
-assertEquals(3, props(a).length, "proplen3");
-
-for (var hest = 'hest' in {}) { }
-assertEquals('hest', hest, "empty-no-override");
-
-var result = '';
-for (var p in {a : [0], b : 1}) { result += p; }
-assertEquals('ab', result, "ab");
-
-var result = '';
-for (var p in {a : {v:1}, b : 1}) { result += p; }
-assertEquals('ab', result, "ab-nodeep");
-
-var result = '';
-for (var p in { get a() {}, b : 1}) { result += p; }
-assertEquals('ab', result, "abget");
-
-var result = '';
-for (var p in { get a() {}, set a(x) {}, b : 1}) { result += p; }
-assertEquals('ab', result, "abgetset");
+(function forInPrototype() {
+  // Fast properties + fast elements
+  var obj = {a:true, 3:true, 4:true};
+  obj.__proto__ = {c:true, b:true, 2:true, 1:true, 5:true};
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a125cb".split(""), props(obj));
+  }
+  // Fast properties + dictionary elements
+  delete obj.__proto__[2];
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a15cb".split(""), props(obj));
+  }
+  // Slow properties + dictionary elements
+  delete obj.__proto__.c;
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a15b".split(""), props(obj));
+  }
+  // Slow properties on the receiver as well
+  delete obj.a;
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("3415b".split(""), props(obj));
+  }
+  delete obj[3];
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("415b".split(""), props(obj));
+  }
+})();
+
+(function forInShadowing() {
+  var obj = {a:true, 3:true, 4:true};
+  obj.__proto__ = {
+    c:true, b:true, x:true,
+    2:true, 1:true, 5:true, 9:true};
+  Object.defineProperty(obj, 'x', {value:true, enumerable:false, configurable:true});
+  Object.defineProperty(obj, '9', {value:true, enumerable:false, configurable:true});
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a125cb".split(""), props(obj));
+  }
+  // Fast properties + dictionary elements
+  delete obj.__proto__[2];
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a15cb".split(""), props(obj));
+  }
+  // Slow properties + dictionary elements
+  delete obj.__proto__.c;
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a15b".split(""), props(obj));
+  }
+  // Remove the shadowing properties
+  delete obj.x;
+  delete obj[9];
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34a159bx".split(""), props(obj));
+  }
+  // Slow properties on the receiver as well
+  delete obj.a;
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("34159bx".split(""), props(obj));
+  }
+  delete obj[3];
+  for (var i = 0; i < 3; i++) {
+    assertArrayEquals("4159bx".split(""), props(obj));
+  }
+})();
+
+(function forInCharCodes() {
+  var o = {};
+  var a = [];
+  for (var i = 0x0020; i < 0x01ff; i+=2) {
+    var s = 'char:' + String.fromCharCode(i);
+    a.push(s);
+    o[s] = i;
+  }
+  assertArrayEquals(a, props(o), "charcodes");
+})();
+
+(function forInArray() {
+  var a = [];
+  assertEquals(0, props(a).length, "proplen0");
+  a[Math.pow(2,30)-1] = 0;
+  assertEquals(1, props(a).length, "proplen1");
+  a[Math.pow(2,31)-1] = 0;
+  assertEquals(2, props(a).length, "proplen2");
+  a[1] = 0;
+  assertEquals(3, props(a).length, "proplen3");
+})();
+
+(function forInInitialize() {
+  for (var hest = 'hest' in {}) { }
+  assertEquals('hest', hest, "empty-no-override");
+})();
+
+(function forInObjects() {
+  var result = '';
+  for (var p in {a : [0], b : 1}) { result += p; }
+  assertEquals('ab', result, "ab");
+
+  var result = '';
+  for (var p in {a : {v:1}, b : 1}) { result += p; }
+  assertEquals('ab', result, "ab-nodeep");
+
+  var result = '';
+  for (var p in { get a() {}, b : 1}) { result += p; }
+  assertEquals('ab', result, "abget");
+
+  var result = '';
+  for (var p in { get a() {}, set a(x) {}, b : 1}) { result += p; }
+  assertEquals('ab', result, "abgetset");
+})();
 
 
 // Test that for-in in the global scope works with a keyed property as "each".