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.

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

[Merged by Bors] - Implement var initializers in for-in loops #2842

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 10 additions & 8 deletions boa_ast/src/statement/iteration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mod for_of_loop;
mod while_loop;

use crate::{
declaration::Binding,
declaration::{Binding, Variable},
expression::{access::PropertyAccess, Identifier},
pattern::Pattern,
};
Expand Down Expand Up @@ -43,7 +43,7 @@ pub enum IterableLoopInitializer {
/// A property access.
Access(PropertyAccess),
/// A new var declaration.
Var(Binding),
Var(Variable),
/// A new let declaration.
Let(Binding),
/// A new const declaration.
Expand All @@ -58,12 +58,12 @@ impl ToInternedString for IterableLoopInitializer {
Self::Identifier(ident) => return ident.to_interned_string(interner),
Self::Pattern(pattern) => return pattern.to_interned_string(interner),
Self::Access(access) => return access.to_interned_string(interner),
Self::Var(binding) => (binding, "var"),
Self::Let(binding) => (binding, "let"),
Self::Const(binding) => (binding, "const"),
Self::Var(binding) => (binding.to_interned_string(interner), "var"),
Self::Let(binding) => (binding.to_interned_string(interner), "let"),
Self::Const(binding) => (binding.to_interned_string(interner), "const"),
};

format!("{pre} {}", binding.to_interned_string(interner))
format!("{pre} {binding}")
}
}

Expand All @@ -75,7 +75,8 @@ impl VisitWith for IterableLoopInitializer {
match self {
Self::Identifier(id) => visitor.visit_identifier(id),
Self::Access(pa) => visitor.visit_property_access(pa),
Self::Var(b) | Self::Let(b) | Self::Const(b) => visitor.visit_binding(b),
Self::Var(b) => visitor.visit_variable(b),
Self::Let(b) | Self::Const(b) => visitor.visit_binding(b),
Self::Pattern(p) => visitor.visit_pattern(p),
}
}
Expand All @@ -87,7 +88,8 @@ impl VisitWith for IterableLoopInitializer {
match self {
Self::Identifier(id) => visitor.visit_identifier_mut(id),
Self::Access(pa) => visitor.visit_property_access_mut(pa),
Self::Var(b) | Self::Let(b) | Self::Const(b) => visitor.visit_binding_mut(b),
Self::Var(b) => visitor.visit_variable_mut(b),
Self::Let(b) | Self::Const(b) => visitor.visit_binding_mut(b),
Self::Pattern(p) => visitor.visit_pattern_mut(p),
}
}
Expand Down
5 changes: 2 additions & 3 deletions boa_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ repository.workspace = true
rust-version.workspace = true

[dependencies]
boa_engine = { workspace = true, features = ["deser", "console", "flowgraph", "trace", "annex-b"] }
boa_engine = { workspace = true, features = ["deser", "flowgraph", "trace", "console"] }
boa_ast = { workspace = true, features = ["serde"] }
boa_parser.workspace = true
boa_gc.workspace = true
Expand All @@ -26,8 +26,7 @@ phf = { version = "0.11.1", features = ["macros"] }
pollster = "0.3.0"

[features]
default = ["intl"]
intl = ["boa_engine/intl"]
default = ["boa_engine/annex-b", "boa_engine/intl"]

[target.x86_64-unknown-linux-gnu.dependencies]
jemallocator = "0.5.0"
Expand Down
40 changes: 26 additions & 14 deletions boa_engine/src/bytecompiler/statement/loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,16 @@ impl ByteCompiler<'_, '_> {
label: Option<Sym>,
configurable_globals: bool,
) {
// Handle https://tc39.es/ecma262/#prod-annexB-ForInOfStatement
if let IterableLoopInitializer::Var(var) = for_in_loop.initializer() {
if let Binding::Identifier(ident) = var.binding() {
if let Some(init) = var.init() {
self.compile_expr(init, true);
self.create_mutable_binding(*ident, true, true);
self.emit_binding(BindingOpcode::InitVar, *ident);
}
}
}
let initializer_bound_names = match for_in_loop.initializer() {
IterableLoopInitializer::Let(declaration)
| IterableLoopInitializer::Const(declaration) => bound_names(declaration),
Expand Down Expand Up @@ -137,9 +147,7 @@ impl ByteCompiler<'_, '_> {
match for_in_loop.initializer() {
IterableLoopInitializer::Identifier(ident) => {
self.create_mutable_binding(*ident, true, true);
let binding = self.set_mutable_binding(*ident);
let index = self.get_or_insert_binding(binding);
self.emit(Opcode::DefInitVar, &[index]);
self.emit_binding(BindingOpcode::InitVar, *ident);
}
IterableLoopInitializer::Access(access) => {
self.access_set(
Expand All @@ -148,7 +156,7 @@ impl ByteCompiler<'_, '_> {
ByteCompiler::access_set_top_of_stack_expr_fn,
);
}
IterableLoopInitializer::Var(declaration) => match declaration {
IterableLoopInitializer::Var(declaration) => match declaration.binding() {
Binding::Identifier(ident) => {
self.create_mutable_binding(*ident, true, configurable_globals);
self.emit_binding(BindingOpcode::InitVar, *ident);
Expand Down Expand Up @@ -284,18 +292,22 @@ impl ByteCompiler<'_, '_> {
ByteCompiler::access_set_top_of_stack_expr_fn,
);
}
IterableLoopInitializer::Var(declaration) => match declaration {
Binding::Identifier(ident) => {
self.create_mutable_binding(*ident, true, false);
self.emit_binding(BindingOpcode::InitVar, *ident);
}
Binding::Pattern(pattern) => {
for ident in bound_names(pattern) {
self.create_mutable_binding(ident, true, false);
IterableLoopInitializer::Var(declaration) => {
// ignore initializers since those aren't allowed on for-of loops.
assert!(declaration.init().is_none());
match declaration.binding() {
Binding::Identifier(ident) => {
self.create_mutable_binding(*ident, true, false);
self.emit_binding(BindingOpcode::InitVar, *ident);
}
Binding::Pattern(pattern) => {
for ident in bound_names(pattern) {
self.create_mutable_binding(ident, true, false);
}
self.compile_declaration_pattern(pattern, BindingOpcode::InitVar);
}
self.compile_declaration_pattern(pattern, BindingOpcode::InitVar);
}
},
}
IterableLoopInitializer::Let(declaration) => match declaration {
Binding::Identifier(ident) => {
self.create_mutable_binding(*ident, false, false);
Expand Down
14 changes: 9 additions & 5 deletions boa_parser/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,18 +125,22 @@ impl Error {
}
}

/// Creates a "general" parsing error with the specific error message for a wrong function declaration in non-strict mode.
pub(crate) fn wrong_function_declaration_non_strict(position: Position) -> Self {
/// Creates a "general" parsing error with the specific error message for a misplaced function declaration.
pub(crate) fn misplaced_function_declaration(position: Position, strict: bool) -> Self {
Self::General {
message: "In non-strict mode code, functions can only be declared at top level, inside a block, or as the body of an if statement.".into(),
position
message: format!(
"{}functions can only be declared at the top level or inside a block.",
if strict { "in strict mode code, " } else { "" }
)
.into(),
position,
}
}

/// Creates a "general" parsing error with the specific error message for a wrong function declaration with label.
pub(crate) fn wrong_labelled_function_declaration(position: Position) -> Self {
Self::General {
message: "Labelled functions can only be declared at top level or inside a block"
message: "labelled functions can only be declared at the top level or inside a block"
.into(),
position,
}
Expand Down
2 changes: 1 addition & 1 deletion boa_parser/src/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl<R> Lexer<R> {
where
R: Read,
{
if !cfg!(feature = "annex-b") || self.module() {
if cfg!(not(feature = "annex-b")) || self.module() {
return Ok(());
}

Expand Down
11 changes: 6 additions & 5 deletions boa_parser/src/parser/statement/if_stm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,8 @@ where
TokenKind::Keyword((Keyword::Function, _)) => {
// FunctionDeclarations in IfStatement Statement Clauses
// https://tc39.es/ecma262/#sec-functiondeclarations-in-ifstatement-statement-clauses
if strict {
// This production only applies when parsing non-strict code.
return Err(Error::wrong_function_declaration_non_strict(position));
if cfg!(not(feature = "annex-b")) || strict {
return Err(Error::misplaced_function_declaration(position, strict));
}
// Source text matched by this production is processed as if each matching
// occurrence of FunctionDeclaration[?Yield, ?Await, ~Default] was the sole
Expand Down Expand Up @@ -117,8 +116,10 @@ where
TokenKind::Keyword((Keyword::Function, _)) => {
// FunctionDeclarations in IfStatement Statement Clauses
// https://tc39.es/ecma262/#sec-functiondeclarations-in-ifstatement-statement-clauses
if strict {
return Err(Error::wrong_function_declaration_non_strict(position));
if cfg!(not(feature = "annex-b")) || strict {
return Err(Error::misplaced_function_declaration(
position, strict,
));
}

// Source text matched by this production is processed as if each matching
Expand Down
48 changes: 35 additions & 13 deletions boa_parser/src/parser/statement/iteration/for_statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ use crate::{
},
Error,
};
use ast::operations::{bound_names, var_declared_names};
use ast::{
declaration::Binding,
operations::{bound_names, var_declared_names},
};
use boa_ast::{
self as ast,
statement::{
Expand Down Expand Up @@ -157,9 +160,13 @@ where
));
}
(Some(init), TokenKind::Keyword((kw @ (Keyword::In | Keyword::Of), false))) => {
let kw = *kw;
let init =
initializer_to_iterable_loop_initializer(init, position, cursor.strict())?;
let in_loop = kw == &Keyword::In;
let init = initializer_to_iterable_loop_initializer(
init,
position,
cursor.strict(),
in_loop,
)?;

cursor.advance(interner);
let expr = Expression::new(None, true, self.allow_yield, self.allow_await)
Expand Down Expand Up @@ -208,7 +215,7 @@ where
}
}
}
return Ok(if kw == Keyword::In {
return Ok(if in_loop {
ForInLoop::new(init, expr, body).into()
} else {
ForOfLoop::new(init, expr, body, r#await).into()
Expand Down Expand Up @@ -288,7 +295,9 @@ fn initializer_to_iterable_loop_initializer(
initializer: ForLoopInitializer,
position: Position,
strict: bool,
in_loop: bool,
) -> ParseResult<IterableLoopInitializer> {
let loop_type = if in_loop { "for-in" } else { "for-of" };
match initializer {
ForLoopInitializer::Expression(mut expr) => {
while let ast::Expression::Parenthesized(p) = expr {
Expand Down Expand Up @@ -338,7 +347,7 @@ fn initializer_to_iterable_loop_initializer(
[declaration] => {
if declaration.init().is_some() {
return Err(Error::lex(LexError::Syntax(
"a declaration in the head of a for-of loop can't have an initializer"
format!("a lexical declaration in the head of a {loop_type} loop can't have an initializer")
.into(),
position,
)));
Expand All @@ -353,25 +362,38 @@ fn initializer_to_iterable_loop_initializer(
})
}
_ => Err(Error::lex(LexError::Syntax(
"only one variable can be declared in the head of a for-of loop".into(),
format!("only one variable can be declared in the head of a {loop_type} loop")
.into(),
position,
))),
},
ForLoopInitializer::Var(decl) => match decl.0.as_ref() {
[declaration] => {
// TODO: implement initializers in ForIn heads
// https://tc39.es/ecma262/#sec-initializers-in-forin-statement-heads
if declaration.init().is_some() {
let is_pattern = matches!(declaration.binding(), Binding::Pattern(_));
if declaration.init().is_some()
&& (cfg!(not(feature = "annex-b")) || strict || !in_loop || is_pattern)
{
return Err(Error::lex(LexError::Syntax(
"a declaration in the head of a for-of loop can't have an initializer"
.into(),
format!(
"{}a {} declaration in the head of a {loop_type} loop \
cannot have an initializer",
if strict { "in strict mode, " } else { "" },
if is_pattern {
"binding pattern"
} else {
"binding declaration"
}
)
.into(),
position,
)));
}
Ok(IterableLoopInitializer::Var(declaration.binding().clone()))
Ok(IterableLoopInitializer::Var(declaration.clone()))
}
_ => Err(Error::lex(LexError::Syntax(
"only one variable can be declared in the head of a for-of loop".into(),
format!("only one variable can be declared in the head of a {loop_type} loop")
.into(),
position,
))),
},
Expand Down
18 changes: 11 additions & 7 deletions boa_parser/src/parser/statement/labelled_stm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,22 @@ where
// Early Error: It is a Syntax Error if any strict mode source code matches this rule.
// https://tc39.es/ecma262/#sec-labelled-statements-static-semantics-early-errors
// https://tc39.es/ecma262/#sec-labelled-function-declarations
TokenKind::Keyword((Keyword::Function, _)) if strict => {
return Err(Error::general(
"In strict mode code, functions can only be declared at top level or inside a block.",
next_token.span().start()
TokenKind::Keyword((Keyword::Function, _))
if cfg!(not(feature = "annex-b")) || strict =>
{
return Err(Error::misplaced_function_declaration(
next_token.span().start(),
strict,
))
}
TokenKind::Keyword((Keyword::Function, _)) => {
FunctionDeclaration::new(self.allow_yield, self.allow_await, false)
.parse(cursor, interner)?
.into()
.parse(cursor, interner)?
.into()
}
_ => Statement::new(self.allow_yield, self.allow_await, self.allow_return).parse(cursor, interner)?.into()
_ => Statement::new(self.allow_yield, self.allow_await, self.allow_return)
.parse(cursor, interner)?
.into(),
};

Ok(ast::statement::Labelled::new(labelled_item, label))
Expand Down
9 changes: 7 additions & 2 deletions boa_parser/src/parser/statement/variable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,13 @@ where
.is_some()
{
Some(
Initializer::new(Some(ident), true, self.allow_yield, self.allow_await)
.parse(cursor, interner)?,
Initializer::new(
Some(ident),
self.allow_in,
self.allow_yield,
self.allow_await,
)
.parse(cursor, interner)?,
)
} else {
None
Expand Down
5 changes: 2 additions & 3 deletions boa_tester/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ repository.workspace = true
rust-version.workspace = true

[dependencies]
boa_engine = { workspace = true, features = ["annex-b"] }
boa_engine.workspace = true
boa_gc.workspace = true
clap = { version = "4.2.4", features = ["derive"] }
serde = { version = "1.0.160", features = ["derive"] }
Expand All @@ -31,5 +31,4 @@ comfy-table = "6.1.4"
serde_repr = "0.1.12"

[features]
default = ["intl"]
intl = ["boa_engine/intl"]
default = ["boa_engine/intl", "boa_engine/annex-b"]
5 changes: 4 additions & 1 deletion boa_wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@ repository.workspace = true
rust-version.workspace = true

[dependencies]
boa_engine = { workspace = true, features = ["console", "annex-b"] }
boa_engine = { workspace = true, features = ["console"] }
wasm-bindgen = "0.2.84"
getrandom = { version = "0.2.9", features = ["js"] }
chrono = { version = "0.4.24", features = ["clock", "std", "wasmbind"] }

[features]
default = ["boa_engine/annex-b"]

[lib]
crate-type = ["cdylib", "lib"]
name = "boa_wasm"
Expand Down