From ee719d33c55d885504205cb7f10875c817405c25 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Thu, 27 Jun 2024 19:15:38 +0200 Subject: [PATCH] Remove `FormalParameterList` from CodeBlock It is not used in strict mode and in non-strict mode it's only used to construct the binding indices for the `MappedArguments`. --- .../engine/src/builtins/function/arguments.rs | 72 ++++++++++--------- core/engine/src/bytecompiler/declarations.rs | 1 + core/engine/src/bytecompiler/mod.rs | 15 +++- core/engine/src/vm/code_block.rs | 10 +-- core/engine/src/vm/opcode/arguments.rs | 2 +- .../src/vm/opcode/rest_parameter/mod.rs | 6 +- 6 files changed, 61 insertions(+), 45 deletions(-) diff --git a/core/engine/src/builtins/function/arguments.rs b/core/engine/src/builtins/function/arguments.rs index 001185c24b0..2112aeaf62e 100644 --- a/core/engine/src/builtins/function/arguments.rs +++ b/core/engine/src/builtins/function/arguments.rs @@ -13,6 +13,7 @@ use crate::{ use boa_ast::{function::FormalParameterList, operations::bound_names}; use boa_gc::{Finalize, Gc, Trace}; use rustc_hash::FxHashMap; +use thin_vec::{thin_vec, ThinVec}; #[derive(Debug, Copy, Clone, Trace, Finalize, JsData)] #[boa_gc(empty_trace)] @@ -137,30 +138,7 @@ impl MappedArguments { } impl MappedArguments { - /// Creates a new mapped Arguments exotic object. - /// - /// - #[allow(clippy::new_ret_no_self)] - pub(crate) fn new( - func: &JsObject, - formals: &FormalParameterList, - arguments_list: &[JsValue], - env: &Gc, - context: &mut Context, - ) -> JsObject { - // 1. Assert: formals does not contain a rest parameter, any binding patterns, or any initializers. - // It may contain duplicate identifiers. - // 2. Let len be the number of elements in argumentsList. - let len = arguments_list.len(); - - // 3. Let obj be ! MakeBasicObject(« [[Prototype]], [[Extensible]], [[ParameterMap]] »). - // 4. Set obj.[[GetOwnProperty]] as specified in 10.4.4.1. - // 5. Set obj.[[DefineOwnProperty]] as specified in 10.4.4.2. - // 6. Set obj.[[Get]] as specified in 10.4.4.3. - // 7. Set obj.[[Set]] as specified in 10.4.4.4. - // 8. Set obj.[[Delete]] as specified in 10.4.4.5. - // 9. Set obj.[[Prototype]] to %Object.prototype%. - + pub(crate) fn binding_indices(formals: &FormalParameterList) -> ThinVec> { // Section 17-19 are done first, for easier object creation in 11. // // The section 17-19 differs from the spec, due to the way the runtime environments work. @@ -196,14 +174,9 @@ impl MappedArguments { // In the case of duplicate parameter names, the last one is bound as the environment binding. // // The following logic implements the steps 17-19 adjusted for our environment structure. - let mut bindings = FxHashMap::default(); let mut property_index = 0; for name in bound_names(formals) { - if property_index >= len { - break; - } - // NOTE(HalidOdat): Offset by +1 to account for the first binding ("argument"). let binding_index = bindings.len() as u32 + 1; @@ -215,15 +188,44 @@ impl MappedArguments { property_index += 1; } - let mut map = MappedArguments { - binding_indices: vec![None; property_index], - environment: env.clone(), - }; - + let mut binding_indices = thin_vec![None; property_index]; for (binding_index, property_index) in bindings.values() { - map.binding_indices[*property_index] = Some(*binding_index); + binding_indices[*property_index] = Some(*binding_index); } + binding_indices + } + + /// Creates a new mapped Arguments exotic object. + /// + /// + #[allow(clippy::new_ret_no_self)] + pub(crate) fn new( + func: &JsObject, + binding_indices: &[Option], + arguments_list: &[JsValue], + env: &Gc, + context: &mut Context, + ) -> JsObject { + // 1. Assert: formals does not contain a rest parameter, any binding patterns, or any initializers. + // It may contain duplicate identifiers. + // 2. Let len be the number of elements in argumentsList. + let len = arguments_list.len(); + + // 3. Let obj be ! MakeBasicObject(« [[Prototype]], [[Extensible]], [[ParameterMap]] »). + // 4. Set obj.[[GetOwnProperty]] as specified in 10.4.4.1. + // 5. Set obj.[[DefineOwnProperty]] as specified in 10.4.4.2. + // 6. Set obj.[[Get]] as specified in 10.4.4.3. + // 7. Set obj.[[Set]] as specified in 10.4.4.4. + // 8. Set obj.[[Delete]] as specified in 10.4.4.5. + // 9. Set obj.[[Prototype]] to %Object.prototype%. + + let range = binding_indices.len().min(len); + let map = MappedArguments { + binding_indices: binding_indices[..range].to_vec(), + environment: env.clone(), + }; + // %Array.prototype.values% let values_function = context.intrinsics().objects().array_prototype_values(); diff --git a/core/engine/src/bytecompiler/declarations.rs b/core/engine/src/bytecompiler/declarations.rs index 793f8a25d43..109aa23b267 100644 --- a/core/engine/src/bytecompiler/declarations.rs +++ b/core/engine/src/bytecompiler/declarations.rs @@ -1174,6 +1174,7 @@ impl ByteCompiler<'_> { if strict || !formals.is_simple() { // i. Let ao be CreateUnmappedArgumentsObject(argumentsList). self.emit_opcode(Opcode::CreateUnmappedArgumentsObject); + self.pushed_mapped_arguments_object_opcode = true; } // b. Else, else { diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index bad92dad8e8..c519878853c 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -14,7 +14,7 @@ mod utils; use std::{cell::Cell, rc::Rc}; use crate::{ - builtins::function::ThisMode, + builtins::function::{arguments::MappedArguments, ThisMode}, environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment}, js_string, vm::{ @@ -307,6 +307,9 @@ pub struct ByteCompiler<'ctx> { /// Whether the function is in a `with` statement. pub(crate) in_with: bool, + /// Used to determine if a we emited a `CreateUnmappedArgumentsObject` opcode + pub(crate) pushed_mapped_arguments_object_opcode: bool, + pub(crate) interner: &'ctx mut Interner, #[cfg(feature = "annex-b")] @@ -361,6 +364,7 @@ impl<'ctx> ByteCompiler<'ctx> { #[cfg(feature = "annex-b")] annex_b_function_names: Vec::new(), in_with, + pushed_mapped_arguments_object_opcode: false, } } @@ -1575,12 +1579,19 @@ impl<'ctx> ByteCompiler<'ctx> { handler.stack_count += self.register_count; } + let mapped_arguments_binding_indices = if self.pushed_mapped_arguments_object_opcode { + MappedArguments::binding_indices(&self.params) + } else { + ThinVec::new() + }; + CodeBlock { name: self.function_name, length: self.length, register_count: self.register_count, this_mode: self.this_mode, - params: self.params, + parameter_length: self.params.as_ref().len() as u32, + mapped_arguments_binding_indices, bytecode: self.bytecode.into_boxed_slice(), constants: self.constants, bindings: self.bindings.into_boxed_slice(), diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index 66f8682690c..b3fbb40a78b 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -12,7 +12,6 @@ use crate::{ Context, JsBigInt, JsString, JsValue, }; use bitflags::bitflags; -use boa_ast::function::FormalParameterList; use boa_gc::{empty_trace, Finalize, Gc, Trace}; use boa_profiler::Profiler; use std::{cell::Cell, fmt::Display, mem::size_of, rc::Rc}; @@ -137,14 +136,16 @@ pub struct CodeBlock { /// The number of arguments expected. pub(crate) length: u32, + pub(crate) parameter_length: u32, + pub(crate) register_count: u32, /// `[[ThisMode]]` pub(crate) this_mode: ThisMode, - /// Parameters passed to this function. + /// Used for constructing a `MappedArguments` object. #[unsafe_ignore_trace] - pub(crate) params: FormalParameterList, + pub(crate) mapped_arguments_binding_indices: ThinVec>, /// Bytecode #[unsafe_ignore_trace] @@ -180,7 +181,8 @@ impl CodeBlock { length, register_count: 0, this_mode: ThisMode::Global, - params: FormalParameterList::default(), + mapped_arguments_binding_indices: ThinVec::new(), + parameter_length: 0, handlers: ThinVec::default(), ic: Box::default(), } diff --git a/core/engine/src/vm/opcode/arguments.rs b/core/engine/src/vm/opcode/arguments.rs index 7daf372ee31..92a86560528 100644 --- a/core/engine/src/vm/opcode/arguments.rs +++ b/core/engine/src/vm/opcode/arguments.rs @@ -31,7 +31,7 @@ impl Operation for CreateMappedArgumentsObject { let env = context.vm.environments.current(); let arguments = MappedArguments::new( &function_object, - &code.params, + &code.mapped_arguments_binding_indices, &args, env.declarative_expect(), context, diff --git a/core/engine/src/vm/opcode/rest_parameter/mod.rs b/core/engine/src/vm/opcode/rest_parameter/mod.rs index eda0764c610..daabd434f5d 100644 --- a/core/engine/src/vm/opcode/rest_parameter/mod.rs +++ b/core/engine/src/vm/opcode/rest_parameter/mod.rs @@ -17,12 +17,12 @@ impl Operation for RestParameterInit { const COST: u8 = 6; fn execute(context: &mut Context) -> JsResult { - let argument_count = context.vm.frame().argument_count as usize; - let param_count = context.vm.frame().code_block().params.as_ref().len(); + let argument_count = context.vm.frame().argument_count; + let param_count = context.vm.frame().code_block().parameter_length; let array = if argument_count >= param_count { let rest_count = argument_count - param_count + 1; - let args = context.vm.pop_n_values(rest_count); + let args = context.vm.pop_n_values(rest_count as usize); Array::create_array_from_list(args, context) } else { Array::array_create(0, None, context).expect("could not create an empty array")