Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Merged by Bors] - Align iterator loops to the spec #2686

Closed
wants to merge 7 commits into from
Closed

[Merged by Bors] - Align iterator loops to the spec #2686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7596678
Conform to the `AsyncIterator` API on `for await .. of` loops
jedel1043 Mar 14, 2023
ffb5528
Fix emerging bugs
jedel1043 Mar 14, 2023
fea4965
Restructure iterator ops
jedel1043 Mar 17, 2023
7948232
Apply review
jedel1043 Mar 18, 2023
2819830
Add test
jedel1043 Mar 20, 2023
7772f2c
Fix jump bug
jedel1043 Mar 21, 2023
766f809
Fix panics
jedel1043 Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion boa_engine/src/builtins/iterable/async_from_sync_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl AsyncFromSyncIterator {
// a. Let result be Completion(IteratorNext(syncIteratorRecord, value)).
// 6. Else,
// a. Let result be Completion(IteratorNext(syncIteratorRecord)).
let result = sync_iterator_record.next(args.get(0).cloned(), context);
let result = sync_iterator_record.next(args.get(0), context);

// 7. IfAbruptRejectPromise(result, promiseCapability).
if_abrupt_reject_promise!(result, promise_capability, context);
Expand Down
26 changes: 10 additions & 16 deletions boa_engine/src/builtins/iterable/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,26 +392,20 @@ impl IteratorRecord {
/// [spec]: https://tc39.es/ecma262/#sec-iteratornext
pub(crate) fn next(
&self,
value: Option<JsValue>,
value: Option<&JsValue>,
context: &mut Context<'_>,
) -> JsResult<IteratorResult> {
let _timer = Profiler::global().start_event("IteratorRecord::next", "iterator");

// Note: We check if iteratorRecord.[[NextMethod]] is callable here.
// This check would happen in `Call` according to the spec, but we do not implement call for `JsValue`.
let next_method = self.next_method.as_callable().ok_or_else(|| {
JsNativeError::typ().with_message("iterable next method not a function")
})?;

let result = if let Some(value) = value {
// 2. Else,
// a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]], « value »).
next_method.call(&self.iterator.clone().into(), &[value], context)?
} else {
// 1. If value is not present, then
// a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]]).
next_method.call(&self.iterator.clone().into(), &[], context)?
};
// 1. If value is not present, then
// a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]]).
// 2. Else,
// a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]], « value »).
let result = self.next_method.call(
&self.iterator.clone().into(),
value.map_or(&[], std::slice::from_ref),
context,
)?;

// 3. If Type(result) is not Object, throw a TypeError exception.
// 4. Return result.
Expand Down
7 changes: 3 additions & 4 deletions boa_engine/src/builtins/object/for_in_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,15 @@ impl ForInIterator {
/// - [ECMA reference][spec]
///
/// [spec]: https://tc39.es/ecma262/#sec-createforiniterator
pub(crate) fn create_for_in_iterator(object: JsValue, context: &Context<'_>) -> JsValue {
let for_in_iterator = JsObject::from_proto_and_data(
pub(crate) fn create_for_in_iterator(object: JsValue, context: &Context<'_>) -> JsObject {
JsObject::from_proto_and_data(
context
.intrinsics()
.objects()
.iterator_prototypes()
.for_in(),
ObjectData::for_in_iterator(Self::new(object)),
);
for_in_iterator.into()
)
}

/// %ForInIteratorPrototype%.next( )
Expand Down
169 changes: 100 additions & 69 deletions boa_engine/src/bytecompiler/declaration/declaration_pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,80 +175,111 @@ impl ByteCompiler<'_, '_> {
}
Pattern::Array(pattern) => {
self.emit_opcode(Opcode::ValueNotNullOrUndefined);
self.emit_opcode(Opcode::InitIterator);

for binding in pattern.bindings().iter() {
use ArrayPatternElement::{
Elision, Pattern, PatternRest, PropertyAccess, PropertyAccessRest,
SingleName, SingleNameRest,
};

match binding {
// ArrayBindingPattern : [ Elision ]
Elision => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(Opcode::Pop);
self.emit_opcode(Opcode::GetIterator);
match pattern.bindings().split_last() {
None => self.emit_opcode(Opcode::PushFalse),
Some((last, rest)) => {
for element in rest {
self.compile_array_pattern_element(element, def, false);
}
// SingleNameBinding : BindingIdentifier Initializer[opt]
SingleName {
ident,
default_init,
} => {
self.emit_opcode(Opcode::IteratorNext);
if let Some(init) = default_init {
let skip =
self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
self.compile_expr(init, true);
self.patch_jump(skip);
}
self.emit_binding(def, *ident);
}
PropertyAccess { access } => {
self.emit_opcode(Opcode::IteratorNext);
self.access_set(
Access::Property { access },
false,
ByteCompiler::access_set_top_of_stack_expr_fn,
);
}
// BindingElement : BindingPattern Initializer[opt]
Pattern {
pattern,
default_init,
} => {
self.emit_opcode(Opcode::IteratorNext);
self.compile_array_pattern_element(last, def, true);
}
}
self.iterator_close(false);
}
}
}

if let Some(init) = default_init {
let skip =
self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
self.compile_expr(init, true);
self.patch_jump(skip);
}
fn compile_array_pattern_element(
&mut self,
element: &ArrayPatternElement,
def: BindingOpcode,
with_done: bool,
) {
use ArrayPatternElement::{
Elision, Pattern, PatternRest, PropertyAccess, PropertyAccessRest, SingleName,
SingleNameRest,
};

self.compile_declaration_pattern(pattern, def);
}
// BindingRestElement : ... BindingIdentifier
SingleNameRest { ident } => {
self.emit_opcode(Opcode::IteratorToArray);
self.emit_binding(def, *ident);
}
PropertyAccessRest { access } => {
self.emit_opcode(Opcode::IteratorToArray);
self.access_set(
Access::Property { access },
false,
ByteCompiler::access_set_top_of_stack_expr_fn,
);
}
// BindingRestElement : ... BindingPattern
PatternRest { pattern } => {
self.emit_opcode(Opcode::IteratorToArray);
self.compile_declaration_pattern(pattern, def);
}
}
let unwrapping = if with_done {
Opcode::IteratorUnwrapNext
} else {
Opcode::IteratorUnwrapValue
};
match element {
// ArrayBindingPattern : [ Elision ]
Elision => {
self.emit_opcode(Opcode::IteratorNext);
if with_done {
self.emit_opcode(Opcode::IteratorUnwrapNext);
}
self.emit_opcode(Opcode::Pop);
}
// SingleNameBinding : BindingIdentifier Initializer[opt]
SingleName {
ident,
default_init,
} => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(unwrapping);
if let Some(init) = default_init {
let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
self.compile_expr(init, true);
self.patch_jump(skip);
}
self.emit_binding(def, *ident);
}
PropertyAccess { access } => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(unwrapping);
self.access_set(
Access::Property { access },
false,
ByteCompiler::access_set_top_of_stack_expr_fn,
);
}
// BindingElement : BindingPattern Initializer[opt]
Pattern {
pattern,
default_init,
} => {
self.emit_opcode(Opcode::IteratorNext);
self.emit_opcode(unwrapping);

if let Some(init) = default_init {
let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
self.compile_expr(init, true);
self.patch_jump(skip);
}

self.emit_opcode(Opcode::IteratorClose);
self.compile_declaration_pattern(pattern, def);
}
// BindingRestElement : ... BindingIdentifier
SingleNameRest { ident } => {
self.emit_opcode(Opcode::IteratorToArray);
self.emit_binding(def, *ident);
if with_done {
self.emit_opcode(Opcode::PushTrue);
}
}
PropertyAccessRest { access } => {
self.emit_opcode(Opcode::IteratorToArray);
self.access_set(
Access::Property { access },
false,
ByteCompiler::access_set_top_of_stack_expr_fn,
);
if with_done {
self.emit_opcode(Opcode::PushTrue);
}
}
// BindingRestElement : ... BindingPattern
PatternRest { pattern } => {
self.emit_opcode(Opcode::IteratorToArray);
self.compile_declaration_pattern(pattern, def);
if with_done {
self.emit_opcode(Opcode::PushTrue);
}
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions boa_engine/src/bytecompiler/expression/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl ByteCompiler<'_, '_> {
if let Some(element) = element {
self.compile_expr(element, true);
if let Expression::Spread(_) = element {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::GetIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
Expand Down Expand Up @@ -163,9 +163,9 @@ impl ByteCompiler<'_, '_> {

if r#yield.delegate() {
if self.in_async_generator {
self.emit_opcode(Opcode::InitIteratorAsync);
self.emit_opcode(Opcode::GetAsyncIterator);
} else {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::GetIterator);
}
self.emit_opcode(Opcode::PushUndefined);
let start_address = self.next_opcode_location();
Expand All @@ -174,15 +174,15 @@ impl ByteCompiler<'_, '_> {
self.patch_jump(start);
} else if self.in_async_generator {
self.emit_opcode(Opcode::Await);
self.emit_opcode(Opcode::AsyncGeneratorNext);
let jump_return = self.emit_opcode_with_operand(Opcode::JumpIfFalse);
let jump = self.emit_opcode_with_operand(Opcode::JumpIfFalse);
let (skip_yield, skip_yield_await) =
self.emit_opcode_with_two_operands(Opcode::AsyncGeneratorNext);
self.emit_opcode(Opcode::PushUndefined);
self.emit_opcode(Opcode::Yield);
self.emit_opcode(Opcode::GeneratorNext);
self.patch_jump(jump);
self.patch_jump(skip_yield);
self.emit_opcode(Opcode::Await);
self.emit_opcode(Opcode::GeneratorNext);
self.patch_jump(jump_return);
self.patch_jump(skip_yield_await);
} else {
self.emit_opcode(Opcode::Yield);
self.emit_opcode(Opcode::GeneratorNext);
Expand Down Expand Up @@ -265,7 +265,7 @@ impl ByteCompiler<'_, '_> {
for arg in super_call.arguments() {
self.compile_expr(arg, true);
if let Expression::Spread(_) = arg {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::GetIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
Expand Down
26 changes: 13 additions & 13 deletions boa_engine/src/bytecompiler/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod function;
mod jump_control;
mod module;
mod statement;
mod utils;

use crate::{
builtins::function::ThisMode,
Expand Down Expand Up @@ -508,23 +509,23 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
}

fn jump(&mut self) -> Label {
let index = self.next_opcode_location();
self.emit(Opcode::Jump, &[Self::DUMMY_ADDRESS]);
Label { index }
self.emit_opcode_with_operand(Opcode::Jump)
}

fn jump_if_true(&mut self) -> Label {
self.emit_opcode_with_operand(Opcode::JumpIfTrue)
}

fn jump_if_false(&mut self) -> Label {
let index = self.next_opcode_location();
self.emit(Opcode::JumpIfFalse, &[Self::DUMMY_ADDRESS]);
self.emit_opcode_with_operand(Opcode::JumpIfFalse)
}

Label { index }
fn jump_if_not_undefined(&mut self) -> Label {
self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined)
}

fn jump_if_null_or_undefined(&mut self) -> Label {
let index = self.next_opcode_location();
self.emit(Opcode::JumpIfNullOrUndefined, &[Self::DUMMY_ADDRESS]);

Label { index }
self.emit_opcode_with_operand(Opcode::JumpIfNullOrUndefined)
}

/// Emit an opcode with a dummy operand.
Expand Down Expand Up @@ -900,7 +901,6 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
self.patch_jump(label);
}

self.emit_opcode(Opcode::Pop);
self.emit_opcode(Opcode::PushUndefined);

self.patch_jump(skip_undef);
Expand Down Expand Up @@ -957,7 +957,7 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
for arg in args {
self.compile_expr(arg, true);
if let Expression::Spread(_) = arg {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::GetIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
Expand Down Expand Up @@ -1274,7 +1274,7 @@ impl<'b, 'host> ByteCompiler<'b, 'host> {
for arg in call.args() {
self.compile_expr(arg, true);
if let Expression::Spread(_) = arg {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::GetIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
Expand Down
Loading