From 7c466f7a2ef1bc18fc345138b97966ac4fb268f4 Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Sat, 13 May 2023 04:52:27 +0200 Subject: [PATCH] Implement Private Runtime Environments --- boa_ast/src/function/class.rs | 8 +- boa_ast/src/operations.rs | 181 +++++++++--------- boa_engine/src/builtins/function/mod.rs | 42 ++-- boa_engine/src/bytecompiler/class.rs | 19 ++ boa_engine/src/bytecompiler/declarations.rs | 15 +- boa_engine/src/environments/mod.rs | 2 +- boa_engine/src/environments/runtime/mod.rs | 65 ++++++- .../src/environments/runtime/private.rs | 34 ++++ boa_engine/src/object/jsobject.rs | 9 +- boa_engine/src/object/mod.rs | 19 +- boa_engine/src/object/operations.rs | 5 +- boa_engine/src/vm/code_block.rs | 6 + boa_engine/src/vm/flowgraph/mod.rs | 9 +- boa_engine/src/vm/opcode/binary_ops/mod.rs | 8 +- boa_engine/src/vm/opcode/get/private.rs | 7 + boa_engine/src/vm/opcode/mod.rs | 14 ++ boa_engine/src/vm/opcode/push/class/field.rs | 3 +- .../src/vm/opcode/push/class/private.rs | 12 +- boa_engine/src/vm/opcode/push/environment.rs | 59 ++++++ boa_engine/src/vm/opcode/set/private.rs | 27 ++- boa_parser/src/parser/cursor/mod.rs | 36 ---- .../parser/expression/left_hand_side/call.rs | 6 - .../expression/left_hand_side/member.rs | 6 - .../expression/left_hand_side/optional/mod.rs | 14 +- boa_parser/src/parser/expression/mod.rs | 8 - boa_parser/src/parser/mod.rs | 16 +- .../declaration/hoistable/class_decl/mod.rs | 22 +-- boa_parser/src/parser/statement/mod.rs | 14 +- 28 files changed, 438 insertions(+), 228 deletions(-) create mode 100644 boa_engine/src/environments/runtime/private.rs diff --git a/boa_ast/src/function/class.rs b/boa_ast/src/function/class.rs index 35f5456226c..7fb02e29c4c 100644 --- a/boa_ast/src/function/class.rs +++ b/boa_ast/src/function/class.rs @@ -538,9 +538,6 @@ 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 { @@ -548,10 +545,7 @@ impl PrivateName { #[inline] #[must_use] pub const fn new(description: Sym) -> Self { - Self { - description, - indices: (0, 0), - } + Self { description } } /// Get the description of the private name. diff --git a/boa_ast/src/operations.rs b/boa_ast/src/operations.rs index 60bc3e0a265..306f9a37918 100644 --- a/boa_ast/src/operations.rs +++ b/boa_ast/src/operations.rs @@ -6,14 +6,18 @@ 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::{ @@ -21,7 +25,7 @@ use crate::{ 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 bool { - /// Visitor used by the function to search for an identifier with the name `arguments`. - #[derive(Debug, Clone)] - struct ClassPrivateNameResolver { - private_environments_stack: Vec>, - 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) -> bool +where + &'a N: Into>, +{ + AllPrivateIdentifiersValidVisitor(private_names) + .visit(node.into()) + .is_continue() +} - impl<'ast> VisitorMut<'ast> for ClassPrivateNameResolver { - type BreakTy = (); +struct AllPrivateIdentifiersValidVisitor(Vec); - #[inline] - fn visit_class_mut(&mut self, node: &'ast mut Class) -> ControlFlow { - 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 { + 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 { - 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 { + 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 { + if self.0.contains(&node.lhs().description()) { + self.visit(node.rhs()) + } else { + ControlFlow::Break(()) + } + } } /// Errors that can occur when checking labels. diff --git a/boa_engine/src/builtins/function/mod.rs b/boa_engine/src/builtins/function/mod.rs index 6b30c913e58..a7e70f09828 100644 --- a/boa_engine/src/builtins/function/mod.rs +++ b/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) { + 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(¶meters, 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) diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index 60bb3e26551..84a9e3828f6 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/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, diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index 7fdb5a45669..d6101326815 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -7,8 +7,9 @@ use boa_ast::{ declaration::{Binding, LexicalDeclaration, VariableList}, function::FormalParameterList, operations::{ - bound_names, lexically_scoped_declarations, top_level_lexically_declared_names, - top_level_var_declared_names, top_level_var_scoped_declarations, VarScopedDeclaration, + all_private_identifiers_valid, bound_names, lexically_scoped_declarations, + top_level_lexically_declared_names, top_level_var_declared_names, + top_level_var_scoped_declarations, VarScopedDeclaration, }, visitor::NodeRef, Declaration, StatementList, StatementListItem, @@ -428,9 +429,17 @@ impl ByteCompiler<'_, '_> { // 5. Let pointer be privateEnv. // 6. Repeat, while pointer is not null, // a. For each Private Name binding of pointer.[[Names]], do - // i. If privateIdentifiers does not contain binding.[[Description]], append binding.[[Description]] to privateIdentifiers. + // i. If privateIdentifiers does not contain binding.[[Description]], + // append binding.[[Description]] to privateIdentifiers. // b. Set pointer to pointer.[[OuterPrivateEnvironment]]. + let private_identifiers = self.context.vm.environments.private_name_descriptions(); + // 7. If AllPrivateIdentifiersValid of body with argument privateIdentifiers is false, throw a SyntaxError exception. + if !all_private_identifiers_valid(body, private_identifiers) { + return Err(JsNativeError::syntax() + .with_message("invalid private identifier") + .into()); + } // 8. Let functionsToInitialize be a new empty List. let mut functions_to_initialize = Vec::new(); diff --git a/boa_engine/src/environments/mod.rs b/boa_engine/src/environments/mod.rs index 616b9e3c2dc..458ea04e70b 100644 --- a/boa_engine/src/environments/mod.rs +++ b/boa_engine/src/environments/mod.rs @@ -31,7 +31,7 @@ pub(crate) use { compile::CompileTimeEnvironment, runtime::{ BindingLocator, DeclarativeEnvironment, Environment, EnvironmentStack, FunctionSlots, - ThisBindingStatus, + PrivateEnvironment, ThisBindingStatus, }, }; diff --git a/boa_engine/src/environments/runtime/mod.rs b/boa_engine/src/environments/runtime/mod.rs index a537779bb2b..198bf05cc8c 100644 --- a/boa_engine/src/environments/runtime/mod.rs +++ b/boa_engine/src/environments/runtime/mod.rs @@ -1,17 +1,24 @@ use crate::{ - environments::CompileTimeEnvironment, error::JsNativeError, object::JsObject, Context, - JsResult, JsString, JsSymbol, JsValue, + environments::CompileTimeEnvironment, + error::JsNativeError, + object::{JsObject, PrivateName}, + Context, JsResult, JsString, JsSymbol, JsValue, }; use boa_ast::expression::Identifier; use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace}; +use boa_interner::Sym; use rustc_hash::FxHashSet; mod declarative; +mod private; use self::declarative::ModuleEnvironment; -pub(crate) use self::declarative::{ - DeclarativeEnvironment, DeclarativeEnvironmentKind, FunctionEnvironment, FunctionSlots, - LexicalEnvironment, ThisBindingStatus, +pub(crate) use self::{ + declarative::{ + DeclarativeEnvironment, DeclarativeEnvironmentKind, FunctionEnvironment, FunctionSlots, + LexicalEnvironment, ThisBindingStatus, + }, + private::PrivateEnvironment, }; /// The environment stack holds all environments at runtime. @@ -21,6 +28,8 @@ pub(crate) use self::declarative::{ #[derive(Clone, Debug, Trace, Finalize)] pub(crate) struct EnvironmentStack { stack: Vec, + + private_stack: Vec>, } /// A runtime environment. @@ -56,6 +65,7 @@ impl EnvironmentStack { )); Self { stack: vec![Environment::Declarative(global)], + private_stack: Vec::new(), } } @@ -457,6 +467,51 @@ impl EnvironmentStack { env.set(binding_index, value); } } + + /// Push a private environment to the private environment stack. + pub(crate) fn push_private(&mut self, environment: Gc) { + self.private_stack.push(environment); + } + + /// Pop a private environment from the private environment stack. + pub(crate) fn pop_private(&mut self) { + self.private_stack.pop(); + } + + /// `ResolvePrivateIdentifier ( privEnv, identifier )` + /// + /// More information: + /// - [ECMAScript specification][spec] + /// + /// [spec]: https://tc39.es/ecma262/#sec-resolve-private-identifier + pub(crate) fn resolve_private_identifier(&self, identifier: Sym) -> Option { + // 1. Let names be privEnv.[[Names]]. + // 2. For each Private Name pn of names, do + // a. If pn.[[Description]] is identifier, then + // i. Return pn. + // 3. Let outerPrivEnv be privEnv.[[OuterPrivateEnvironment]]. + // 4. Assert: outerPrivEnv is not null. + // 5. Return ResolvePrivateIdentifier(outerPrivEnv, identifier). + for environment in self.private_stack.iter().rev() { + if environment.descriptions().contains(&identifier) { + return Some(PrivateName::new(identifier, environment.id())); + } + } + None + } + + /// Return all private name descriptions in all private environments. + pub(crate) fn private_name_descriptions(&self) -> Vec { + 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); + } + } + } + names + } } /// A binding locator contains all information about a binding that is needed to resolve it at runtime. diff --git a/boa_engine/src/environments/runtime/private.rs b/boa_engine/src/environments/runtime/private.rs new file mode 100644 index 00000000000..20261b2ff83 --- /dev/null +++ b/boa_engine/src/environments/runtime/private.rs @@ -0,0 +1,34 @@ +use boa_gc::{empty_trace, Finalize, Trace}; +use boa_interner::Sym; + +/// Private runtime environment. +#[derive(Clone, Debug, Finalize)] +pub(crate) struct PrivateEnvironment { + /// The unique identifier of the private names. + id: usize, + + /// The `[[Description]]` internal slot of the private names. + descriptions: Vec, +} + +// Safety: PrivateEnvironment does not contain any objects that need to be traced. +unsafe impl Trace for PrivateEnvironment { + empty_trace!(); +} + +impl PrivateEnvironment { + /// Creates a new `PrivateEnvironment`. + pub(crate) fn new(id: usize, descriptions: Vec) -> Self { + Self { id, descriptions } + } + + /// Gets the id of this private environment. + pub(crate) const fn id(&self) -> usize { + self.id + } + + /// Gets the descriptions of this private environment. + pub(crate) fn descriptions(&self) -> &[Sym] { + &self.descriptions + } +} diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index df62ce1b068..b2e6f51c7dc 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -5,7 +5,7 @@ use super::{ internal_methods::{InternalObjectMethods, ARRAY_EXOTIC_INTERNAL_METHODS}, shape::{shared_shape::SharedShape, Shape}, - JsPrototype, NativeObject, Object, PropertyMap, + JsPrototype, NativeObject, Object, PrivateName, PropertyMap, }; use crate::{ context::intrinsics::Intrinsics, @@ -17,6 +17,7 @@ use crate::{ Context, JsResult, JsValue, }; use boa_gc::{self, Finalize, Gc, GcRefCell, Trace}; +use boa_interner::Sym; use std::{ cell::RefCell, collections::HashMap, @@ -944,6 +945,12 @@ Cannot both specify accessors and a value or writable attribute", pub(crate) const fn inner(&self) -> &Gc { &self.inner } + + /// Create a new private name with this object as the unique identifier. + pub(crate) fn private_name(&self, description: Sym) -> PrivateName { + let ptr: *const _ = self.as_ref(); + PrivateName::new(description, ptr as usize) + } } impl AsRef> for JsObject { diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 39033caeedc..74f123105ac 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -2,7 +2,7 @@ //! //! For the builtin object wrappers, please see [`object::builtins`][builtins] for implementors. -use boa_ast::function::PrivateName; +use boa_interner::Sym; pub use jsobject::{RecursionLimiter, Ref, RefMut}; pub use operations::IntegrityLevel; pub use property_map::*; @@ -164,6 +164,23 @@ unsafe impl Trace for Object { }); } +/// A Private Name. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PrivateName { + /// The `[[Description]]` internal slot of the private name. + description: Sym, + + /// The unique identifier of the private name. + id: usize, +} + +impl PrivateName { + /// Create a new private name. + pub(crate) const fn new(description: Sym, id: usize) -> Self { + Self { description, id } + } +} + /// The representation of private object elements. #[derive(Clone, Debug, Trace, Finalize)] pub enum PrivateElement { diff --git a/boa_engine/src/object/operations.rs b/boa_engine/src/object/operations.rs index 80b8025f47e..6c061e417ce 100644 --- a/boa_engine/src/object/operations.rs +++ b/boa_engine/src/object/operations.rs @@ -2,16 +2,13 @@ use crate::{ builtins::{function::ClassFieldDefinition, Array}, context::intrinsics::{StandardConstructor, StandardConstructors}, error::JsNativeError, - object::{JsObject, PrivateElement, PROTOTYPE}, + object::{JsFunction, JsObject, PrivateElement, PrivateName, CONSTRUCTOR, PROTOTYPE}, property::{PropertyDescriptor, PropertyDescriptorBuilder, PropertyKey, PropertyNameKind}, realm::Realm, string::utf16, value::Type, Context, JsResult, JsSymbol, JsValue, }; -use boa_ast::function::PrivateName; - -use super::{JsFunction, CONSTRUCTOR}; /// Object integrity level. #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 452c0dd972b..6428d6ecae6 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -391,6 +391,11 @@ impl CodeBlock { interner.resolve_expect(self.private_names[operand as usize].description()), ) } + Opcode::PushPrivateEnvironment => { + let count = self.read::(*pc); + *pc += size_of::() * (count as usize + 1); + String::new() + } Opcode::Pop | Opcode::PopIfThrown | Opcode::Dup @@ -505,6 +510,7 @@ impl CodeBlock { | Opcode::PushObjectEnvironment | Opcode::IsObject | Opcode::SetNameByLocator + | Opcode::PopPrivateEnvironment | Opcode::Nop => String::new(), } } diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index eb70abc6ca0..817475b6840 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -478,6 +478,12 @@ impl CodeBlock { ); } } + Opcode::PushPrivateEnvironment => { + let count = self.read::(pc); + pc += size_of::() * (count as usize + 1); + graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); + graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); + } Opcode::Pop | Opcode::PopIfThrown | Opcode::Dup @@ -588,7 +594,8 @@ impl CodeBlock { | Opcode::IsObject | Opcode::SetNameByLocator | Opcode::Nop - | Opcode::PushObjectEnvironment => { + | Opcode::PushObjectEnvironment + | Opcode::PopPrivateEnvironment => { graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); } diff --git a/boa_engine/src/vm/opcode/binary_ops/mod.rs b/boa_engine/src/vm/opcode/binary_ops/mod.rs index 3babc85736a..b4d115cf80c 100644 --- a/boa_engine/src/vm/opcode/binary_ops/mod.rs +++ b/boa_engine/src/vm/opcode/binary_ops/mod.rs @@ -122,12 +122,18 @@ impl Operation for InPrivate { )) .into()); }; + + let name = context + .vm + .environments + .resolve_private_identifier(name.description()) + .expect("private name must be in environment"); + if rhs.private_element_find(&name, true, true).is_some() { context.vm.push(true); } else { context.vm.push(false); } - Ok(CompletionType::Normal) } } diff --git a/boa_engine/src/vm/opcode/get/private.rs b/boa_engine/src/vm/opcode/get/private.rs index e869618c818..b466e371c28 100644 --- a/boa_engine/src/vm/opcode/get/private.rs +++ b/boa_engine/src/vm/opcode/get/private.rs @@ -19,6 +19,13 @@ impl Operation for GetPrivateField { let name = context.vm.frame().code_block.private_names[index as usize]; let value = context.vm.pop(); let base_obj = value.to_object(context)?; + + let name = context + .vm + .environments + .resolve_private_identifier(name.description()) + .expect("private name must be in environment"); + let result = base_obj.private_get(&name, context)?; context.vm.push(result); Ok(CompletionType::Normal) diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 0a57ec89852..bdab2afcc07 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -1619,6 +1619,20 @@ generate_impl! { /// Stack: count * (cooked_value, raw_value) **=>** template TemplateCreate, + /// Push a private environment. + /// + /// Operands: count: `u32`, count * private_name_index: `u32` + /// + /// Stack: class **=>** class + PushPrivateEnvironment, + + /// Pop a private environment. + /// + /// Operands: + /// + /// Stack: **=>** + PopPrivateEnvironment, + /// No-operation instruction, does nothing. /// /// Operands: diff --git a/boa_engine/src/vm/opcode/push/class/field.rs b/boa_engine/src/vm/opcode/push/class/field.rs index 17359b6f268..b3df732ae4d 100644 --- a/boa_engine/src/vm/opcode/push/class/field.rs +++ b/boa_engine/src/vm/opcode/push/class/field.rs @@ -74,12 +74,13 @@ impl Operation for PushClassFieldPrivate { .expect("class must be function object"); field_function.set_home_object(class_object.clone()); field_function.set_class_object(class_object.clone()); + class_object .borrow_mut() .as_function_mut() .expect("class must be function object") .push_field_private( - name, + class_object.private_name(name.description()), 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 407f0ea621a..f7f07dab102 100644 --- a/boa_engine/src/vm/opcode/push/class/private.rs +++ b/boa_engine/src/vm/opcode/push/class/private.rs @@ -36,11 +36,15 @@ impl Operation for PushClassPrivateMethod { let class = context.vm.pop(); let class_object = class.as_object().expect("class must be function object"); + class_object .borrow_mut() .as_function_mut() .expect("class must be function object") - .push_private_method(name, PrivateElement::Method(method_object.clone())); + .push_private_method( + class_object.private_name(name.description()), + PrivateElement::Method(method_object.clone()), + ); let mut method_object_mut = method_object.borrow_mut(); let function = method_object_mut @@ -69,12 +73,13 @@ impl Operation for PushClassPrivateGetter { let getter_object = getter.as_callable().expect("getter must be callable"); let class = context.vm.pop(); let class_object = class.as_object().expect("class must be function object"); + class_object .borrow_mut() .as_function_mut() .expect("class must be function object") .push_private_method( - name, + class_object.private_name(name.description()), PrivateElement::Accessor { getter: Some(getter_object.clone()), setter: None, @@ -107,12 +112,13 @@ impl Operation for PushClassPrivateSetter { let setter_object = setter.as_callable().expect("getter must be callable"); let class = context.vm.pop(); let class_object = class.as_object().expect("class must be function object"); + class_object .borrow_mut() .as_function_mut() .expect("class must be function object") .push_private_method( - name, + class_object.private_name(name.description()), 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 a3408d44688..dc9091fd660 100644 --- a/boa_engine/src/vm/opcode/push/environment.rs +++ b/boa_engine/src/vm/opcode/push/environment.rs @@ -1,7 +1,9 @@ use crate::{ + environments::PrivateEnvironment, vm::{opcode::Operation, CompletionType}, Context, JsResult, }; +use boa_gc::Gc; /// `PushDeclarativeEnvironment` implements the Opcode Operation for `Opcode::PushDeclarativeEnvironment` /// @@ -74,3 +76,60 @@ impl Operation for PushObjectEnvironment { Ok(CompletionType::Normal) } } + +/// `PushPrivateEnvironment` implements the Opcode Operation for `Opcode::PushPrivateEnvironment` +/// +/// Operation: +/// - Push a private environment. +#[derive(Debug, Clone, Copy)] +pub(crate) struct PushPrivateEnvironment; + +impl Operation for PushPrivateEnvironment { + const NAME: &'static str = "PushPrivateEnvironment"; + const INSTRUCTION: &'static str = "INST - PushPrivateEnvironment"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let class_value = context.vm.pop(); + let class = class_value.to_object(context)?; + + let count = context.vm.read::(); + 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 ptr: *const _ = class.as_ref(); + let environment = Gc::new(PrivateEnvironment::new(ptr as usize, names)); + + class + .borrow_mut() + .as_function_mut() + .expect("class object must be function") + .push_private_environment(environment.clone()); + context.vm.environments.push_private(environment); + + context.vm.push(class_value); + + Ok(CompletionType::Normal) + } +} + +/// `PopPrivateEnvironment` implements the Opcode Operation for `Opcode::PopPrivateEnvironment` +/// +/// Operation: +/// - Pop a private environment. +#[derive(Debug, Clone, Copy)] +pub(crate) struct PopPrivateEnvironment; + +impl Operation for PopPrivateEnvironment { + const NAME: &'static str = "PopPrivateEnvironment"; + const INSTRUCTION: &'static str = "INST - PopPrivateEnvironment"; + + fn execute(context: &mut Context<'_>) -> JsResult { + context.vm.environments.pop_private(); + + Ok(CompletionType::Normal) + } +} diff --git a/boa_engine/src/vm/opcode/set/private.rs b/boa_engine/src/vm/opcode/set/private.rs index 388e3d636fe..3a5cd944c60 100644 --- a/boa_engine/src/vm/opcode/set/private.rs +++ b/boa_engine/src/vm/opcode/set/private.rs @@ -23,6 +23,13 @@ impl Operation for SetPrivateField { let value = context.vm.pop(); let object = context.vm.pop(); let base_obj = object.to_object(context)?; + + let name = context + .vm + .environments + .resolve_private_identifier(name.description()) + .expect("private name must be in environment"); + base_obj.private_set(&name, value.clone(), context)?; context.vm.push(value); Ok(CompletionType::Normal) @@ -48,9 +55,11 @@ impl Operation for DefinePrivateField { let object = object .as_object() .expect("class prototype must be an object"); - object - .borrow_mut() - .append_private_element(name, PrivateElement::Field(value)); + + object.borrow_mut().append_private_element( + object.private_name(name.description()), + PrivateElement::Field(value), + ); Ok(CompletionType::Normal) } @@ -88,9 +97,11 @@ impl Operation for SetPrivateMethod { let object = object .as_object() .expect("class prototype must be an object"); - object - .borrow_mut() - .append_private_element(name, PrivateElement::Method(value.clone())); + + object.borrow_mut().append_private_element( + object.private_name(name.description()), + PrivateElement::Method(value.clone()), + ); let mut value_mut = value.borrow_mut(); let function = value_mut .as_function_mut() @@ -123,7 +134,7 @@ impl Operation for SetPrivateSetter { .expect("class prototype must be an object"); object.borrow_mut().append_private_element( - name, + object.private_name(name.description()), PrivateElement::Accessor { getter: None, setter: Some(value.clone()), @@ -161,7 +172,7 @@ impl Operation for SetPrivateGetter { .expect("class prototype must be an object"); object.borrow_mut().append_private_element( - name, + object.private_name(name.description()), PrivateElement::Accessor { getter: Some(value.clone()), setter: None, diff --git a/boa_parser/src/parser/cursor/mod.rs b/boa_parser/src/parser/cursor/mod.rs index 0b91a3cb060..0c301442769 100644 --- a/boa_parser/src/parser/cursor/mod.rs +++ b/boa_parser/src/parser/cursor/mod.rs @@ -25,12 +25,6 @@ pub(super) enum SemicolonResult<'s> { pub(super) struct Cursor { buffered_lexer: BufferedLexer, - /// Tracks the number of nested private environments that the cursor is in. - private_environment_nested_index: usize, - - /// Tracks the number of private environments on the root level of the code that is parsed. - private_environment_root_index: usize, - /// Tracks if the cursor is in a arrow function declaration. arrow: bool, @@ -53,8 +47,6 @@ where pub(super) fn new(reader: R) -> Self { Self { buffered_lexer: Lexer::new(reader).into(), - private_environment_nested_index: 0, - private_environment_root_index: 0, arrow: false, json_parse: false, identifier: 0, @@ -150,34 +142,6 @@ where self.json_parse = json_parse; } - /// Push a new private environment. - #[inline] - pub(super) fn push_private_environment(&mut self) { - if !self.in_class() { - self.private_environment_root_index += 1; - } - - self.private_environment_nested_index += 1; - } - - /// Pop a private environment. - #[inline] - pub(super) fn pop_private_environment(&mut self) { - self.private_environment_nested_index -= 1; - } - - /// Returns the current private environment root index. - #[inline] - pub(super) const fn private_environment_root_index(&self) -> usize { - self.private_environment_root_index - } - - /// Returns if the cursor is in a class. - #[inline] - pub(super) const fn in_class(&self) -> bool { - self.private_environment_nested_index != 0 - } - /// Set the identifier of the cursor. #[inline] pub(super) fn set_identifier(&mut self, identifier: u32) { diff --git a/boa_parser/src/parser/expression/left_hand_side/call.rs b/boa_parser/src/parser/expression/left_hand_side/call.rs index 2e91a4cc66e..ea2ef195307 100644 --- a/boa_parser/src/parser/expression/left_hand_side/call.rs +++ b/boa_parser/src/parser/expression/left_hand_side/call.rs @@ -112,12 +112,6 @@ where } TokenKind::NullLiteral => SimplePropertyAccess::new(lhs, Sym::NULL).into(), TokenKind::PrivateIdentifier(name) => { - if !cursor.in_class() { - return Err(Error::general( - "Private identifier outside of class", - token.span().start(), - )); - } PrivatePropertyAccess::new(lhs, PrivateName::new(*name)).into() } _ => { diff --git a/boa_parser/src/parser/expression/left_hand_side/member.rs b/boa_parser/src/parser/expression/left_hand_side/member.rs index e42c34fe729..206c28fce8d 100644 --- a/boa_parser/src/parser/expression/left_hand_side/member.rs +++ b/boa_parser/src/parser/expression/left_hand_side/member.rs @@ -201,12 +201,6 @@ where } TokenKind::NullLiteral => SimplePropertyAccess::new(lhs, Sym::NULL).into(), TokenKind::PrivateIdentifier(name) => { - if !cursor.in_class() { - return Err(Error::general( - "Private identifier outside of class", - token.span().start(), - )); - } PrivatePropertyAccess::new(lhs, PrivateName::new(*name)).into() } _ => { diff --git a/boa_parser/src/parser/expression/left_hand_side/optional/mod.rs b/boa_parser/src/parser/expression/left_hand_side/optional/mod.rs index 75eb8a9e24a..8e739477789 100644 --- a/boa_parser/src/parser/expression/left_hand_side/optional/mod.rs +++ b/boa_parser/src/parser/expression/left_hand_side/optional/mod.rs @@ -60,8 +60,7 @@ where type Output = Optional; fn parse(self, cursor: &mut Cursor, interner: &mut Interner) -> ParseResult { - fn parse_const_access( - cursor: &mut Cursor, + fn parse_const_access( token: &Token, interner: &mut Interner, ) -> ParseResult { @@ -84,13 +83,6 @@ where field: PropertyAccessField::Const(Sym::NULL), }, TokenKind::PrivateIdentifier(name) => { - if !cursor.in_class() { - return Err(Error::general( - "Private identifier outside of class", - token.span().start(), - )); - } - OptionalOperationKind::PrivatePropertyAccess { field: PrivateName::new(*name), } @@ -121,7 +113,7 @@ where cursor.advance(interner); let field = cursor.next(interner).or_abrupt()?; - let item = parse_const_access(cursor, &field, interner)?; + let item = parse_const_access(&field, interner)?; items.push(OptionalOperation::new(item, false)); continue; @@ -162,7 +154,7 @@ where } _ => { let token = cursor.next(interner)?.expect("token disappeared"); - parse_const_access(cursor, &token, interner)? + parse_const_access(&token, interner)? } }; diff --git a/boa_parser/src/parser/expression/mod.rs b/boa_parser/src/parser/expression/mod.rs index 4f304c28dca..e8160dd9409 100644 --- a/boa_parser/src/parser/expression/mod.rs +++ b/boa_parser/src/parser/expression/mod.rs @@ -567,7 +567,6 @@ where if self.allow_in.0 { let token = cursor.peek(0, interner).or_abrupt()?; - let span = token.span(); if let TokenKind::PrivateIdentifier(identifier) = token.kind() { let identifier = *identifier; let token = cursor.peek(1, interner).or_abrupt()?; @@ -582,13 +581,6 @@ where cursor.advance(interner); cursor.advance(interner); - if !cursor.in_class() { - return Err(Error::general( - "Private identifier outside of class", - span.start(), - )); - } - let rhs = ShiftExpression::new(self.name, self.allow_yield, self.allow_await) .parse(cursor, interner)?; diff --git a/boa_parser/src/parser/mod.rs b/boa_parser/src/parser/mod.rs index 1e33bdf3156..8ca8ea9485a 100644 --- a/boa_parser/src/parser/mod.rs +++ b/boa_parser/src/parser/mod.rs @@ -22,9 +22,9 @@ use boa_ast::{ expression::Identifier, function::FormalParameterList, operations::{ - check_labels, contains, contains_invalid_object_literal, lexically_declared_names, - top_level_lexically_declared_names, top_level_var_declared_names, var_declared_names, - ContainsSymbol, + all_private_identifiers_valid, check_labels, contains, contains_invalid_object_literal, + lexically_declared_names, top_level_lexically_declared_names, top_level_var_declared_names, + var_declared_names, ContainsSymbol, }, ModuleItemList, Position, StatementList, }; @@ -340,6 +340,16 @@ where Position::new(1, 1), )); } + + // It is a Syntax Error if AllPrivateIdentifiersValid of StatementList with + // argument « » is false unless the source text containing ScriptBody is + // eval code that is being processed by a direct eval. + if !all_private_identifiers_valid(&body, Vec::new()) { + return Err(Error::general( + "invalid private identifier usage", + Position::new(1, 1), + )); + } } if let Err(error) = check_labels(&body) { diff --git a/boa_parser/src/parser/statement/declaration/hoistable/class_decl/mod.rs b/boa_parser/src/parser/statement/declaration/hoistable/class_decl/mod.rs index effe75e43f3..4c3fdb871d9 100644 --- a/boa_parser/src/parser/statement/declaration/hoistable/class_decl/mod.rs +++ b/boa_parser/src/parser/statement/declaration/hoistable/class_decl/mod.rs @@ -17,8 +17,7 @@ use crate::{ use ast::{ function::PrivateName, operations::{ - check_labels, class_private_name_resolver, contains_invalid_object_literal, - lexically_declared_names, var_declared_names, + check_labels, contains_invalid_object_literal, lexically_declared_names, var_declared_names, }, }; use boa_ast::{ @@ -182,16 +181,12 @@ where self.has_binding_identifier, )) } else { - cursor.push_private_environment(); - let body_start = cursor.peek(0, interner).or_abrupt()?.span().start(); let (constructor, elements) = ClassBody::new(self.name, self.allow_yield, self.allow_await) .parse(cursor, interner)?; cursor.expect(Punctuator::CloseBlock, "class tail", interner)?; - cursor.pop_private_environment(); - if super_ref.is_none() { if let Some(constructor) = &constructor { if contains(constructor, ContainsSymbol::SuperCall) { @@ -203,24 +198,13 @@ where } } - let mut class = Class::new( + Ok(Class::new( self.name, super_ref, constructor, elements.into(), self.has_binding_identifier, - ); - - if !cursor.in_class() - && !class_private_name_resolver(&mut class, cursor.private_environment_root_index()) - { - return Err(Error::lex(LexError::Syntax( - "invalid private name usage".into(), - body_start, - ))); - } - - Ok(class) + )) } } } diff --git a/boa_parser/src/parser/statement/mod.rs b/boa_parser/src/parser/statement/mod.rs index 33809428fae..cbe56a471f0 100644 --- a/boa_parser/src/parser/statement/mod.rs +++ b/boa_parser/src/parser/statement/mod.rs @@ -47,7 +47,7 @@ use crate::{ Error, }; use ast::{ - operations::{check_labels, contains_invalid_object_literal}, + operations::{all_private_identifiers_valid, check_labels, contains_invalid_object_literal}, Position, }; use boa_ast::{ @@ -929,7 +929,17 @@ where list.push(item); } - Ok(list.into()) + let list = list.into(); + + // It is a Syntax Error if AllPrivateIdentifiersValid of ModuleItemList with argument « » is false. + if !all_private_identifiers_valid(&list, Vec::new()) { + return Err(Error::general( + "invalid private identifier usage", + Position::new(1, 1), + )); + } + + Ok(list) } }