Implement Private Runtime Environments (#2929)

boa_ast/src/function/class.rs

@@ -538,20 +538,14 @@ impl VisitWith for ClassElement {
 pub struct PrivateName {
     /// The `[[Description]]` internal slot of the private name.
     description: Sym,
-
-    /// The indices of the private name are included to ensure that private names are unique.
-    pub(crate) indices: (usize, usize),
 }
 
 impl PrivateName {
     /// Create a new private name.
     #[inline]
     #[must_use]
     pub const fn new(description: Sym) -> Self {
-        Self {
-            description,
-            indices: (0, 0),
-        }
+        Self { description }
     }
 
     /// Get the description of the private name.

boa_ast/src/operations.rs

@@ -6,22 +6,26 @@ use core::ops::ControlFlow;
 use std::convert::Infallible;
 
 use boa_interner::{Interner, Sym};
-use rustc_hash::{FxHashMap, FxHashSet};
+use rustc_hash::FxHashSet;
 
 use crate::{
     declaration::{Binding, ExportDeclaration, ImportDeclaration, VarDeclaration, Variable},
-    expression::{access::SuperPropertyAccess, Await, Identifier, SuperCall, Yield},
+    expression::{
+        access::{PrivatePropertyAccess, SuperPropertyAccess},
+        operator::BinaryInPrivate,
+        Await, Identifier, SuperCall, Yield,
+    },
     function::{
         ArrowFunction, AsyncArrowFunction, AsyncFunction, AsyncGenerator, Class, ClassElement,
-        Function, Generator, PrivateName,
+        Function, Generator,
     },
     property::{MethodDefinition, PropertyDefinition},
     statement::{
         iteration::{ForLoopInitializer, IterableLoopInitializer},
         LabelledItem,
     },
     try_break,
-    visitor::{NodeRef, VisitWith, Visitor, VisitorMut},
+    visitor::{NodeRef, VisitWith, Visitor},
     Declaration, Expression, ModuleItem, Statement, StatementList, StatementListItem,
 };
 
@@ -835,108 +839,105 @@ pub fn top_level_var_declared_names(stmts: &StatementList) -> FxHashSet<Identifi
     names
 }
 
-/// Resolves the private names of a class and all of the contained classes and private identifiers.
-pub fn class_private_name_resolver(node: &mut Class, top_level_class_index: usize) -> bool {
-    /// Visitor used by the function to search for an identifier with the name `arguments`.
-    #[derive(Debug, Clone)]
-    struct ClassPrivateNameResolver {
-        private_environments_stack: Vec<FxHashMap<Sym, PrivateName>>,
-        top_level_class_index: usize,
-    }
+/// Returns `true` if all private identifiers in a node are valid.
+///
+/// This is equivalent to the [`AllPrivateIdentifiersValid`][spec] syntax operation in the spec.
+///
+/// [spec]: https://tc39.es/ecma262/#sec-static-semantics-allprivateidentifiersvalid
+#[must_use]
+#[inline]
+pub fn all_private_identifiers_valid<'a, N>(node: &'a N, private_names: Vec<Sym>) -> bool
+where
+    &'a N: Into<NodeRef<'a>>,
+{
+    AllPrivateIdentifiersValidVisitor(private_names)
+        .visit(node.into())
+        .is_continue()
+}
 
-    impl<'ast> VisitorMut<'ast> for ClassPrivateNameResolver {
-        type BreakTy = ();
+struct AllPrivateIdentifiersValidVisitor(Vec<Sym>);
 
-        #[inline]
-        fn visit_class_mut(&mut self, node: &'ast mut Class) -> ControlFlow<Self::BreakTy> {
-            let mut names = FxHashMap::default();
-
-            for element in node.elements.iter_mut() {
-                match element {
-                    ClassElement::PrivateMethodDefinition(name, _)
-                    | ClassElement::PrivateStaticMethodDefinition(name, _)
-                    | ClassElement::PrivateFieldDefinition(name, _)
-                    | ClassElement::PrivateStaticFieldDefinition(name, _) => {
-                        name.indices = (
-                            self.top_level_class_index,
-                            self.private_environments_stack.len(),
-                        );
-                        names.insert(name.description(), *name);
-                    }
-                    _ => {}
+impl<'ast> Visitor<'ast> for AllPrivateIdentifiersValidVisitor {
+    type BreakTy = ();
+
+    fn visit_class(&mut self, node: &'ast Class) -> ControlFlow<Self::BreakTy> {
+        if let Some(node) = node.super_ref() {
+            try_break!(self.visit(node));
+        }
+
+        let mut names = self.0.clone();
+        for element in node.elements() {
+            match element {
+                ClassElement::PrivateMethodDefinition(name, _)
+                | ClassElement::PrivateStaticMethodDefinition(name, _)
+                | ClassElement::PrivateFieldDefinition(name, _)
+                | ClassElement::PrivateStaticFieldDefinition(name, _) => {
+                    names.push(name.description());
                 }
+                _ => {}
             }
+        }
 
-            self.private_environments_stack.push(names);
+        let mut visitor = Self(names);
 
-            for element in node.elements.iter_mut() {
-                match element {
-                    ClassElement::MethodDefinition(name, method)
-                    | ClassElement::StaticMethodDefinition(name, method) => {
-                        try_break!(self.visit_property_name_mut(name));
-                        try_break!(self.visit_method_definition_mut(method));
-                    }
-                    ClassElement::PrivateMethodDefinition(_, method)
-                    | ClassElement::PrivateStaticMethodDefinition(_, method) => {
-                        try_break!(self.visit_method_definition_mut(method));
-                    }
-                    ClassElement::FieldDefinition(name, expression)
-                    | ClassElement::StaticFieldDefinition(name, expression) => {
-                        try_break!(self.visit_property_name_mut(name));
-                        if let Some(expression) = expression {
-                            try_break!(self.visit_expression_mut(expression));
-                        }
-                    }
-                    ClassElement::PrivateFieldDefinition(_, expression)
-                    | ClassElement::PrivateStaticFieldDefinition(_, expression) => {
-                        if let Some(expression) = expression {
-                            try_break!(self.visit_expression_mut(expression));
-                        }
+        if let Some(node) = node.constructor() {
+            try_break!(visitor.visit(node));
+        }
+
+        for element in node.elements() {
+            match element {
+                ClassElement::MethodDefinition(name, method)
+                | ClassElement::StaticMethodDefinition(name, method) => {
+                    try_break!(visitor.visit(name));
+                    try_break!(visitor.visit(method));
+                }
+                ClassElement::FieldDefinition(name, expression)
+                | ClassElement::StaticFieldDefinition(name, expression) => {
+                    try_break!(visitor.visit(name));
+                    if let Some(expression) = expression {
+                        try_break!(visitor.visit(expression));
                     }
-                    ClassElement::StaticBlock(statement_list) => {
-                        try_break!(self.visit_statement_list_mut(statement_list));
+                }
+                ClassElement::PrivateMethodDefinition(_, method)
+                | ClassElement::PrivateStaticMethodDefinition(_, method) => {
+                    try_break!(visitor.visit(method));
+                }
+                ClassElement::PrivateFieldDefinition(_, expression)
+                | ClassElement::PrivateStaticFieldDefinition(_, expression) => {
+                    if let Some(expression) = expression {
+                        try_break!(visitor.visit(expression));
                     }
                 }
+                ClassElement::StaticBlock(statement_list) => {
+                    try_break!(visitor.visit(statement_list));
+                }
             }
-
-            if let Some(function) = &mut node.constructor {
-                try_break!(self.visit_function_mut(function));
-            }
-
-            self.private_environments_stack.pop();
-
-            ControlFlow::Continue(())
         }
 
-        #[inline]
-        fn visit_private_name_mut(
-            &mut self,
-            node: &'ast mut PrivateName,
-        ) -> ControlFlow<Self::BreakTy> {
-            let mut found = false;
-
-            for environment in self.private_environments_stack.iter().rev() {
-                if let Some(n) = environment.get(&node.description()) {
-                    found = true;
-                    node.indices = n.indices;
-                    break;
-                }
-            }
+        ControlFlow::Continue(())
+    }
 
-            if found {
-                ControlFlow::Continue(())
-            } else {
-                ControlFlow::Break(())
-            }
+    fn visit_private_property_access(
+        &mut self,
+        node: &'ast PrivatePropertyAccess,
+    ) -> ControlFlow<Self::BreakTy> {
+        if self.0.contains(&node.field().description()) {
+            self.visit(node.target())
+        } else {
+            ControlFlow::Break(())
         }
     }
 
-    let mut visitor = ClassPrivateNameResolver {
-        private_environments_stack: Vec::new(),
-        top_level_class_index,
-    };
-
-    visitor.visit_class_mut(node).is_continue()
+    fn visit_binary_in_private(
+        &mut self,
+        node: &'ast BinaryInPrivate,
+    ) -> ControlFlow<Self::BreakTy> {
+        if self.0.contains(&node.lhs().description()) {
+            self.visit(node.rhs())
+        } else {
+            ControlFlow::Break(())
+        }
+    }
 }
 
 /// Errors that can occur when checking labels.

boa_engine/src/builtins/function/mod.rs

@@ -12,15 +12,15 @@
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function
 
 use crate::{
-    builtins::BuiltInObject,
+    builtins::{BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject},
     bytecompiler::FunctionCompiler,
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
-    environments::EnvironmentStack,
+    environments::{EnvironmentStack, PrivateEnvironment},
     error::JsNativeError,
     js_string,
     native_function::NativeFunction,
     object::{internal_methods::get_prototype_from_constructor, JsObject, Object, ObjectData},
-    object::{JsFunction, PrivateElement},
+    object::{JsFunction, PrivateElement, PrivateName},
     property::{Attribute, PropertyDescriptor, PropertyKey},
     realm::Realm,
     string::utf16,
@@ -30,21 +30,22 @@ use crate::{
     Context, JsArgs, JsResult, JsString, JsValue,
 };
 use boa_ast::{
-    function::{FormalParameterList, PrivateName},
-    operations::{bound_names, contains, lexically_declared_names, ContainsSymbol},
+    function::FormalParameterList,
+    operations::{
+        all_private_identifiers_valid, bound_names, contains, lexically_declared_names,
+        ContainsSymbol,
+    },
     StatementList,
 };
 use boa_gc::{self, custom_trace, Finalize, Gc, Trace};
 use boa_interner::Sym;
 use boa_parser::{Parser, Source};
 use boa_profiler::Profiler;
-use thin_vec::ThinVec;
-
 use std::{fmt, io::Read};
-
-use super::{BuiltInBuilder, BuiltInConstructor, IntrinsicObject};
+use thin_vec::ThinVec;
 
 pub(crate) mod arguments;
+
 #[cfg(test)]
 mod tests;
 
@@ -308,6 +309,13 @@ impl Function {
         Self { kind, realm }
     }
 
+    /// Push a private environment to the function.
+    pub(crate) fn push_private_environment(&mut self, environment: Gc<PrivateEnvironment>) {
+        if let FunctionKind::Ordinary { environments, .. } = &mut self.kind {
+            environments.push_private(environment);
+        }
+    }
+
     /// Returns true if the function object is a derived constructor.
     pub(crate) const fn is_derived_constructor(&self) -> bool {
         if let FunctionKind::Ordinary {
@@ -368,9 +376,9 @@ impl Function {
     }
 
     /// Pushes a private value to the `[[Fields]]` internal slot if present.
-    pub(crate) fn push_field_private(&mut self, key: PrivateName, value: JsFunction) {
+    pub(crate) fn push_field_private(&mut self, name: PrivateName, value: JsFunction) {
         if let FunctionKind::Ordinary { fields, .. } = &mut self.kind {
-            fields.push(ClassFieldDefinition::Private(key, value));
+            fields.push(ClassFieldDefinition::Private(name, value));
         }
     }
 
@@ -705,6 +713,18 @@ impl BuiltInFunctionObject {
                 }
             }
 
+            if !all_private_identifiers_valid(&parameters, Vec::new()) {
+                return Err(JsNativeError::syntax()
+                    .with_message("invalid private identifier usage")
+                    .into());
+            }
+
+            if !all_private_identifiers_valid(&body, Vec::new()) {
+                return Err(JsNativeError::syntax()
+                    .with_message("invalid private identifier usage")
+                    .into());
+            }
+
             let code = FunctionCompiler::new()
                 .name(Sym::ANONYMOUS)
                 .generator(generator)

boa_engine/src/bytecompiler/class.rs

@@ -100,6 +100,23 @@ impl ByteCompiler<'_, '_> {
         self.emit_opcode(Opcode::SetClassPrototype);
         self.emit_opcode(Opcode::Swap);
 
+        let count_label = self.emit_opcode_with_operand(Opcode::PushPrivateEnvironment);
+        let mut count = 0;
+        for element in class.elements() {
+            match element {
+                ClassElement::PrivateMethodDefinition(name, _)
+                | ClassElement::PrivateStaticMethodDefinition(name, _)
+                | ClassElement::PrivateFieldDefinition(name, _)
+                | ClassElement::PrivateStaticFieldDefinition(name, _) => {
+                    count += 1;
+                    let index = self.get_or_insert_private_name(*name);
+                    self.emit_u32(index);
+                }
+                _ => {}
+            }
+        }
+        self.patch_jump_with_target(count_label, count);
+
         // TODO: set function name for getter and setters
         for element in class.elements() {
             match element {
@@ -536,6 +553,8 @@ impl ByteCompiler<'_, '_> {
             self.emit_opcode(Opcode::PopEnvironment);
         }
 
+        self.emit_opcode(Opcode::PopPrivateEnvironment);
+
         if !expression {
             self.emit_binding(
                 BindingOpcode::InitVar,