Split Node into Statement, Expression and Declaration (#2319)

This Pull Request fixes #1371. And yeah, the number of file changes is real... It changes the following: - Split the ast `Node` into `Statement` and `Expression`. - Rewrite the parser and bytecompiler to conform to this change. - Refactor some ast nodes into reusable structures. - Rewrite `contains_arguments` and `contains` to ease the transition into a future ast visitor. List of things that were apparently fixed by this refactor?: - Implement read-assign operation for private accessors (e.g. `this.#field ||= 5`). - `var await` declaration now allowed outside `async` functions and inside functions nested in async functions. - Reject redeclarations of variables declared in the init list of a for loop. Still missing some documentation adjustments, will try to do it ASAP.
boa-dev · Oct 16, 2022 · 959b9f1 · 959b9f1
1 parent 13dcdd4
commit 959b9f1
Show file tree

Hide file tree

Showing 191 changed files with 11,624 additions and 11,439 deletions.
diff --git a/boa_cli/src/main.rs b/boa_cli/src/main.rs
@@ -59,7 +59,7 @@
     rustdoc::missing_doc_code_examples
 )]
 
-use boa_engine::{syntax::ast::node::StatementList, Context};
+use boa_engine::{syntax::ast::StatementList, Context};
 use clap::{Parser, ValueEnum, ValueHint};
 use colored::{Color, Colorize};
 use rustyline::{config::Config, error::ReadlineError, EditMode, Editor};

diff --git a/boa_engine/src/builtins/eval/mod.rs b/boa_engine/src/builtins/eval/mod.rs
@@ -66,9 +66,7 @@ impl Eval {
         context: &mut Context,
     ) -> Result<JsValue, JsValue> {
         // 1. Assert: If direct is false, then strictCaller is also false.
-        if !direct {
-            debug_assert!(!strict);
-        }
+        debug_assert!(direct || !strict);
 
         // 2. If Type(x) is not String, return x.
         let x = if let Some(x) = x.as_string() {
@@ -102,14 +100,14 @@ impl Eval {
 
             // Error if any var declaration in the eval code already exists as a let/const declaration in the current running environment.
             let mut vars = FxHashSet::default();
-            body.var_declared_names_new(&mut vars);
+            body.var_declared_names(&mut vars);
             if let Some(name) = context
                 .realm
                 .environments
                 .has_lex_binding_until_function_environment(&vars)
             {
-                let name = context.interner().resolve_expect(name);
-                let msg = format!("variable declaration {name} in eval function already exists as lexically declaration");
+                let name = context.interner().resolve_expect(name.sym());
+                let msg = format!("variable declaration `{name}` in eval function already exists as lexically declaration");
                 return context.throw_syntax_error(msg);
             }
 

diff --git a/boa_engine/src/builtins/function/arguments.rs b/boa_engine/src/builtins/function/arguments.rs
@@ -3,7 +3,7 @@ use crate::{
     object::{JsObject, ObjectData},
     property::PropertyDescriptor,
     symbol::{self, WellKnownSymbols},
-    syntax::ast::node::FormalParameterList,
+    syntax::ast::function::FormalParameterList,
     Context, JsValue,
 };
 use boa_gc::{Finalize, Gc, Trace};

diff --git a/boa_engine/src/builtins/function/mod.rs b/boa_engine/src/builtins/function/mod.rs
@@ -13,7 +13,7 @@
 
 use crate::{
     builtins::{BuiltIn, JsArgs},
-    bytecompiler::{FunctionCompiler, FunctionKind},
+    bytecompiler::FunctionCompiler,
     context::intrinsics::StandardConstructors,
     environments::DeclarativeEnvironmentStack,
     js_string,
@@ -25,10 +25,7 @@ use crate::{
     property::{Attribute, PropertyDescriptor, PropertyKey},
     string::utf16,
     symbol::WellKnownSymbols,
-    syntax::{
-        ast::node::{FormalParameterList, StatementList},
-        Parser,
-    },
+    syntax::{ast::function::FormalParameterList, ast::StatementList, Parser},
     value::IntegerOrInfinity,
     Context, JsResult, JsString, JsValue,
 };
@@ -594,7 +591,7 @@ impl BuiltInFunctionObject {
                         {
                             return context.throw_syntax_error(format!(
                                 "Redeclaration of formal parameter `{}`",
-                                context.interner().resolve_expect(param_name)
+                                context.interner().resolve_expect(param_name.sym())
                             ));
                         }
                     }
@@ -605,7 +602,6 @@ impl BuiltInFunctionObject {
                 .name(Sym::ANONYMOUS)
                 .generator(generator)
                 .r#async(r#async)
-                .kind(FunctionKind::Expression)
                 .compile(&parameters, &body, context)?;
 
             let environments = context.realm.environments.pop_to_global();
@@ -623,7 +619,6 @@ impl BuiltInFunctionObject {
             let code = FunctionCompiler::new()
                 .name(Sym::ANONYMOUS)
                 .generator(true)
-                .kind(FunctionKind::Expression)
                 .compile(
                     &FormalParameterList::empty(),
                     &StatementList::default(),
@@ -637,14 +632,11 @@ impl BuiltInFunctionObject {
 
             Ok(function_object)
         } else {
-            let code = FunctionCompiler::new()
-                .name(Sym::ANONYMOUS)
-                .kind(FunctionKind::Expression)
-                .compile(
-                    &FormalParameterList::empty(),
-                    &StatementList::default(),
-                    context,
-                )?;
+            let code = FunctionCompiler::new().name(Sym::ANONYMOUS).compile(
+                &FormalParameterList::empty(),
+                &StatementList::default(),
+                context,
+            )?;
 
             let environments = context.realm.environments.pop_to_global();
             let function_object =

diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs
@@ -1,7 +1,7 @@
 use crate::{
     builtins::function::ThisMode,
-    bytecompiler::{ByteCompiler, FunctionKind},
-    syntax::ast::node::{Declaration, FormalParameterList, StatementList},
+    bytecompiler::ByteCompiler,
+    syntax::ast::{declaration::Binding, function::FormalParameterList, StatementList},
     vm::{BindingOpcode, CodeBlock, Opcode},
     Context, JsResult,
 };
@@ -11,12 +11,13 @@ use rustc_hash::FxHashMap;
 
 /// `FunctionCompiler` is used to compile AST functions to bytecode.
 #[derive(Debug, Clone, Copy)]
+#[allow(clippy::struct_excessive_bools)]
 pub(crate) struct FunctionCompiler {
     name: Sym,
     generator: bool,
     r#async: bool,
     strict: bool,
-    kind: FunctionKind,
+    arrow: bool,
 }
 
 impl FunctionCompiler {
@@ -28,7 +29,7 @@ impl FunctionCompiler {
             generator: false,
             r#async: false,
             strict: false,
-            kind: FunctionKind::Declaration,
+            arrow: false,
         }
     }
 
@@ -45,6 +46,12 @@ impl FunctionCompiler {
         self
     }
 
+    /// Indicate if the function is an arrow function.
+    #[inline]
+    pub(crate) fn arrow(mut self, arrow: bool) -> Self {
+        self.arrow = arrow;
+        self
+    }
     /// Indicate if the function is a generator function.
     #[inline]
     pub(crate) fn generator(mut self, generator: bool) -> Self {
@@ -66,13 +73,6 @@ impl FunctionCompiler {
         self
     }
 
-    /// Indicate if the function is a declaration, expression or arrow function.
-    #[inline]
-    pub(crate) fn kind(mut self, kind: FunctionKind) -> Self {
-        self.kind = kind;
-        self
-    }
-
     /// Compile a function statement list and it's parameters into bytecode.
     pub(crate) fn compile(
         mut self,
@@ -85,7 +85,7 @@ impl FunctionCompiler {
         let length = parameters.length();
         let mut code = CodeBlock::new(self.name, length, self.strict);
 
-        if self.kind == FunctionKind::Arrow {
+        if self.arrow {
             code.this_mode = ThisMode::Lexical;
         }
 
@@ -106,14 +106,14 @@ impl FunctionCompiler {
         // - If the parameter list does not contain `arguments` (10.2.11.17)
         // Note: This following just means, that we add an extra environment for the arguments.
         // - If there are default parameters or if lexical names and function names do not contain `arguments` (10.2.11.18)
-        if !(self.kind == FunctionKind::Arrow) && !parameters.has_arguments() {
+        if !(self.arrow) && !parameters.has_arguments() {
             compiler
                 .context
-                .create_mutable_binding(Sym::ARGUMENTS, false);
+                .create_mutable_binding(Sym::ARGUMENTS.into(), false);
             compiler.code_block.arguments_binding = Some(
                 compiler
                     .context
-                    .initialize_mutable_binding(Sym::ARGUMENTS, false),
+                    .initialize_mutable_binding(Sym::ARGUMENTS.into(), false),
             );
         }
 
@@ -122,20 +122,27 @@ impl FunctionCompiler {
                 compiler.emit_opcode(Opcode::RestParameterInit);
             }
 
-            match parameter.declaration() {
-                Declaration::Identifier { ident, .. } => {
-                    compiler.context.create_mutable_binding(ident.sym(), false);
-                    if let Some(init) = parameter.declaration().init() {
+            match parameter.variable().binding() {
+                Binding::Identifier(ident) => {
+                    compiler.context.create_mutable_binding(*ident, false);
+                    // TODO: throw custom error if ident is in init
+                    if let Some(init) = parameter.variable().init() {
                         let skip = compiler.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
                         compiler.compile_expr(init, true)?;
                         compiler.patch_jump(skip);
                     }
-                    compiler.emit_binding(BindingOpcode::InitArg, ident.sym());
+                    compiler.emit_binding(BindingOpcode::InitArg, *ident);
                 }
-                Declaration::Pattern(pattern) => {
+                Binding::Pattern(pattern) => {
                     for ident in pattern.idents() {
                         compiler.context.create_mutable_binding(ident, false);
                     }
+                    // TODO: throw custom error if ident is in init
+                    if let Some(init) = parameter.variable().init() {
+                        let skip = compiler.emit_opcode_with_operand(Opcode::JumpIfNotUndefined);
+                        compiler.compile_expr(init, true)?;
+                        compiler.patch_jump(skip);
+                    }
                     compiler.compile_declaration_pattern(pattern, BindingOpcode::InitArg)?;
                 }
             }
@@ -161,8 +168,8 @@ impl FunctionCompiler {
             compiler.emit_opcode(Opcode::Yield);
         }
 
-        compiler.create_declarations(body.items())?;
-        compiler.compile_statement_list(body.items(), false)?;
+        compiler.create_decls(body);
+        compiler.compile_statement_list(body, false)?;
 
         if let Some(env_label) = env_label {
             let (num_bindings, compile_environment) =