diff --git a/Libraries/LibJS/Bytecode/Interpreter.cpp b/Libraries/LibJS/Bytecode/Interpreter.cpp index 90edbe4a3a6..c8d9e01ff80 100644 --- a/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -37,30 +37,6 @@ #include #include -namespace JS { - -struct PropertyKeyAndEnumerableFlag { - JS::PropertyKey key; - bool enumerable { false }; -}; - -} - -namespace AK { -template<> -struct Traits : public DefaultTraits { - static unsigned hash(JS::PropertyKeyAndEnumerableFlag const& entry) - { - return Traits::hash(entry.key); - } - - static bool equals(JS::PropertyKeyAndEnumerableFlag const& a, JS::PropertyKeyAndEnumerableFlag const& b) - { - return Traits::equals(a.key, b.key); - } -}; -} - namespace JS::Bytecode { bool g_dump_bytecode = false; @@ -1718,6 +1694,16 @@ inline ThrowCompletionOr delete_by_value_with_this(Bytecode::Interpreter& return Value(TRY(reference.delete_(vm))); } +class PropertyNameIteratorState final : public GC::Cell { + GC_CELL(PropertyNameIteratorState, GC::Cell); + GC_DECLARE_ALLOCATOR(PropertyNameIteratorState); + +public: + HashTable non_enumerable_property_keys; +}; + +GC_DEFINE_ALLOCATOR(PropertyNameIteratorState); + // 14.7.5.9 EnumerateObjectProperties ( O ), https://tc39.es/ecma262/#sec-enumerate-object-properties inline ThrowCompletionOr get_object_property_iterator(VM& vm, Value value) { @@ -1738,60 +1724,74 @@ inline ThrowCompletionOr get_object_property_iterator(VM& vm, Value val // Note: While the spec doesn't explicitly require these to be ordered, it says that the values should be retrieved via OwnPropertyKeys, // so we just keep the order consistent anyway. - OrderedHashTable properties; + struct ObjectWithProperties { + WeakPtr object; + OrderedHashTable property_names; + }; + Vector objects_with_properties; + HashTable> seen_objects; + HashTable seen_property_keys; // Collect all keys immediately (invariant no. 5) for (auto object_to_check = GC::Ptr { object.ptr() }; object_to_check && !seen_objects.contains(*object_to_check); object_to_check = TRY(object_to_check->internal_get_prototype_of())) { seen_objects.set(*object_to_check); + objects_with_properties.prepend(ObjectWithProperties { + .object = *object_to_check, + .property_names = {}, + }); + auto& properties = objects_with_properties.first().property_names; for (auto& key : TRY(object_to_check->internal_own_property_keys())) { if (key.is_symbol()) continue; - // NOTE: If there is a non-enumerable property higher up the prototype chain with the same key, - // we mustn't include this property even if it's enumerable (invariant no. 5 and 6) - // This is achieved with the PropertyKeyAndEnumerableFlag struct, which doesn't consider - // the enumerable flag when comparing keys. - PropertyKeyAndEnumerableFlag new_entry { - .key = TRY(PropertyKey::from_value(vm, key)), - .enumerable = false, - }; + auto property_key = TRY(PropertyKey::from_value(vm, key)); - if (properties.contains(new_entry)) + if (seen_property_keys.set(property_key, AK::HashSetExistingEntryBehavior::Keep) == HashSetResult::KeptExistingEntry) continue; - auto descriptor = TRY(object_to_check->internal_get_own_property(new_entry.key)); - if (!descriptor.has_value()) - continue; - - new_entry.enumerable = *descriptor->enumerable; - properties.set(move(new_entry), AK::HashSetExistingEntryBehavior::Keep); + properties.set(property_key); } } - properties.remove_all_matching([&](auto& entry) { return !entry.enumerable; }); - auto& realm = *vm.current_realm(); auto result_object = Object::create_with_premade_shape(realm.intrinsics().iterator_result_object_shape()); auto value_offset = realm.intrinsics().iterator_result_object_value_offset(); auto done_offset = realm.intrinsics().iterator_result_object_done_offset(); + auto state = realm.heap().allocate(); + auto callback = NativeFunction::create( - *vm.current_realm(), [items = move(properties), result_object, value_offset, done_offset](VM& vm) mutable -> ThrowCompletionOr { - auto& iterated_object = vm.this_value().as_object(); + *vm.current_realm(), [objects_with_properties = move(objects_with_properties), result_object, value_offset, done_offset, state](VM& vm) mutable -> ThrowCompletionOr { while (true) { - if (items.is_empty()) { + if (objects_with_properties.is_empty()) { result_object->put_direct(done_offset, JS::Value(true)); return result_object; } - auto key = move(items.take_first().key); + auto& object = objects_with_properties.last(); - // If the property is deleted, don't include it (invariant no. 2) - if (!TRY(iterated_object.has_property(key))) + if (object.property_names.is_empty() + || !object.object) { + objects_with_properties.take_last(); + continue; + } + + auto key = object.property_names.take_first(); + + // NOTE: If there is a non-enumerable property higher up the prototype chain with the same key, + // we mustn't include this property even if it's enumerable (invariant no. 5 and 6) + if (state->non_enumerable_property_keys.contains(key)) continue; - result_object->put_direct(done_offset, JS::Value(false)); + auto descriptor = TRY(object.object->internal_get_own_property(key)); + if (!descriptor.has_value()) + continue; + + if (!*descriptor->enumerable) { + state->non_enumerable_property_keys.set(move(key)); + continue; + } if (key.is_number()) result_object->put_direct(value_offset, PrimitiveString::create(vm, String::number(key.as_number()))); @@ -1800,6 +1800,7 @@ inline ThrowCompletionOr get_object_property_iterator(VM& vm, Value val else VERIFY_NOT_REACHED(); // We should not have non-string/number keys. + result_object->put_direct(done_offset, JS::Value(false)); return result_object; } }, diff --git a/Libraries/LibJS/Tests/builtins/Proxy/for-in-iteration-traps.js b/Libraries/LibJS/Tests/builtins/Proxy/for-in-iteration-traps.js new file mode 100644 index 00000000000..81d57824d09 --- /dev/null +++ b/Libraries/LibJS/Tests/builtins/Proxy/for-in-iteration-traps.js @@ -0,0 +1,40 @@ +test("for..in iteration Proxy traps", () => { + let traps = []; + let array = [1, 2, 3]; + let from = new Proxy(array, { + getPrototypeOf: function (t) { + traps.push("getPrototypeOf"); + return Reflect.getPrototypeOf(t); + }, + ownKeys: function (t) { + traps.push("ownKeys"); + return Reflect.ownKeys(t); + }, + has: function (t, p) { + traps.push("has"); + return Reflect.has(t, p); + }, + getOwnPropertyDescriptor: function (t, p) { + traps.push("getOwnPropertyDescriptor"); + return Reflect.getOwnPropertyDescriptor(t, p); + }, + }); + const to = []; + for (const prop in from) { + to.push(prop); + from.pop(); + } + + expect(to).toEqual(["0", "1"]); + + expect(traps).toEqual([ + "ownKeys", + "getPrototypeOf", + "getOwnPropertyDescriptor", + "getOwnPropertyDescriptor", + "getOwnPropertyDescriptor", + "getOwnPropertyDescriptor", + "getOwnPropertyDescriptor", + "getOwnPropertyDescriptor", + ]); +});