Browse Source

Remove `FormalParameterList` from CodeBlock (#3882)

It is not used in strict mode and in non-strict mode it's only used to
construct the binding indices for the `MappedArguments`.
pull/3883/head
Haled Odat 6 months ago committed by GitHub
parent
commit
2d91cd1e2c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 72
      core/engine/src/builtins/function/arguments.rs
  2. 1
      core/engine/src/bytecompiler/declarations.rs
  3. 15
      core/engine/src/bytecompiler/mod.rs
  4. 10
      core/engine/src/vm/code_block.rs
  5. 2
      core/engine/src/vm/opcode/arguments.rs
  6. 6
      core/engine/src/vm/opcode/rest_parameter/mod.rs

72
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.
///
/// <https://tc39.es/ecma262/#sec-createmappedargumentsobject>
#[allow(clippy::new_ret_no_self)]
pub(crate) fn new(
func: &JsObject,
formals: &FormalParameterList,
arguments_list: &[JsValue],
env: &Gc<DeclarativeEnvironment>,
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<Option<u32>> {
// 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.
///
/// <https://tc39.es/ecma262/#sec-createmappedargumentsobject>
#[allow(clippy::new_ret_no_self)]
pub(crate) fn new(
func: &JsObject,
binding_indices: &[Option<u32>],
arguments_list: &[JsValue],
env: &Gc<DeclarativeEnvironment>,
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();

1
core/engine/src/bytecompiler/declarations.rs

@ -1182,6 +1182,7 @@ impl ByteCompiler<'_> {
// default value initializers, or any destructured parameters.
// ii. Let ao be CreateMappedArgumentsObject(func, formals, argumentsList, env).
self.emit_opcode(Opcode::CreateMappedArgumentsObject);
self.emitted_mapped_arguments_object_opcode = true;
}
// c. If strict is true, then

15
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) emitted_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,
emitted_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.emitted_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(),

10
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<Option<u32>>,
/// 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(),
}

2
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,

6
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<CompletionType> {
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")

Loading…
Cancel
Save