From 0c790ac623e4377332b74da2790a5931a2026db5 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 10 Jun 2023 23:40:55 +0200 Subject: [PATCH] Merge `private_names` into `names` constant pool (#3023) * Merge `private_names` into `names` constant pool * Fix clippy lint --- boa_engine/src/bytecompiler/declarations.rs | 9 +++++++ boa_engine/src/bytecompiler/mod.rs | 18 ++----------- boa_engine/src/environments/runtime/mod.rs | 9 +++---- .../src/environments/runtime/private.rs | 9 ++++--- boa_engine/src/object/jsobject.rs | 3 +-- boa_engine/src/object/mod.rs | 7 +++-- boa_engine/src/object/operations.rs | 4 +-- boa_engine/src/vm/code_block.rs | 22 ++++----------- boa_engine/src/vm/opcode/binary_ops/mod.rs | 4 +-- boa_engine/src/vm/opcode/get/private.rs | 4 +-- boa_engine/src/vm/opcode/push/class/field.rs | 4 +-- .../src/vm/opcode/push/class/private.rs | 14 +++++----- boa_engine/src/vm/opcode/push/environment.rs | 4 +-- boa_engine/src/vm/opcode/set/private.rs | 27 +++++++++---------- 14 files changed, 59 insertions(+), 79 deletions(-) diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index b66b1d2f20..d95b54df80 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -437,6 +437,15 @@ impl ByteCompiler<'_, '_> { // append binding.[[Description]] to privateIdentifiers. // b. Set pointer to pointer.[[OuterPrivateEnvironment]]. let private_identifiers = self.context.vm.environments.private_name_descriptions(); + let private_identifiers = private_identifiers + .into_iter() + .map(|ident| { + self.context + .interner() + .get(ident.as_slice()) + .expect("string should be in interner") + }) + .collect(); // 7. If AllPrivateIdentifiersValid of body with argument privateIdentifiers is false, throw a SyntaxError exception. if !all_private_identifiers_valid(body, private_identifiers) { diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index 996a6acaa3..51bdf9c4b2 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -231,12 +231,9 @@ pub struct ByteCompiler<'ctx, 'host> { /// Literals pub(crate) literals: Vec, - /// Property field names. + /// Property field names and private name `[[Description]]`s. pub(crate) names: Vec, - /// Private names. - pub(crate) private_names: Vec, - /// Locators for all bindings in the codeblock. pub(crate) bindings: Vec, @@ -256,7 +253,6 @@ pub struct ByteCompiler<'ctx, 'host> { literals_map: FxHashMap, names_map: FxHashMap, - private_names_map: FxHashMap, bindings_map: FxHashMap, jump_info: Vec, in_async_generator: bool, @@ -292,7 +288,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { bytecode: Vec::default(), literals: Vec::default(), names: Vec::default(), - private_names: Vec::default(), bindings: Vec::default(), functions: Vec::default(), this_mode: ThisMode::Global, @@ -303,7 +298,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { literals_map: FxHashMap::default(), names_map: FxHashMap::default(), - private_names_map: FxHashMap::default(), bindings_map: FxHashMap::default(), jump_info: Vec::new(), in_async_generator: false, @@ -354,14 +348,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { #[inline] fn get_or_insert_private_name(&mut self, name: PrivateName) -> u32 { - if let Some(index) = self.private_names_map.get(&name) { - return *index; - } - - let index = self.private_names.len() as u32; - self.private_names.push(name); - self.private_names_map.insert(name, index); - index + self.get_or_insert_name(Identifier::new(name.description())) } #[inline] @@ -1361,7 +1348,6 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { bytecode: self.bytecode.into_boxed_slice(), literals: self.literals.into_boxed_slice(), names: self.names.into_boxed_slice(), - private_names: self.private_names.into_boxed_slice(), bindings: self.bindings.into_boxed_slice(), functions: self.functions.into_boxed_slice(), compile_environments: self.compile_environments.into_boxed_slice(), diff --git a/boa_engine/src/environments/runtime/mod.rs b/boa_engine/src/environments/runtime/mod.rs index acc3dca751..748948125b 100644 --- a/boa_engine/src/environments/runtime/mod.rs +++ b/boa_engine/src/environments/runtime/mod.rs @@ -6,7 +6,6 @@ use crate::{ }; use boa_ast::expression::Identifier; use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace}; -use boa_interner::Sym; use rustc_hash::FxHashSet; mod declarative; @@ -486,7 +485,7 @@ impl EnvironmentStack { /// - [ECMAScript specification][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-resolve-private-identifier - pub(crate) fn resolve_private_identifier(&self, identifier: Sym) -> Option { + pub(crate) fn resolve_private_identifier(&self, identifier: JsString) -> Option { // 1. Let names be privEnv.[[Names]]. // 2. For each Private Name pn of names, do // a. If pn.[[Description]] is identifier, then @@ -503,12 +502,12 @@ impl EnvironmentStack { } /// Return all private name descriptions in all private environments. - pub(crate) fn private_name_descriptions(&self) -> Vec { + pub(crate) fn private_name_descriptions(&self) -> Vec<&JsString> { let mut names = Vec::new(); for environment in self.private_stack.iter().rev() { for name in environment.descriptions() { - if !names.contains(name) { - names.push(*name); + if !names.contains(&name) { + names.push(name); } } } diff --git a/boa_engine/src/environments/runtime/private.rs b/boa_engine/src/environments/runtime/private.rs index 20261b2ff8..60962627bb 100644 --- a/boa_engine/src/environments/runtime/private.rs +++ b/boa_engine/src/environments/runtime/private.rs @@ -1,5 +1,6 @@ use boa_gc::{empty_trace, Finalize, Trace}; -use boa_interner::Sym; + +use crate::JsString; /// Private runtime environment. #[derive(Clone, Debug, Finalize)] @@ -8,7 +9,7 @@ pub(crate) struct PrivateEnvironment { id: usize, /// The `[[Description]]` internal slot of the private names. - descriptions: Vec, + descriptions: Vec, } // Safety: PrivateEnvironment does not contain any objects that need to be traced. @@ -18,7 +19,7 @@ unsafe impl Trace for PrivateEnvironment { impl PrivateEnvironment { /// Creates a new `PrivateEnvironment`. - pub(crate) fn new(id: usize, descriptions: Vec) -> Self { + pub(crate) fn new(id: usize, descriptions: Vec) -> Self { Self { id, descriptions } } @@ -28,7 +29,7 @@ impl PrivateEnvironment { } /// Gets the descriptions of this private environment. - pub(crate) fn descriptions(&self) -> &[Sym] { + pub(crate) fn descriptions(&self) -> &[JsString] { &self.descriptions } } diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index 6d48b7bdf4..2b583a4e75 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -17,7 +17,6 @@ use crate::{ Context, JsResult, JsString, JsValue, }; use boa_gc::{self, Finalize, Gc, GcRefCell, Trace}; -use boa_interner::Sym; use std::{ cell::RefCell, collections::HashMap, @@ -947,7 +946,7 @@ Cannot both specify accessors and a value or writable attribute", } /// Create a new private name with this object as the unique identifier. - pub(crate) fn private_name(&self, description: Sym) -> PrivateName { + pub(crate) fn private_name(&self, description: JsString) -> PrivateName { let ptr: *const _ = self.as_ref(); PrivateName::new(description, ptr as usize) } diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 74f123105a..072693f79a 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -2,7 +2,6 @@ //! //! For the builtin object wrappers, please see [`object::builtins`][builtins] for implementors. -use boa_interner::Sym; pub use jsobject::{RecursionLimiter, Ref, RefMut}; pub use operations::IntegrityLevel; pub use property_map::*; @@ -165,10 +164,10 @@ unsafe impl Trace for Object { } /// A Private Name. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct PrivateName { /// The `[[Description]]` internal slot of the private name. - description: Sym, + description: JsString, /// The unique identifier of the private name. id: usize, @@ -176,7 +175,7 @@ pub struct PrivateName { impl PrivateName { /// Create a new private name. - pub(crate) const fn new(description: Sym, id: usize) -> Self { + pub(crate) const fn new(description: JsString, id: usize) -> Self { Self { description, id } } } diff --git a/boa_engine/src/object/operations.rs b/boa_engine/src/object/operations.rs index 6c061e417c..a2861c4701 100644 --- a/boa_engine/src/object/operations.rs +++ b/boa_engine/src/object/operations.rs @@ -762,7 +762,7 @@ impl JsObject { // 4. Append PrivateElement { [[Key]]: P, [[Kind]]: field, [[Value]]: value } to O.[[PrivateElements]]. self.borrow_mut() .private_elements - .push((*name, PrivateElement::Field(value))); + .push((name.clone(), PrivateElement::Field(value))); // 5. Return unused. Ok(()) @@ -811,7 +811,7 @@ impl JsObject { // 5. Append method to O.[[PrivateElements]]. self.borrow_mut() - .append_private_element(*name, method.clone()); + .append_private_element(name.clone(), method.clone()); // 6. Return unused. Ok(()) diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 9255ab213e..2acaa5b662 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -19,7 +19,7 @@ use crate::{ Context, JsError, JsResult, JsString, JsValue, }; use bitflags::bitflags; -use boa_ast::function::{FormalParameterList, PrivateName}; +use boa_ast::function::FormalParameterList; use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace}; use boa_interner::Sym; use boa_profiler::Profiler; @@ -115,13 +115,9 @@ pub struct CodeBlock { /// Literals pub(crate) literals: Box<[JsValue]>, - /// Property field names. + /// Property field names and private names `[[description]]`s. pub(crate) names: Box<[JsString]>, - /// Private names. - #[unsafe_ignore_trace] - pub(crate) private_names: Box<[PrivateName]>, - /// Locators for all bindings in the codeblock. #[unsafe_ignore_trace] pub(crate) bindings: Box<[BindingLocator]>, @@ -148,7 +144,6 @@ impl CodeBlock { bytecode: Box::default(), literals: Box::default(), names: Box::default(), - private_names: Box::default(), bindings: Box::default(), functions: Box::default(), name, @@ -399,15 +394,8 @@ impl CodeBlock { | Opcode::SetPropertySetterByName | Opcode::DefineClassStaticSetterByName | Opcode::DefineClassSetterByName - | Opcode::DeletePropertyByName => { - let operand = self.read::(*pc); - *pc += size_of::(); - format!( - "{operand:04}: '{}'", - self.names[operand as usize].to_std_string_escaped(), - ) - } - Opcode::SetPrivateField + | Opcode::DeletePropertyByName + | Opcode::SetPrivateField | Opcode::DefinePrivateField | Opcode::SetPrivateMethod | Opcode::SetPrivateSetter @@ -422,7 +410,7 @@ impl CodeBlock { *pc += size_of::(); format!( "{operand:04}: '{}'", - interner.resolve_expect(self.private_names[operand as usize].description()), + self.names[operand as usize].to_std_string_escaped(), ) } Opcode::PushPrivateEnvironment => { diff --git a/boa_engine/src/vm/opcode/binary_ops/mod.rs b/boa_engine/src/vm/opcode/binary_ops/mod.rs index b4d115cf80..b262cc7200 100644 --- a/boa_engine/src/vm/opcode/binary_ops/mod.rs +++ b/boa_engine/src/vm/opcode/binary_ops/mod.rs @@ -111,7 +111,7 @@ impl Operation for InPrivate { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let rhs = context.vm.pop(); let Some(rhs) = rhs.as_object() else { @@ -126,7 +126,7 @@ impl Operation for InPrivate { let name = context .vm .environments - .resolve_private_identifier(name.description()) + .resolve_private_identifier(name) .expect("private name must be in environment"); if rhs.private_element_find(&name, true, true).is_some() { diff --git a/boa_engine/src/vm/opcode/get/private.rs b/boa_engine/src/vm/opcode/get/private.rs index b466e371c2..2d6b1139ca 100644 --- a/boa_engine/src/vm/opcode/get/private.rs +++ b/boa_engine/src/vm/opcode/get/private.rs @@ -16,14 +16,14 @@ impl Operation for GetPrivateField { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let base_obj = value.to_object(context)?; let name = context .vm .environments - .resolve_private_identifier(name.description()) + .resolve_private_identifier(name) .expect("private name must be in environment"); let result = base_obj.private_get(&name, context)?; diff --git a/boa_engine/src/vm/opcode/push/class/field.rs b/boa_engine/src/vm/opcode/push/class/field.rs index b3df732ae4..f48bfbfece 100644 --- a/boa_engine/src/vm/opcode/push/class/field.rs +++ b/boa_engine/src/vm/opcode/push/class/field.rs @@ -58,7 +58,7 @@ impl Operation for PushClassFieldPrivate { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let field_function_value = context.vm.pop(); let class_value = context.vm.pop(); @@ -80,7 +80,7 @@ impl Operation for PushClassFieldPrivate { .as_function_mut() .expect("class must be function object") .push_field_private( - class_object.private_name(name.description()), + class_object.private_name(name), JsFunction::from_object_unchecked(field_function_object.clone()), ); Ok(CompletionType::Normal) diff --git a/boa_engine/src/vm/opcode/push/class/private.rs b/boa_engine/src/vm/opcode/push/class/private.rs index f7f07dab10..31797f39fc 100644 --- a/boa_engine/src/vm/opcode/push/class/private.rs +++ b/boa_engine/src/vm/opcode/push/class/private.rs @@ -19,11 +19,11 @@ impl Operation for PushClassPrivateMethod { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let method = context.vm.pop(); let method_object = method.as_callable().expect("method must be callable"); - let name_string = format!("#{}", context.interner().resolve_expect(name.description())); + let name_string = format!("#{}", name.to_std_string_escaped()); let desc = PropertyDescriptor::builder() .value(name_string) .writable(false) @@ -42,7 +42,7 @@ impl Operation for PushClassPrivateMethod { .as_function_mut() .expect("class must be function object") .push_private_method( - class_object.private_name(name.description()), + class_object.private_name(name), PrivateElement::Method(method_object.clone()), ); @@ -68,7 +68,7 @@ impl Operation for PushClassPrivateGetter { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let getter = context.vm.pop(); let getter_object = getter.as_callable().expect("getter must be callable"); let class = context.vm.pop(); @@ -79,7 +79,7 @@ impl Operation for PushClassPrivateGetter { .as_function_mut() .expect("class must be function object") .push_private_method( - class_object.private_name(name.description()), + class_object.private_name(name), PrivateElement::Accessor { getter: Some(getter_object.clone()), setter: None, @@ -107,7 +107,7 @@ impl Operation for PushClassPrivateSetter { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let setter = context.vm.pop(); let setter_object = setter.as_callable().expect("getter must be callable"); let class = context.vm.pop(); @@ -118,7 +118,7 @@ impl Operation for PushClassPrivateSetter { .as_function_mut() .expect("class must be function object") .push_private_method( - class_object.private_name(name.description()), + class_object.private_name(name), PrivateElement::Accessor { getter: None, setter: Some(setter_object.clone()), diff --git a/boa_engine/src/vm/opcode/push/environment.rs b/boa_engine/src/vm/opcode/push/environment.rs index 8ecd80558c..47ebd68fbf 100644 --- a/boa_engine/src/vm/opcode/push/environment.rs +++ b/boa_engine/src/vm/opcode/push/environment.rs @@ -91,8 +91,8 @@ impl Operation for PushPrivateEnvironment { let mut names = Vec::with_capacity(count as usize); for _ in 0..count { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; - names.push(name.description()); + let name = context.vm.frame().code_block.names[index as usize].clone(); + names.push(name); } let ptr: *const _ = class.as_ref(); diff --git a/boa_engine/src/vm/opcode/set/private.rs b/boa_engine/src/vm/opcode/set/private.rs index 3a5cd944c6..b5bfe61d0c 100644 --- a/boa_engine/src/vm/opcode/set/private.rs +++ b/boa_engine/src/vm/opcode/set/private.rs @@ -19,7 +19,7 @@ impl Operation for SetPrivateField { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let object = context.vm.pop(); let base_obj = object.to_object(context)?; @@ -27,7 +27,7 @@ impl Operation for SetPrivateField { let name = context .vm .environments - .resolve_private_identifier(name.description()) + .resolve_private_identifier(name) .expect("private name must be in environment"); base_obj.private_set(&name, value.clone(), context)?; @@ -49,17 +49,16 @@ impl Operation for DefinePrivateField { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let object = context.vm.pop(); let object = object .as_object() .expect("class prototype must be an object"); - object.borrow_mut().append_private_element( - object.private_name(name.description()), - PrivateElement::Field(value), - ); + object + .borrow_mut() + .append_private_element(object.private_name(name), PrivateElement::Field(value)); Ok(CompletionType::Normal) } @@ -78,11 +77,11 @@ impl Operation for SetPrivateMethod { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let value = value.as_callable().expect("method must be callable"); - let name_string = format!("#{}", context.interner().resolve_expect(name.description())); + let name_string = format!("#{}", name.to_std_string_escaped()); let desc = PropertyDescriptor::builder() .value(name_string) .writable(false) @@ -99,7 +98,7 @@ impl Operation for SetPrivateMethod { .expect("class prototype must be an object"); object.borrow_mut().append_private_element( - object.private_name(name.description()), + object.private_name(name), PrivateElement::Method(value.clone()), ); let mut value_mut = value.borrow_mut(); @@ -125,7 +124,7 @@ impl Operation for SetPrivateSetter { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let value = value.as_callable().expect("setter must be callable"); let object = context.vm.pop(); @@ -134,7 +133,7 @@ impl Operation for SetPrivateSetter { .expect("class prototype must be an object"); object.borrow_mut().append_private_element( - object.private_name(name.description()), + object.private_name(name), PrivateElement::Accessor { getter: None, setter: Some(value.clone()), @@ -163,7 +162,7 @@ impl Operation for SetPrivateGetter { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.private_names[index as usize]; + let name = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let value = value.as_callable().expect("getter must be callable"); let object = context.vm.pop(); @@ -172,7 +171,7 @@ impl Operation for SetPrivateGetter { .expect("class prototype must be an object"); object.borrow_mut().append_private_element( - object.private_name(name.description()), + object.private_name(name), PrivateElement::Accessor { getter: Some(value.clone()), setter: None,