Skip to content

Commit

Permalink
Remove FormalParameterList from CodeBlock (#3882)
Browse files Browse the repository at this point in the history
It is not used in strict mode and in non-strict mode it's only used to
construct the binding indices for the `MappedArguments`.
  • Loading branch information
HalidOdat authored Jun 29, 2024
1 parent 2458d73 commit 2d91cd1
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 45 deletions.
72 changes: 37 additions & 35 deletions core/engine/src/builtins/function/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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();

Expand Down
1 change: 1 addition & 0 deletions core/engine/src/bytecompiler/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 13 additions & 2 deletions core/engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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(),
Expand Down
10 changes: 6 additions & 4 deletions core/engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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(),
}
Expand Down
2 changes: 1 addition & 1 deletion core/engine/src/vm/opcode/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions core/engine/src/vm/opcode/rest_parameter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit 2d91cd1

Please sign in to comment.