Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(418)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: src/keys.cc

Issue 2014523002: Reland of [keys] Simplify KeyAccumulator (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: fixing wrong handle dereferencing Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/keys.h ('k') | src/objects.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/keys.cc
diff --git a/src/keys.cc b/src/keys.cc
index 17270eb1bede8711201731f3010c02b16a2cb2ed..bacb201b20d44c79513ea7cf5831e4b71ff847af 100644
--- a/src/keys.cc
+++ b/src/keys.cc
@@ -17,9 +17,6 @@ namespace v8 {
namespace internal {
KeyAccumulator::~KeyAccumulator() {
- for (size_t i = 0; i < elements_.size(); i++) {
- delete elements_[i];
- }
}
namespace {
@@ -37,11 +34,12 @@ static bool ContainsOnlyValidKeys(Handle<FixedArray> array) {
MaybeHandle<FixedArray> KeyAccumulator::GetKeys(
Handle<JSReceiver> object, KeyCollectionType type, PropertyFilter filter,
- GetKeysConversion keys_conversion, bool filter_proxy_keys) {
+ GetKeysConversion keys_conversion, bool filter_proxy_keys, bool is_for_in) {
USE(ContainsOnlyValidKeys);
Isolate* isolate = object->GetIsolate();
KeyAccumulator accumulator(isolate, type, filter);
accumulator.set_filter_proxy_keys(filter_proxy_keys);
+ accumulator.set_is_for_in(is_for_in);
MAYBE_RETURN(accumulator.CollectKeys(object, object),
MaybeHandle<FixedArray>());
Handle<FixedArray> keys = accumulator.GetKeys(keys_conversion);
@@ -50,181 +48,41 @@ MaybeHandle<FixedArray> KeyAccumulator::GetKeys(
}
Handle<FixedArray> KeyAccumulator::GetKeys(GetKeysConversion convert) {
- if (length_ == 0) {
+ if (keys_.is_null()) {
return isolate_->factory()->empty_fixed_array();
}
- // Make sure we have all the lengths collected.
- NextPrototype();
-
- if (type_ == OWN_ONLY && !ownProxyKeys_.is_null()) {
- return ownProxyKeys_;
- }
- // Assemble the result array by first adding the element keys and then the
- // property keys. We use the total number of String + Symbol keys per level in
- // |level_lengths_| and the available element keys in the corresponding bucket
- // in |elements_| to deduce the number of keys to take from the
- // |string_properties_| and |symbol_properties_| set.
- Handle<FixedArray> result = isolate_->factory()->NewFixedArray(length_);
- int insertion_index = 0;
- int string_properties_index = 0;
- int symbol_properties_index = 0;
- // String and Symbol lengths always come in pairs:
- size_t max_level = level_lengths_.size() / 2;
- for (size_t level = 0; level < max_level; level++) {
- int num_string_properties = level_lengths_[level * 2];
- int num_symbol_properties = level_lengths_[level * 2 + 1];
- int num_elements = 0;
- if (num_string_properties < 0) {
- // If the |num_string_properties| is negative, the current level contains
- // properties from a proxy, hence we skip the integer keys in |elements_|
- // since proxies define the complete ordering.
- num_string_properties = -num_string_properties;
- } else if (level < elements_.size()) {
- // Add the element indices for this prototype level.
- std::vector<uint32_t>* elements = elements_[level];
- num_elements = static_cast<int>(elements->size());
- for (int i = 0; i < num_elements; i++) {
- Handle<Object> key;
- if (convert == KEEP_NUMBERS) {
- key = isolate_->factory()->NewNumberFromUint(elements->at(i));
- } else {
- key = isolate_->factory()->Uint32ToString(elements->at(i));
- }
- result->set(insertion_index, *key);
- insertion_index++;
- }
- }
- // Add the string property keys for this prototype level.
- for (int i = 0; i < num_string_properties; i++) {
- Object* key = string_properties_->KeyAt(string_properties_index);
- result->set(insertion_index, key);
- insertion_index++;
- string_properties_index++;
- }
- // Add the symbol property keys for this prototype level.
- for (int i = 0; i < num_symbol_properties; i++) {
- Object* key = symbol_properties_->KeyAt(symbol_properties_index);
- result->set(insertion_index, key);
- insertion_index++;
- symbol_properties_index++;
- }
- if (FLAG_trace_for_in_enumerate) {
- PrintF("| strings=%d symbols=%d elements=%i ", num_string_properties,
- num_symbol_properties, num_elements);
- }
+ if (type_ == OWN_ONLY &&
+ keys_->map() == isolate_->heap()->fixed_array_map()) {
+ return Handle<FixedArray>::cast(keys_);
}
- if (FLAG_trace_for_in_enumerate) {
- PrintF("|| prototypes=%zu ||\n", max_level);
- }
-
- DCHECK_EQ(insertion_index, length_);
- return result;
+ return OrderedHashSet::ConvertToKeysArray(keys(), convert);
}
-namespace {
-
-bool AccumulatorHasKey(std::vector<uint32_t>* sub_elements, uint32_t key) {
- return std::binary_search(sub_elements->begin(), sub_elements->end(), key);
-}
-
-} // namespace
-
-bool KeyAccumulator::AddKey(Object* key, AddKeyConversion convert) {
- return AddKey(handle(key, isolate_), convert);
+void KeyAccumulator::AddKey(Object* key, AddKeyConversion convert) {
+ AddKey(handle(key, isolate_), convert);
}
-bool KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
+void KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
if (key->IsSymbol()) {
- if (filter_ & SKIP_SYMBOLS) return false;
- if (Handle<Symbol>::cast(key)->is_private()) return false;
- return AddSymbolKey(key);
- }
- if (filter_ & SKIP_STRINGS) return false;
- // Make sure we do not add keys to a proxy-level (see AddKeysFromJSProxy).
- DCHECK_LE(0, level_string_length_);
- // In some cases (e.g. proxies) we might get in String-converted ints which
- // should be added to the elements list instead of the properties. For
- // proxies we have to convert as well but also respect the original order.
- // Therefore we add a converted key to both sides
- if (convert == CONVERT_TO_ARRAY_INDEX || convert == PROXY_MAGIC) {
- uint32_t index = 0;
- int prev_length = length_;
- int prev_proto = level_string_length_;
- if ((key->IsString() && Handle<String>::cast(key)->AsArrayIndex(&index)) ||
- key->ToArrayIndex(&index)) {
- bool key_was_added = AddIntegerKey(index);
- if (convert == CONVERT_TO_ARRAY_INDEX) return key_was_added;
- if (convert == PROXY_MAGIC) {
- // If we had an array index (number) and it wasn't added, the key
- // already existed before, hence we cannot add it to the properties
- // keys as it would lead to duplicate entries.
- if (!key_was_added) {
- return false;
- }
- length_ = prev_length;
- level_string_length_ = prev_proto;
- }
- }
- }
- return AddStringKey(key, convert);
-}
-
-bool KeyAccumulator::AddKey(uint32_t key) { return AddIntegerKey(key); }
-
-bool KeyAccumulator::AddIntegerKey(uint32_t key) {
- // Make sure we do not add keys to a proxy-level (see AddKeysFromJSProxy).
- // We mark proxy-levels with a negative length
- DCHECK_LE(0, level_string_length_);
- // Binary search over all but the last level. The last one might not be
- // sorted yet.
- for (size_t i = 1; i < elements_.size(); i++) {
- if (AccumulatorHasKey(elements_[i - 1], key)) return false;
- }
- elements_.back()->push_back(key);
- length_++;
- return true;
-}
-
-bool KeyAccumulator::AddStringKey(Handle<Object> key,
- AddKeyConversion convert) {
- if (string_properties_.is_null()) {
- string_properties_ = OrderedHashSet::Allocate(isolate_, 16);
- }
- // TODO(cbruni): remove this conversion once we throw the correct TypeError
- // for non-string/symbol elements returned by proxies
- if (convert == PROXY_MAGIC && key->IsNumber()) {
- key = isolate_->factory()->NumberToString(key);
- }
- int prev_size = string_properties_->NumberOfElements();
- string_properties_ = OrderedHashSet::Add(string_properties_, key);
- if (prev_size < string_properties_->NumberOfElements()) {
- length_++;
- level_string_length_++;
- return true;
- } else {
- return false;
+ if (filter_ & SKIP_SYMBOLS) return;
+ if (Handle<Symbol>::cast(key)->is_private()) return;
+ } else if (filter_ & SKIP_STRINGS) {
+ return;
}
-}
-
-bool KeyAccumulator::AddSymbolKey(Handle<Object> key) {
- if (symbol_properties_.is_null()) {
- symbol_properties_ = OrderedHashSet::Allocate(isolate_, 16);
+ if (keys_.is_null()) {
+ keys_ = OrderedHashSet::Allocate(isolate_, 16);
}
- int prev_size = symbol_properties_->NumberOfElements();
- symbol_properties_ = OrderedHashSet::Add(symbol_properties_, key);
- if (prev_size < symbol_properties_->NumberOfElements()) {
- length_++;
- level_symbol_length_++;
- return true;
- } else {
- return false;
+ uint32_t index;
+ if (convert == CONVERT_TO_ARRAY_INDEX && key->IsString() &&
+ Handle<String>::cast(key)->AsArrayIndex(&index)) {
+ key = isolate_->factory()->NewNumberFromUint(index);
}
+ keys_ = OrderedHashSet::Add(keys(), key);
}
void KeyAccumulator::AddKeys(Handle<FixedArray> array,
AddKeyConversion convert) {
int add_length = array->length();
- if (add_length == 0) return;
for (int i = 0; i < add_length; i++) {
Handle<Object> current(array->get(i), isolate_);
AddKey(current, convert);
@@ -271,65 +129,20 @@ MaybeHandle<FixedArray> FilterProxyKeys(Isolate* isolate, Handle<JSProxy> owner,
Maybe<bool> KeyAccumulator::AddKeysFromJSProxy(Handle<JSProxy> proxy,
Handle<FixedArray> keys) {
if (filter_proxy_keys_) {
+ DCHECK(!is_for_in_);
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate_, keys, FilterProxyKeys(isolate_, proxy, keys, filter_),
Nothing<bool>());
}
- // Proxies define a complete list of keys with no distinction of
- // elements and properties, which breaks the normal assumption for the
- // KeyAccumulator.
- if (type_ == OWN_ONLY) {
- ownProxyKeys_ = keys;
- level_string_length_ = keys->length();
- length_ = level_string_length_;
- } else {
- AddKeys(keys, PROXY_MAGIC);
+ if (type_ == OWN_ONLY && !is_for_in_) {
+ // If we collect only the keys from a JSProxy do not sort or deduplicate it.
+ keys_ = keys;
+ return Just(true);
}
- // Invert the current length to indicate a present proxy, so we can ignore
- // element keys for this level. Otherwise we would not fully respect the order
- // given by the proxy.
- level_string_length_ = -level_string_length_;
+ AddKeys(keys, is_for_in_ ? CONVERT_TO_ARRAY_INDEX : DO_NOT_CONVERT);
return Just(true);
}
-void KeyAccumulator::AddElementKeysFromInterceptor(
- Handle<JSObject> array_like) {
- AddKeys(array_like, CONVERT_TO_ARRAY_INDEX);
- // The interceptor might introduce duplicates for the current level, since
- // these keys get added after the objects's normal element keys.
- SortCurrentElementsListRemoveDuplicates();
-}
-
-void KeyAccumulator::SortCurrentElementsListRemoveDuplicates() {
- // Sort and remove duplicates from the current elements level and adjust.
- // the lengths accordingly.
- auto last_level = elements_.back();
- size_t nof_removed_keys = last_level->size();
- std::sort(last_level->begin(), last_level->end());
- last_level->erase(std::unique(last_level->begin(), last_level->end()),
- last_level->end());
- // Adjust total length by the number of removed duplicates.
- nof_removed_keys -= last_level->size();
- length_ -= static_cast<int>(nof_removed_keys);
-}
-
-void KeyAccumulator::SortCurrentElementsList() {
- if (elements_.empty()) return;
- auto element_keys = elements_.back();
- std::sort(element_keys->begin(), element_keys->end());
-}
-
-void KeyAccumulator::NextPrototype() {
- // Store the protoLength on the first call of this method.
- if (!elements_.empty()) {
- level_lengths_.push_back(level_string_length_);
- level_lengths_.push_back(level_symbol_length_);
- }
- elements_.push_back(new std::vector<uint32_t>());
- level_string_length_ = 0;
- level_symbol_length_ = 0;
-}
-
Maybe<bool> KeyAccumulator::CollectKeys(Handle<JSReceiver> receiver,
Handle<JSReceiver> object) {
// Proxies have no hidden prototype and we should not trigger the
@@ -595,17 +408,19 @@ MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysFast(
MaybeHandle<FixedArray> FastKeyAccumulator::GetKeysSlow(
GetKeysConversion convert) {
- return JSReceiver::GetKeys(receiver_, type_, filter_, KEEP_NUMBERS,
- filter_proxy_keys_);
+ return KeyAccumulator::GetKeys(receiver_, type_, filter_, KEEP_NUMBERS,
+ filter_proxy_keys_, is_for_in_);
}
+namespace {
+
enum IndexedOrNamed { kIndexed, kNamed };
// Returns |true| on success, |nothing| on exception.
template <class Callback, IndexedOrNamed type>
-static Maybe<bool> GetKeysFromInterceptor(Handle<JSReceiver> receiver,
- Handle<JSObject> object,
- KeyAccumulator* accumulator) {
+Maybe<bool> GetKeysFromInterceptor(Handle<JSReceiver> receiver,
+ Handle<JSObject> object,
+ KeyAccumulator* accumulator) {
Isolate* isolate = accumulator->isolate();
if (type == kIndexed) {
if (!object->HasIndexedInterceptor()) return Just(true);
@@ -632,54 +447,90 @@ static Maybe<bool> GetKeysFromInterceptor(Handle<JSReceiver> receiver,
}
RETURN_VALUE_IF_SCHEDULED_EXCEPTION(isolate, Nothing<bool>());
if (result.is_null()) return Just(true);
- DCHECK(result->IsJSArray() || result->HasSloppyArgumentsElements());
- // The accumulator takes care of string/symbol filtering.
- if (type == kIndexed) {
- accumulator->AddElementKeysFromInterceptor(result);
- } else {
- accumulator->AddKeys(result, DO_NOT_CONVERT);
- }
+ accumulator->AddKeys(
+ result, type == kIndexed ? CONVERT_TO_ARRAY_INDEX : DO_NOT_CONVERT);
return Just(true);
}
-void KeyAccumulator::CollectOwnElementIndices(Handle<JSObject> object) {
- if (filter_ & SKIP_STRINGS) return;
+} // namespace
+
+Maybe<bool> KeyAccumulator::CollectOwnElementIndices(
+ Handle<JSReceiver> receiver, Handle<JSObject> object) {
+ if (filter_ & SKIP_STRINGS || skip_indices_) return Just(true);
+
ElementsAccessor* accessor = object->GetElementsAccessor();
accessor->CollectElementIndices(object, this);
+
+ return GetKeysFromInterceptor<v8::IndexedPropertyEnumeratorCallback,
+ kIndexed>(receiver, object, this);
}
-void KeyAccumulator::CollectOwnPropertyNames(Handle<JSObject> object) {
- if (object->HasFastProperties()) {
- int real_size = object->map()->NumberOfOwnDescriptors();
- Handle<DescriptorArray> descs(object->map()->instance_descriptors(),
- isolate_);
- for (int i = 0; i < real_size; i++) {
- PropertyDetails details = descs->GetDetails(i);
- if ((details.attributes() & filter_) != 0) 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 (key->FilterKey(filter_)) continue;
- AddKey(key, DO_NOT_CONVERT);
+namespace {
+
+template <bool skip_symbols>
+int CollectOwnPropertyNamesInternal(Handle<JSObject> object,
+ KeyAccumulator* keys,
+ Handle<DescriptorArray> descs,
+ int start_index, int limit) {
+ int first_skipped = -1;
+ for (int i = start_index; i < limit; i++) {
+ PropertyDetails details = descs->GetDetails(i);
+ if ((details.attributes() & keys->filter()) != 0) continue;
+ if (keys->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;
}
- } else if (object->IsJSGlobalObject()) {
- GlobalDictionary::CollectKeysTo(
- handle(object->global_dictionary(), isolate_), this, filter_);
+ 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);
+ }
+ return first_skipped;
+}
+
+} // namespace
+
+Maybe<bool> KeyAccumulator::CollectOwnPropertyNames(Handle<JSReceiver> receiver,
+ Handle<JSObject> object) {
+ if (filter_ == ENUMERABLE_STRINGS) {
+ Handle<FixedArray> enum_keys =
+ KeyAccumulator::GetEnumPropertyKeys(isolate_, object);
+ AddKeys(enum_keys, DO_NOT_CONVERT);
} else {
- NameDictionary::CollectKeysTo(
- handle(object->property_dictionary(), isolate_), this, filter_);
+ if (object->HasFastProperties()) {
+ int limit = object->map()->NumberOfOwnDescriptors();
+ Handle<DescriptorArray> descs(object->map()->instance_descriptors(),
+ isolate_);
+ // First collect the strings,
+ int first_symbol =
+ CollectOwnPropertyNamesInternal<true>(object, this, descs, 0, limit);
+ // then the symbols.
+ if (first_symbol != -1) {
+ CollectOwnPropertyNamesInternal<false>(object, this, descs,
+ first_symbol, limit);
+ }
+ } else if (object->IsJSGlobalObject()) {
+ GlobalDictionary::CollectKeysTo(
+ handle(object->global_dictionary(), isolate_), this, filter_);
+ } else {
+ NameDictionary::CollectKeysTo(
+ handle(object->property_dictionary(), isolate_), this, filter_);
+ }
}
+ // Add the property keys from the interceptor.
+ return GetKeysFromInterceptor<v8::GenericNamedPropertyEnumeratorCallback,
+ kNamed>(receiver, object, this);
}
// Returns |true| on success, |false| if prototype walking should be stopped,
// |nothing| if an exception was thrown.
Maybe<bool> KeyAccumulator::CollectOwnKeys(Handle<JSReceiver> receiver,
Handle<JSObject> object) {
- NextPrototype();
// Check access rights if required.
if (object->IsAccessCheckNeeded() &&
!isolate_->MayAccess(handle(isolate_->context()), object)) {
@@ -692,27 +543,8 @@ Maybe<bool> KeyAccumulator::CollectOwnKeys(Handle<JSReceiver> receiver,
DCHECK_EQ(OWN_ONLY, type_);
filter_ = static_cast<PropertyFilter>(filter_ | ONLY_ALL_CAN_READ);
}
-
- CollectOwnElementIndices(object);
-
- // Add the element keys from the interceptor.
- Maybe<bool> success =
- GetKeysFromInterceptor<v8::IndexedPropertyEnumeratorCallback, kIndexed>(
- receiver, object, this);
- MAYBE_RETURN(success, Nothing<bool>());
-
- if (filter_ == ENUMERABLE_STRINGS) {
- Handle<FixedArray> enum_keys =
- KeyAccumulator::GetEnumPropertyKeys(isolate_, object);
- AddKeys(enum_keys, DO_NOT_CONVERT);
- } else {
- CollectOwnPropertyNames(object);
- }
-
- // Add the property keys from the interceptor.
- success = GetKeysFromInterceptor<v8::GenericNamedPropertyEnumeratorCallback,
- kNamed>(receiver, object, this);
- MAYBE_RETURN(success, Nothing<bool>());
+ MAYBE_RETURN(CollectOwnElementIndices(receiver, object), Nothing<bool>());
+ MAYBE_RETURN(CollectOwnPropertyNames(receiver, object), Nothing<bool>());
return Just(true);
}
@@ -822,7 +654,6 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
// (No-op, just keep it in |target_keys|.)
}
}
- NextPrototype(); // Prepare for accumulating keys.
// 15. If extensibleTarget is true and targetNonconfigurableKeys is empty,
// then:
if (extensible_target && nonconfigurable_keys_length == 0) {
@@ -896,7 +727,6 @@ Maybe<bool> KeyAccumulator::CollectOwnJSProxyTargetKeys(
Handle<FixedArray> keys;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate_, keys, JSReceiver::OwnPropertyKeys(target), Nothing<bool>());
- NextPrototype(); // Prepare for accumulating keys.
bool prev_filter_proxy_keys_ = filter_proxy_keys_;
filter_proxy_keys_ = false;
Maybe<bool> result = AddKeysFromJSProxy(proxy, keys);
« no previous file with comments | « src/keys.h ('k') | src/objects.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698