Skip to content

Commit

Permalink
Align iterator loops to the spec (#2686)
Browse files Browse the repository at this point in the history 
~Depends on #2683.~ Merged.

This Pull Request fixes #2658.

It changes the following:

- Makes `for .. of` loop execution more spec compliant.
- Rewrites iterator related opcodes to be able to use it on all for .. of/in loops.
- Adds some utility op codes.
  • Loading branch information
@jedel1043
jedel1043 committed Mar 21, 2023
1 parent 3b51226 commit cb4e49a
Show file tree
Hide file tree
Showing 26 changed files with 577 additions and 448 deletions.
2 changes: 1 addition & 1 deletion boa_engine/src/builtins/iterable/async_from_sync_iterator.rs
View file
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
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
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
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
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
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

0 comments on commit cb4e49a

Please sign in to comment.