diff --git a/boa_engine/src/builtins/function/arguments.rs b/boa_engine/src/builtins/function/arguments.rs index 8251e649e4..307e77c5d3 100644 --- a/boa_engine/src/builtins/function/arguments.rs +++ b/boa_engine/src/builtins/function/arguments.rs @@ -155,18 +155,22 @@ impl Arguments { // We use indices to access environment bindings at runtime. // To map to function parameters to binding indices, we use the fact, that bindings in a // function environment start with all of the arguments in order: + // + // Note: The first binding (binding 0) is where "arguments" is stored. + // // `function f (a,b,c)` // | binding index | `arguments` property key | identifier | - // | 0 | 0 | a | - // | 1 | 1 | b | - // | 2 | 2 | c | + // | 1 | 0 | a | + // | 2 | 1 | b | + // | 3 | 2 | c | // // Notice that the binding index does not correspond to the argument index: // `function f (a,a,b)` => binding indices 0 (a), 1 (b), 2 (c) // | binding index | `arguments` property key | identifier | // | - | 0 | - | - // | 0 | 1 | a | - // | 1 | 2 | b | + // | 1 | 1 | a | + // | 2 | 2 | b | + // // While the `arguments` object contains all arguments, they must not be all bound. // In the case of duplicate parameter names, the last one is bound as the environment binding. // @@ -178,10 +182,14 @@ impl Arguments { if property_index >= len { break; } - let binding_index = bindings.len() as u32; + + // NOTE(HalidOdat): Offset by +1 to account for the first binding ("argument"). + let binding_index = bindings.len() as u32 + 1; + let entry = bindings .entry(name) .or_insert((binding_index, property_index)); + entry.1 = property_index; property_index += 1; } diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index 626b079868..570ed81568 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -1,6 +1,9 @@ use crate::{ bytecompiler::{ByteCompiler, FunctionCompiler, FunctionSpec, Label, NodeKind}, - vm::{create_function_object_fast, create_generator_function_object, BindingOpcode, Opcode}, + vm::{ + create_function_object_fast, create_generator_function_object, BindingOpcode, + CodeBlockFlags, Opcode, + }, JsNativeError, JsResult, }; use boa_ast::{ @@ -844,7 +847,7 @@ impl ByteCompiler<'_, '_> { function_names.reverse(); functions_to_initialize.reverse(); - //15. Let argumentsObjectNeeded be true. + // 15. Let argumentsObjectNeeded be true. let mut arguments_object_needed = true; let arguments = Sym::ARGUMENTS.into(); @@ -882,26 +885,10 @@ impl ByteCompiler<'_, '_> { additional_env = true; } - // 21. For each String paramName of parameterNames, do - for param_name in ¶meter_names { - // a. Let alreadyDeclared be ! env.HasBinding(paramName). - let already_declared = self.has_binding(*param_name); - - // b. NOTE: Early errors ensure that duplicate parameter names can only occur in non-strict - // functions that do not have parameter default values or rest parameters. - - // c. If alreadyDeclared is false, then - if !already_declared { - // i. Perform ! env.CreateMutableBinding(paramName, false). - self.create_mutable_binding(*param_name, false); - - // Note: These steps are not necessary in our implementation. - // ii. If hasDuplicates is true, then - // 1. Perform ! env.InitializeBinding(paramName, undefined). - } - } - // 22. If argumentsObjectNeeded is true, then + // + // NOTE(HalidOdat): Has been moved up, so "arguments" gets registed as + // the first binding in the environment with index 0. if arguments_object_needed { // Note: This happens at runtime. // a. If strict is true or simpleParameterList is false, then @@ -918,21 +905,48 @@ impl ByteCompiler<'_, '_> { // ii. NOTE: In strict mode code early errors prevent attempting to assign // to this binding, so its mutability is not observable. self.create_immutable_binding(arguments, false); - self.arguments_binding = Some(self.initialize_immutable_binding(arguments)); } // d. Else, else { // i. Perform ! env.CreateMutableBinding("arguments", false). self.create_mutable_binding(arguments, false); - self.arguments_binding = Some(self.initialize_mutable_binding(arguments, false)); } + self.code_block_flags |= CodeBlockFlags::NEEDS_ARGUMENTS_OBJECT; + } + + // 21. For each String paramName of parameterNames, do + for param_name in ¶meter_names { + // a. Let alreadyDeclared be ! env.HasBinding(paramName). + let already_declared = self.has_binding(*param_name); + + // b. NOTE: Early errors ensure that duplicate parameter names can only occur in non-strict + // functions that do not have parameter default values or rest parameters. + + // c. If alreadyDeclared is false, then + if !already_declared { + // i. Perform ! env.CreateMutableBinding(paramName, false). + self.create_mutable_binding(*param_name, false); + + // Note: These steps are not necessary in our implementation. + // ii. If hasDuplicates is true, then + // 1. Perform ! env.InitializeBinding(paramName, undefined). + } + } + + // 22. If argumentsObjectNeeded is true, then + if arguments_object_needed { + // MOVED: a-d. + // + // NOTE(HalidOdat): Has been moved up, see comment above. + // Note: This happens at runtime. // e. Perform ! env.InitializeBinding("arguments", ao). // f. Let parameterBindings be the list-concatenation of parameterNames and « "arguments" ». parameter_names.push(arguments); } + // 23. Else, // a. Let parameterBindings be parameterNames. let parameter_bindings = parameter_names.clone(); diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index 468cba3cd0..5fb97a2c88 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -242,9 +242,6 @@ pub struct ByteCompiler<'ctx, 'host> { /// Functions inside this function pub(crate) functions: Vec>, - /// The `arguments` binding location of the function, if set. - pub(crate) arguments_binding: Option, - /// Compile time environments in this function. pub(crate) compile_environments: Vec>>, @@ -299,7 +296,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { functions: Vec::default(), this_mode: ThisMode::Global, params: FormalParameterList::default(), - arguments_binding: None, compile_environments: Vec::default(), class_field_initializer_name: None, code_block_flags, @@ -1349,7 +1345,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { private_names: self.private_names.into_boxed_slice(), bindings: self.bindings.into_boxed_slice(), functions: self.functions.into_boxed_slice(), - arguments_binding: self.arguments_binding, compile_environments: self.compile_environments.into_boxed_slice(), class_field_initializer_name: self.class_field_initializer_name, flags: Cell::new(self.code_block_flags), diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 6d52ca12b5..f9dba8a601 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -69,6 +69,11 @@ bitflags! { /// Does this function have a parameters environment. const PARAMETERS_ENV_BINDINGS = 0b0000_1000; + /// Does this function need a `"arguments"` object. + /// + /// The `"arguments"` binding is the first binding. + const NEEDS_ARGUMENTS_OBJECT = 0b0001_0000; + /// Trace instruction execution to `stdout`. #[cfg(feature = "trace")] const TRACEABLE = 0b1000_0000; @@ -124,10 +129,6 @@ pub struct CodeBlock { /// Functions inside this function pub(crate) functions: Box<[Gc]>, - /// The `arguments` binding location of the function, if set. - #[unsafe_ignore_trace] - pub(crate) arguments_binding: Option, - /// Compile time environments in this function. pub(crate) compile_environments: Box<[Gc>]>, @@ -155,7 +156,6 @@ impl CodeBlock { length, this_mode: ThisMode::Global, params: FormalParameterList::default(), - arguments_binding: None, compile_environments: Box::default(), class_field_initializer_name: None, } @@ -206,6 +206,13 @@ impl CodeBlock { .get() .contains(CodeBlockFlags::PARAMETERS_ENV_BINDINGS) } + + /// Does this function need a `"arguments"` object. + pub(crate) fn needs_arguments_object(&self) -> bool { + self.flags + .get() + .contains(CodeBlockFlags::NEEDS_ARGUMENTS_OBJECT) + } } /// ---- `CodeBlock` private API ---- @@ -1078,7 +1085,19 @@ impl JsObject { .push_lexical(code.compile_environments[last_env].clone()); } - if let Some(binding) = code.arguments_binding { + // Taken from: `FunctionDeclarationInstantiation` abstract function. + // + // Spec: https://tc39.es/ecma262/#sec-functiondeclarationinstantiation + // + // 22. If argumentsObjectNeeded is true, then + if code.needs_arguments_object() { + // a. If strict is true or simpleParameterList is false, then + // i. Let ao be CreateUnmappedArgumentsObject(argumentsList). + // b. Else, + // i. NOTE: A mapped argument object is only provided for non-strict functions + // that don't have a rest parameter, any parameter + // default value initializers, or any destructured parameters. + // ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env). let arguments_obj = if code.strict() || !code.params.is_simple() { Arguments::create_unmapped_arguments_object(args, context) } else { @@ -1091,11 +1110,11 @@ impl JsObject { context, ) }; - context.vm.environments.put_lexical_value( - binding.environment_index(), - binding.binding_index(), - arguments_obj.into(), - ); + let env_index = context.vm.environments.len() as u32 - 1; + context + .vm + .environments + .put_lexical_value(env_index, 0, arguments_obj.into()); } let argument_count = args.len(); @@ -1336,7 +1355,19 @@ impl JsObject { .push_lexical(code.compile_environments[0].clone()); } - if let Some(binding) = code.arguments_binding { + // Taken from: `FunctionDeclarationInstantiation` abstract function. + // + // Spec: https://tc39.es/ecma262/#sec-functiondeclarationinstantiation + // + // 22. If argumentsObjectNeeded is true, then + if code.needs_arguments_object() { + // a. If strict is true or simpleParameterList is false, then + // i. Let ao be CreateUnmappedArgumentsObject(argumentsList). + // b. Else, + // i. NOTE: A mapped argument object is only provided for non-strict functions + // that don't have a rest parameter, any parameter + // default value initializers, or any destructured parameters. + // ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env). let arguments_obj = if code.strict() || !code.params.is_simple() { Arguments::create_unmapped_arguments_object(args, context) } else { @@ -1349,11 +1380,12 @@ impl JsObject { context, ) }; - context.vm.environments.put_lexical_value( - binding.environment_index(), - binding.binding_index(), - arguments_obj.into(), - ); + + let env_index = context.vm.environments.len() as u32 - 1; + context + .vm + .environments + .put_lexical_value(env_index, 0, arguments_obj.into()); } let argument_count = args.len();