Refactor binding declarations

boa_ast/src/statement_list.rs

@@ -9,8 +9,6 @@ use crate::{
 use boa_interner::{Interner, ToIndentedString};
 use core::ops::ControlFlow;
 
-use std::cmp::Ordering;
-
 /// An item inside a [`StatementList`] Parse Node, as defined by the [spec].
 ///
 /// Items in a `StatementList` can be either [`Declaration`]s (functions, classes, let/const declarations)
@@ -27,34 +25,6 @@ pub enum StatementListItem {
     Declaration(Declaration),
 }
 
-impl StatementListItem {
-    /// Returns a node ordering based on the hoistability of each statement.
-    #[must_use]
-    pub const fn hoistable_order(a: &Self, b: &Self) -> Ordering {
-        match (a, b) {
-            (
-                _,
-                Self::Declaration(
-                    Declaration::Function(_)
-                    | Declaration::Generator(_)
-                    | Declaration::AsyncFunction(_)
-                    | Declaration::AsyncGenerator(_),
-                ),
-            ) => Ordering::Greater,
-            (
-                Self::Declaration(
-                    Declaration::Function(_)
-                    | Declaration::Generator(_)
-                    | Declaration::AsyncFunction(_)
-                    | Declaration::AsyncGenerator(_),
-                ),
-                _,
-            ) => Ordering::Less,
-            (_, _) => Ordering::Equal,
-        }
-    }
-}
-
 impl ToIndentedString for StatementListItem {
     /// Creates a string of the value of the node with the given indentation. For example, an
     /// indent level of 2 would produce this:

boa_ast/src/visitor.rs

@@ -142,6 +142,7 @@ node_ref! {
     Break,
     Return,
     Labelled,
+    With,
     Throw,
     Try,
     Identifier,
@@ -336,6 +337,7 @@ pub trait Visitor<'ast>: Sized {
             NodeRef::Break(n) => self.visit_break(n),
             NodeRef::Return(n) => self.visit_return(n),
             NodeRef::Labelled(n) => self.visit_labelled(n),
+            NodeRef::With(n) => self.visit_with(n),
             NodeRef::Throw(n) => self.visit_throw(n),
             NodeRef::Try(n) => self.visit_try(n),
             NodeRef::Identifier(n) => self.visit_identifier(n),
@@ -532,6 +534,7 @@ pub trait VisitorMut<'ast>: Sized {
             NodeRefMut::Break(n) => self.visit_break_mut(n),
             NodeRefMut::Return(n) => self.visit_return_mut(n),
             NodeRefMut::Labelled(n) => self.visit_labelled_mut(n),
+            NodeRefMut::With(n) => self.visit_with_mut(n),
             NodeRefMut::Throw(n) => self.visit_throw_mut(n),
             NodeRefMut::Try(n) => self.visit_try_mut(n),
             NodeRefMut::Identifier(n) => self.visit_identifier_mut(n),

boa_engine/src/builtins/eval/mod.rs

@@ -11,12 +11,10 @@
 
 use crate::{
     builtins::BuiltInObject, bytecompiler::ByteCompiler, context::intrinsics::Intrinsics,
-    environments::Environment, error::JsNativeError, object::JsObject, realm::Realm, Context,
-    JsArgs, JsResult, JsString, JsValue,
-};
-use boa_ast::operations::{
-    contains, contains_arguments, top_level_var_declared_names, ContainsSymbol,
+    environments::Environment, error::JsNativeError, object::JsObject, realm::Realm, vm::Opcode,
+    Context, JsArgs, JsResult, JsString, JsValue,
 };
+use boa_ast::operations::{contains, contains_arguments, ContainsSymbol};
 use boa_gc::Gc;
 use boa_interner::Sym;
 use boa_parser::{Parser, Source};
@@ -224,35 +222,27 @@ impl Eval {
             }
         });
 
-        // Only need to check on non-strict mode since strict mode automatically creates a function
-        // environment for all eval calls.
-        if !strict {
-            // Error if any var declaration in the eval code already exists as a let/const declaration in the current running environment.
-            if let Some(name) = context
-                .vm
-                .environments
-                .has_lex_binding_until_function_environment(&top_level_var_declared_names(&body))
-            {
-                let name = context.interner().resolve_expect(name.sym());
-                let msg = format!("variable declaration {name} in eval function already exists as a lexical variable");
-                return Err(JsNativeError::syntax().with_message(msg).into());
-            }
-        }
+        let mut compiler = ByteCompiler::new(
+            Sym::MAIN,
+            body.strict(),
+            false,
+            context.vm.environments.current_compile_environment(),
+            context,
+        );
+
+        compiler.push_compile_environment(strict);
+        let push_env = compiler.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment);
+
+        compiler.eval_declaration_instantiation(&body, strict)?;
+        compiler.compile_statement_list(&body, true);
+
+        let env_info = compiler.pop_compile_environment();
+        compiler.patch_jump_with_target(push_env.0, env_info.num_bindings as u32);
+        compiler.patch_jump_with_target(push_env.1, env_info.index as u32);
+        compiler.emit_opcode(Opcode::PopEnvironment);
+
+        let code_block = Gc::new(compiler.finish());
 
-        // TODO: check if private identifiers inside `eval` are valid.
-
-        // Compile and execute the eval statement list.
-        let code_block = {
-            let mut compiler = ByteCompiler::new(
-                Sym::MAIN,
-                body.strict(),
-                false,
-                context.vm.environments.current_compile_environment(),
-                context,
-            );
-            compiler.compile_statement_list_with_new_declarative(&body, true, strict);
-            Gc::new(compiler.finish())
-        };
         // Indirect calls don't need extensions, because a non-strict indirect call modifies only
         // the global object.
         // Strict direct calls also don't need extensions, since all strict eval calls push a new

boa_engine/src/builtins/function/arguments.rs

@@ -176,7 +176,7 @@ impl Arguments {
             if property_index >= len {
                 break;
             }
-            let binding_index = bindings.len() + 1;
+            let binding_index = bindings.len();
             let entry = bindings
                 .entry(name)
                 .or_insert((binding_index, property_index));

boa_engine/src/builtins/json/mod.rs

@@ -123,7 +123,7 @@ impl Json {
                 context.realm().environment().compile_env(),
                 context,
             );
-            compiler.compile_statement_list(&statement_list, true, false);
+            compiler.compile_statement_list(&statement_list, true);
             Gc::new(compiler.finish())
         };
         let unfiltered = context.execute(code_block)?;

boa_engine/src/bytecompiler/class.rs

@@ -1,10 +1,8 @@
 use super::{ByteCompiler, Literal, NodeKind};
 use crate::vm::{BindingOpcode, Opcode};
 use boa_ast::{
-    declaration::Binding,
     expression::Identifier,
-    function::{Class, ClassElement},
-    operations::bound_names,
+    function::{Class, ClassElement, FormalParameterList},
     property::{MethodDefinition, PropertyName},
 };
 use boa_gc::Gc;
@@ -40,51 +38,22 @@ impl ByteCompiler<'_, '_> {
         if let Some(expr) = class.constructor() {
             compiler.length = expr.parameters().length();
             compiler.params = expr.parameters().clone();
-            compiler.create_mutable_binding(Sym::ARGUMENTS.into(), false, false);
-            compiler.arguments_binding =
-                Some(compiler.initialize_mutable_binding(Sym::ARGUMENTS.into(), false));
-            for parameter in expr.parameters().as_ref() {
-                if parameter.is_rest_param() {
-                    compiler.emit_opcode(Opcode::RestParameterInit);
-                }
 
-                match parameter.variable().binding() {
-                    Binding::Identifier(ident) => {
-                        compiler.create_mutable_binding(*ident, false, false);
-                        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);
-                    }
-                    Binding::Pattern(pattern) => {
-                        for ident in bound_names(pattern) {
-                            compiler.create_mutable_binding(ident, false, false);
-                        }
-                        compiler.compile_declaration_pattern(pattern, BindingOpcode::InitArg);
-                    }
-                }
-            }
-            if !expr.parameters().has_rest_parameter() {
-                compiler.emit_opcode(Opcode::RestParameterPop);
-            }
-            let env_label = if expr.parameters().has_expressions() {
-                compiler.num_bindings = compiler.current_environment.borrow().num_bindings();
-                compiler.push_compile_environment(true);
-                compiler.function_environment_push_location = compiler.next_opcode_location();
-                Some(compiler.emit_opcode_with_two_operands(Opcode::PushFunctionEnvironment))
-            } else {
-                None
-            };
-            compiler.compile_statement_list(expr.body(), false, false);
+            let (env_labels, _) = compiler.function_declaration_instantiation(
+                expr.body(),
+                expr.parameters(),
+                false,
+                true,
+                false,
+            );
+
+            compiler.compile_statement_list(expr.body(), false);
 
             let env_info = compiler.pop_compile_environment();
 
-            if let Some(env_label) = env_label {
-                compiler.patch_jump_with_target(env_label.0, env_info.num_bindings as u32);
-                compiler.patch_jump_with_target(env_label.1, env_info.index as u32);
+            if let Some(env_labels) = env_labels {
+                compiler.patch_jump_with_target(env_labels.0, env_info.num_bindings as u32);
+                compiler.patch_jump_with_target(env_labels.1, env_info.index as u32);
                 compiler.pop_compile_environment();
             } else {
                 compiler.num_bindings = env_info.num_bindings;
@@ -396,8 +365,16 @@ impl ByteCompiler<'_, '_> {
                     compiler.push_compile_environment(false);
                     compiler.create_immutable_binding(class_name.into(), true);
                     compiler.push_compile_environment(true);
-                    compiler.create_declarations(statement_list, false);
-                    compiler.compile_statement_list(statement_list, false, false);
+
+                    compiler.function_declaration_instantiation(
+                        statement_list,
+                        &FormalParameterList::default(),
+                        false,
+                        true,
+                        false,
+                    );
+
+                    compiler.compile_statement_list(statement_list, false);
                     let env_info = compiler.pop_compile_environment();
                     compiler.pop_compile_environment();
                     compiler.num_bindings = env_info.num_bindings;