LibJS: Directly indicate when the IteratorHelper generator is complete

Our existing implementation was checking if the generator step result
was `undefined` to indicate that the generator is complete. This is
not sufficient for an upcoming implementation of Iterator.concat, where
`undefined` is used as an incomplete iteration result.

Instead, let's have the Iterator prototype return an IterationResult to
explicitly indicate its completeness. As a result, the prototype also
looks more like the spec now.
This commit is contained in:
Timothy Flynn
2025-12-02 16:53:09 -05:00
committed by Andreas Kling
parent 6976ff389e
commit 7ee415a7c2
Notes: github-actions[bot] 2025-12-03 11:09:53 +00:00
3 changed files with 51 additions and 81 deletions

View File

@@ -34,29 +34,17 @@ void IteratorHelper::visit_edges(Visitor& visitor)
visitor.visit(m_abrupt_closure);
}
Value IteratorHelper::result(Value value)
{
set_generator_state(value.is_undefined() ? GeneratorState::Completed : GeneratorState::SuspendedYield);
return value;
}
ThrowCompletionOr<Value> IteratorHelper::close_result(VM& vm, Completion completion)
{
set_generator_state(GeneratorState::Completed);
return TRY(iterator_close_all(vm, underlying_iterators(), completion));
}
ThrowCompletionOr<GeneratorObject::IterationResult> IteratorHelper::execute(VM& vm, JS::Completion const& completion)
ThrowCompletionOr<IteratorHelper::IterationResult> IteratorHelper::execute(VM& vm, JS::Completion const& completion)
{
ScopeGuard guard { [&] { vm.pop_execution_context(); } };
if (completion.is_abrupt()) {
if (m_abrupt_closure) {
auto abrupt_result = TRY(m_abrupt_closure->function()(vm, *this, completion));
return IterationResult(abrupt_result, true);
}
auto close_result = TRY(this->close_result(vm, completion));
return IterationResult(close_result, true);
auto abrupt_result = m_abrupt_closure
? TRY(m_abrupt_closure->function()(vm, completion))
: TRY(iterator_close_all(vm, underlying_iterators(), completion));
set_generator_state(GeneratorState::Completed);
return IterationResult(abrupt_result, true);
}
auto result_value = m_closure->function()(vm, *this);
@@ -66,7 +54,10 @@ ThrowCompletionOr<GeneratorObject::IterationResult> IteratorHelper::execute(VM&
return result_value.throw_completion();
}
return IterationResult(result(result_value.release_value()), generator_state() == GeneratorState::Completed);
auto result = result_value.release_value();
set_generator_state(result.done ? GeneratorState::Completed : GeneratorState::SuspendedYield);
return result;
}
}

View File

@@ -19,8 +19,8 @@ class IteratorHelper final : public GeneratorObject {
GC_DECLARE_ALLOCATOR(IteratorHelper);
public:
using Closure = GC::Function<ThrowCompletionOr<Value>(VM&, IteratorHelper&)>;
using AbruptClosure = GC::Function<ThrowCompletionOr<Value>(VM&, IteratorHelper&, Completion const&)>;
using Closure = GC::Function<ThrowCompletionOr<IterationResult>(VM&, IteratorHelper&)>;
using AbruptClosure = GC::Function<ThrowCompletionOr<Value>(VM&, Completion const&)>;
static GC::Ref<IteratorHelper> create(Realm&, ReadonlySpan<GC::Ref<IteratorRecord>>, GC::Ref<Closure>, GC::Ptr<AbruptClosure> = {});
@@ -29,9 +29,6 @@ public:
size_t counter() const { return m_counter; }
void increment_counter() { ++m_counter; }
Value result(Value);
ThrowCompletionOr<Value> close_result(VM&, Completion);
private:
IteratorHelper(Realm&, Object& prototype, ReadonlySpan<GC::Ref<IteratorRecord>>, GC::Ref<Closure>, GC::Ptr<AbruptClosure>);

View File

@@ -117,7 +117,7 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::drop)
// 10. Let closure be a new Abstract Closure with no parameters that captures iterated and integerLimit and performs
// the following steps when called:
auto closure = GC::create_function(realm.heap(), [iterated, integer_limit](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<Value> {
auto closure = GC::create_function(realm.heap(), [iterated, integer_limit](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<IteratorHelper::IterationResult> {
// a. Let remaining be integerLimit.
// b. Repeat, while remaining > 0,
while (iterator.counter() < integer_limit) {
@@ -130,7 +130,7 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::drop)
// iii. If next is DONE, return ReturnCompletion(undefined).
if (next.has<IterationDone>())
return iterator.result(js_undefined());
return IteratorHelper::IterationResult { js_undefined(), true };
}
// c. Repeat,
@@ -140,11 +140,11 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::drop)
// ii. If value is DONE, return ReturnCompletion(undefined).
if (!value.has_value())
return iterator.result(js_undefined());
return IteratorHelper::IterationResult { js_undefined(), true };
// iii. Let completion be Completion(Yield(value)).
// iv. IfAbruptCloseIterator(completion, iterated).
return iterator.result(*value);
return IteratorHelper::IterationResult { *value, false };
});
// 11. Let result be CreateIteratorFromClosure(closure, "Iterator Helper", %IteratorHelperPrototype%, « [[UnderlyingIterators]] »).
@@ -231,7 +231,7 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::filter)
// 6. Let closure be a new Abstract Closure with no parameters that captures iterated and predicate and performs the
// following steps when called:
auto closure = GC::create_function(realm.heap(), [iterated, predicate = GC::Ref { predicate.as_function() }](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<Value> {
auto closure = GC::create_function(realm.heap(), [iterated, predicate = GC::Ref { predicate.as_function() }](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<IteratorHelper::IterationResult> {
// a. Let counter be 0.
// b. Repeat,
while (true) {
@@ -240,24 +240,21 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::filter)
// ii. If value is DONE, return ReturnCompletion(undefined).
if (!value.has_value())
return iterator.result(js_undefined());
return IteratorHelper::IterationResult { js_undefined(), true };
// iii. Let selected be Completion(Call(predicate, undefined, « value, 𝔽(counter) »)).
auto selected = call(vm, *predicate, js_undefined(), *value, Value { iterator.counter() });
// iv. IfAbruptCloseIterator(selected, iterated).
if (selected.is_error())
return iterator.close_result(vm, selected.release_error());
auto selected = TRY_OR_CLOSE_ITERATOR(vm, iterated, call(vm, *predicate, js_undefined(), *value, Value { iterator.counter() }));
// vi. Set counter to counter + 1.
// NOTE: We do this step early to ensure it occurs before returning.
iterator.increment_counter();
// v. If ToBoolean(selected) is true, then
if (selected.value().to_boolean()) {
if (selected.to_boolean()) {
// 1. Let completion be Completion(Yield(value)).
// 2. IfAbruptCloseIterator(completion, iterated).
return iterator.result(*value);
return IteratorHelper::IterationResult { *value, false };
}
}
});
@@ -323,7 +320,7 @@ class FlatMapIterator : public Cell {
GC_DECLARE_ALLOCATOR(FlatMapIterator);
public:
ThrowCompletionOr<Value> next(VM& vm, IteratorRecord& iterated, IteratorHelper& iterator, FunctionObject& mapper)
ThrowCompletionOr<IteratorHelper::IterationResult> next(VM& vm, IteratorRecord& iterated, IteratorHelper& iterator, FunctionObject& mapper)
{
if (m_inner_iterator)
return next_inner_iterator(vm, iterated, iterator, mapper);
@@ -331,21 +328,17 @@ public:
}
// NOTE: This implements step 6.b.vii.4.b of Iterator.prototype.flatMap.
ThrowCompletionOr<Value> on_abrupt_completion(VM& vm, IteratorHelper& iterator, Completion const& completion)
ThrowCompletionOr<Value> on_abrupt_completion(VM& vm, IteratorRecord& iterated, Completion const& completion)
{
VERIFY(m_inner_iterator);
// b. If completion is an abrupt completion, then
// i. Let backupCompletion be Completion(IteratorClose(innerIterator, completion)).
// ii. IfAbruptCloseIterator(backupCompletion, iterated).
TRY_OR_CLOSE_ITERATOR(vm, iterated, iterator_close(vm, *m_inner_iterator, completion));
// i. Let backupCompletion be Completion(IteratorClose(innerIterator, completion)).
auto backup_completion = iterator_close(vm, *m_inner_iterator, completion);
// ii. IfAbruptCloseIterator(backupCompletion, iterated).
if (backup_completion.is_error())
return iterator.close_result(vm, backup_completion.release_error());
// iii. Return ? IteratorClose(completion, iterated).
return iterator.close_result(vm, completion);
// iii. Return ? IteratorClose(completion, iterated).
return TRY(iterator_close(vm, iterated, completion));
}
private:
@@ -357,31 +350,25 @@ private:
visitor.visit(m_inner_iterator);
}
ThrowCompletionOr<Value> next_outer_iterator(VM& vm, IteratorRecord& iterated, IteratorHelper& iterator, FunctionObject& mapper)
ThrowCompletionOr<IteratorHelper::IterationResult> next_outer_iterator(VM& vm, IteratorRecord& iterated, IteratorHelper& iterator, FunctionObject& mapper)
{
// i. Let value be ? IteratorStepValue(iterated).
auto value = TRY(iterator_step_value(vm, iterated));
// ii. If value is DONE, return undefined.
if (!value.has_value())
return iterator.result(js_undefined());
return IteratorHelper::IterationResult { js_undefined(), true };
// iii. Let mapped be Completion(Call(mapper, undefined, « value, 𝔽(counter) »)).
auto mapped = call(vm, mapper, js_undefined(), *value, Value { iterator.counter() });
// iv. IfAbruptCloseIterator(mapped, iterated).
if (mapped.is_error())
return iterator.close_result(vm, mapped.release_error());
auto mapped = TRY_OR_CLOSE_ITERATOR(vm, iterated, call(vm, mapper, js_undefined(), *value, Value { iterator.counter() }));
// v. Let innerIterator be Completion(GetIteratorFlattenable(mapped, reject-primitives)).
auto inner_iterator = get_iterator_flattenable(vm, mapped.release_value(), PrimitiveHandling::RejectPrimitives);
// vi. IfAbruptCloseIterator(innerIterator, iterated).
if (inner_iterator.is_error())
return iterator.close_result(vm, inner_iterator.release_error());
auto inner_iterator = TRY_OR_CLOSE_ITERATOR(vm, iterated, get_iterator_flattenable(vm, mapped, PrimitiveHandling::RejectPrimitives));
// vii. Let innerAlive be true.
m_inner_iterator = inner_iterator.release_value();
m_inner_iterator = inner_iterator;
// ix. Set counter to counter + 1.
// NOTE: We do this step early to ensure it occurs before returning.
@@ -391,19 +378,16 @@ private:
return next_inner_iterator(vm, iterated, iterator, mapper);
}
ThrowCompletionOr<Value> next_inner_iterator(VM& vm, IteratorRecord& iterated, IteratorHelper& iterator, FunctionObject& mapper)
ThrowCompletionOr<IteratorHelper::IterationResult> next_inner_iterator(VM& vm, IteratorRecord& iterated, IteratorHelper& iterator, FunctionObject& mapper)
{
VERIFY(m_inner_iterator);
// 1. Let innerValue be Completion(IteratorStepValue(innerIterator)).
auto inner_value = iterator_step_value(vm, *m_inner_iterator);
// 2. IfAbruptCloseIterator(innerValue, iterated).
if (inner_value.is_error())
return iterator.close_result(vm, inner_value.release_error());
auto inner_value = TRY_OR_CLOSE_ITERATOR(vm, iterated, iterator_step_value(vm, *m_inner_iterator));
// 3. If innerValue is DONE, then
if (!inner_value.value().has_value()) {
if (!inner_value.has_value()) {
// a. Set innerAlive to false.
m_inner_iterator = nullptr;
@@ -413,7 +397,7 @@ private:
else {
// a. Let completion be Completion(Yield(innerValue)).
// NOTE: Step b is implemented via on_abrupt_completion.
return *inner_value.release_value();
return IteratorHelper::IterationResult { *inner_value, false };
}
}
@@ -452,12 +436,12 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::flat_map)
// 7. Let closure be a new Abstract Closure with no parameters that captures iterated and mapper and performs the
// following steps when called:
auto closure = GC::create_function(realm.heap(), [iterated, flat_map_iterator, mapper = GC::Ref { mapper.as_function() }](VM& vm, IteratorHelper& iterator) mutable -> ThrowCompletionOr<Value> {
auto closure = GC::create_function(realm.heap(), [iterated, flat_map_iterator, mapper = GC::Ref { mapper.as_function() }](VM& vm, IteratorHelper& iterator) mutable -> ThrowCompletionOr<IteratorHelper::IterationResult> {
return flat_map_iterator->next(vm, iterated, iterator, *mapper);
});
auto abrupt_closure = GC::create_function(realm.heap(), [flat_map_iterator](VM& vm, IteratorHelper& iterator, Completion const& completion) -> ThrowCompletionOr<Value> {
return flat_map_iterator->on_abrupt_completion(vm, iterator, completion);
auto abrupt_closure = GC::create_function(realm.heap(), [iterated, flat_map_iterator](VM& vm, Completion const& completion) -> ThrowCompletionOr<Value> {
return flat_map_iterator->on_abrupt_completion(vm, iterated, completion);
});
// 8. Let result be CreateIteratorFromClosure(closure, "Iterator Helper", %IteratorHelperPrototype%, « [[UnderlyingIterators]] »).
@@ -540,7 +524,7 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::map)
// 6. Let closure be a new Abstract Closure with no parameters that captures iterated and mapper and performs the
// following steps when called:
auto closure = GC::create_function(realm.heap(), [iterated, mapper = GC::Ref { mapper.as_function() }](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<Value> {
auto closure = GC::create_function(realm.heap(), [iterated, mapper = GC::Ref { mapper.as_function() }](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<IteratorHelper::IterationResult> {
// a. Let counter be 0.
// b. Repeat,
@@ -549,14 +533,11 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::map)
// ii. If value is DONE, return undefined.
if (!value.has_value())
return iterator.result(js_undefined());
return IteratorHelper::IterationResult { js_undefined(), true };
// iii. Let mapped be Completion(Call(mapper, undefined, « value, 𝔽(counter) »)).
auto mapped = call(vm, *mapper, js_undefined(), *value, Value { iterator.counter() });
// iv. IfAbruptCloseIterator(mapped, iterated).
if (mapped.is_error())
return iterator.close_result(vm, mapped.release_error());
auto mapped = TRY_OR_CLOSE_ITERATOR(vm, iterated, call(vm, *mapper, js_undefined(), *value, Value { iterator.counter() }));
// vii. Set counter to counter + 1.
// NOTE: We do this step early to ensure it occurs before returning.
@@ -564,7 +545,7 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::map)
// v. Let completion be Completion(Yield(mapped)).
// vi. IfAbruptCloseIterator(completion, iterated).
return iterator.result(mapped.release_value());
return IteratorHelper::IterationResult { mapped, false };
});
// 7. Let result be CreateIteratorFromClosure(closure, "Iterator Helper", %IteratorHelperPrototype%, « [[UnderlyingIterators]] »).
@@ -745,14 +726,15 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::take)
// 10. Let closure be a new Abstract Closure with no parameters that captures iterated and integerLimit and performs
// the following steps when called:
auto closure = GC::create_function(realm.heap(), [iterated, integer_limit](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<Value> {
auto closure = GC::create_function(realm.heap(), [iterated, integer_limit](VM& vm, IteratorHelper& iterator) -> ThrowCompletionOr<IteratorHelper::IterationResult> {
// a. Let remaining be integerLimit.
// b. Repeat,
// i. If remaining = 0, then
if (iterator.counter() >= integer_limit) {
// 1. Return ? IteratorClose(iterated, NormalCompletion(undefined)).
return iterator.close_result(vm, normal_completion(js_undefined()));
auto close_result = TRY(iterator_close(vm, iterated, normal_completion(js_undefined())));
return IteratorHelper::IterationResult { close_result, true };
}
// ii. If remaining ≠ +∞, then
@@ -762,13 +744,13 @@ JS_DEFINE_NATIVE_FUNCTION(IteratorPrototype::take)
// iii. Let value be ? IteratorStepValue(iterated).
auto value = TRY(iterator_step_value(vm, iterated));
// iv. If value is DONE, return ReturnCompletion(undefined)..
// iv. If value is DONE, return ReturnCompletion(undefined).
if (!value.has_value())
return iterator.result(js_undefined());
return IteratorHelper::IterationResult { js_undefined(), true };
// v. Let completion be Completion(Yield(value)).
// vi. IfAbruptCloseIterator(completion, iterated).
return iterator.result(*value);
return IteratorHelper::IterationResult { *value, false };
});
// 11. Let result be CreateIteratorFromClosure(closure, "Iterator Helper", %IteratorHelperPrototype%, « [[UnderlyingIterators]] »).