Merge `private_names` into `names` constant pool (#3023)

* Merge `private_names` into `names` constant pool

* Fix clippy lint

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) {

boa_engine/src/bytecompiler/mod.rs

@@ -231,12 +231,9 @@ pub struct ByteCompiler<'ctx, 'host> {
     /// Literals
     pub(crate) literals: Vec<JsValue>,
 
-    /// Property field names.
+    /// Property field names and private name `[[Description]]`s.
     pub(crate) names: Vec<JsString>,
 
-    /// Private names.
-    pub(crate) private_names: Vec<PrivateName>,
-
     /// Locators for all bindings in the codeblock.
     pub(crate) bindings: Vec<BindingLocator>,
 
@@ -256,7 +253,6 @@ pub struct ByteCompiler<'ctx, 'host> {
 
     literals_map: FxHashMap<Literal, u32>,
     names_map: FxHashMap<Identifier, u32>,
-    private_names_map: FxHashMap<PrivateName, u32>,
     bindings_map: FxHashMap<BindingLocator, u32>,
     jump_info: Vec<JumpControlInfo>,
     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(),

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<PrivateName> {
+    pub(crate) fn resolve_private_identifier(&self, identifier: JsString) -> Option<PrivateName> {
         // 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<Sym> {
+    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);
                 }
             }
         }

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<Sym>,
+    descriptions: Vec<JsString>,
 }
 
 // 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<Sym>) -> Self {
+    pub(crate) fn new(id: usize, descriptions: Vec<JsString>) -> 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
     }
 }

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)
     }

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 }
     }
 }

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(())

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::<u32>(*pc);
-                *pc += size_of::<u32>();
-                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::<u32>();
                 format!(
                     "{operand:04}: '{}'",
-                    interner.resolve_expect(self.private_names[operand as usize].description()),
+                    self.names[operand as usize].to_std_string_escaped(),
                 )
             }
             Opcode::PushPrivateEnvironment => {

boa_engine/src/vm/opcode/binary_ops/mod.rs

@@ -111,7 +111,7 @@ impl Operation for InPrivate {
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
-        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() {

boa_engine/src/vm/opcode/get/private.rs

@@ -16,14 +16,14 @@ impl Operation for GetPrivateField {
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
-        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)?;

boa_engine/src/vm/opcode/push/class/field.rs

@@ -58,7 +58,7 @@ impl Operation for PushClassFieldPrivate {
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
-        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)

boa_engine/src/vm/opcode/push/class/private.rs

@@ -19,11 +19,11 @@ impl Operation for PushClassPrivateMethod {
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
-        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<CompletionType> {
         let index = context.vm.read::<u32>();
-        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<CompletionType> {
         let index = context.vm.read::<u32>();
-        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()),

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::<u32>();
-            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();

boa_engine/src/vm/opcode/set/private.rs

@@ -19,7 +19,7 @@ impl Operation for SetPrivateField {
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
-        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<CompletionType> {
         let index = context.vm.read::<u32>();
-        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<CompletionType> {
         let index = context.vm.read::<u32>();
-        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<CompletionType> {
         let index = context.vm.read::<u32>();
-        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<CompletionType> {
         let index = context.vm.read::<u32>();
-        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,