From 3725ff85f7a78c5a69b734cec2993ff7d3236da6 Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Wed, 1 Feb 2023 05:17:05 +0000 Subject: [PATCH] Implement binary `in` operation with private names (#2582) This Pull Request changes the following: - Implement binary `in` operation with private names. - Adding a separate `BinaryInPrivate` expression in addition to the existing `Binary` expression seems like the best way to implement this in a typesafe manner. Other methods like adding an enum for the `Binary` lhs result in having to make assertions. --- boa_ast/src/expression/mod.rs | 14 +-- boa_ast/src/expression/operator/binary/mod.rs | 85 +++++++++++++++++-- boa_ast/src/expression/operator/mod.rs | 6 +- boa_ast/src/visitor.rs | 7 +- .../src/bytecompiler/expression/binary.rs | 18 +++- boa_engine/src/bytecompiler/expression/mod.rs | 3 + boa_engine/src/vm/code_block.rs | 3 +- boa_engine/src/vm/flowgraph/mod.rs | 3 +- boa_engine/src/vm/opcode/binary_ops/mod.rs | 34 ++++++++ boa_engine/src/vm/opcode/mod.rs | 7 ++ boa_parser/src/parser/expression/mod.rs | 39 ++++++++- 11 files changed, 203 insertions(+), 16 deletions(-) diff --git a/boa_ast/src/expression/mod.rs b/boa_ast/src/expression/mod.rs index 8b48573df6..31c08ceeb7 100644 --- a/boa_ast/src/expression/mod.rs +++ b/boa_ast/src/expression/mod.rs @@ -9,20 +9,18 @@ //! [primary]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#primary_expressions //! [lhs]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#left-hand-side_expressions -use boa_interner::{Interner, ToIndentedString, ToInternedString}; -use core::ops::ControlFlow; - use self::{ access::PropertyAccess, literal::{ArrayLiteral, Literal, ObjectLiteral, TemplateLiteral}, - operator::{Assign, Binary, Conditional, Unary, Update}, + operator::{Assign, Binary, BinaryInPrivate, Conditional, Unary, Update}, }; - use super::{ function::{ArrowFunction, AsyncFunction, AsyncGenerator, Class, Function, Generator}, function::{AsyncArrowFunction, FormalParameterList}, Statement, }; +use boa_interner::{Interner, ToIndentedString, ToInternedString}; +use core::ops::ControlFlow; mod r#await; mod call; @@ -144,6 +142,9 @@ pub enum Expression { /// See [`Binary`]. Binary(Binary), + /// See [`BinaryInPrivate`]. + BinaryInPrivate(BinaryInPrivate), + /// See [`Conditional`]. Conditional(Conditional), @@ -193,6 +194,7 @@ impl Expression { Self::Unary(unary) => unary.to_interned_string(interner), Self::Update(update) => update.to_interned_string(interner), Self::Binary(bin) => bin.to_interned_string(interner), + Self::BinaryInPrivate(bin) => bin.to_interned_string(interner), Self::Conditional(cond) => cond.to_interned_string(interner), Self::Await(aw) => aw.to_interned_string(interner), Self::Yield(yi) => yi.to_interned_string(interner), @@ -286,6 +288,7 @@ impl VisitWith for Expression { Self::Unary(u) => visitor.visit_unary(u), Self::Update(u) => visitor.visit_update(u), Self::Binary(b) => visitor.visit_binary(b), + Self::BinaryInPrivate(b) => visitor.visit_binary_in_private(b), Self::Conditional(c) => visitor.visit_conditional(c), Self::Await(a) => visitor.visit_await(a), Self::Yield(y) => visitor.visit_yield(y), @@ -325,6 +328,7 @@ impl VisitWith for Expression { Self::Unary(u) => visitor.visit_unary_mut(u), Self::Update(u) => visitor.visit_update_mut(u), Self::Binary(b) => visitor.visit_binary_mut(b), + Self::BinaryInPrivate(b) => visitor.visit_binary_in_private_mut(b), Self::Conditional(c) => visitor.visit_conditional_mut(c), Self::Await(a) => visitor.visit_await_mut(a), Self::Yield(y) => visitor.visit_yield_mut(y), diff --git a/boa_ast/src/expression/operator/binary/mod.rs b/boa_ast/src/expression/operator/binary/mod.rs index cd61ae3998..8c1ddb63e9 100644 --- a/boa_ast/src/expression/operator/binary/mod.rs +++ b/boa_ast/src/expression/operator/binary/mod.rs @@ -16,16 +16,16 @@ mod op; -use core::ops::ControlFlow; -pub use op::*; - -use boa_interner::{Interner, ToInternedString}; - use crate::{ expression::Expression, + function::PrivateName, try_break, visitor::{VisitWith, Visitor, VisitorMut}, }; +use boa_interner::{Interner, ToInternedString}; +use core::ops::ControlFlow; + +pub use op::*; /// Binary operations require two operands, one before the operator and one after the operator. /// @@ -109,3 +109,78 @@ impl VisitWith for Binary { visitor.visit_expression_mut(&mut self.rhs) } } + +/// Binary [relational][relat] `In` expression with a private name on the left hand side. +/// +/// Because the left hand side must be a private name, this is a separate type from [`Binary`]. +/// +/// [relat]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#relational_operators +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] +#[derive(Clone, Debug, PartialEq)] +pub struct BinaryInPrivate { + lhs: PrivateName, + rhs: Box, +} + +impl BinaryInPrivate { + /// Creates a `BinaryInPrivate` AST Expression. + #[inline] + #[must_use] + pub fn new(lhs: PrivateName, rhs: Expression) -> Self { + Self { + lhs, + rhs: Box::new(rhs), + } + } + + /// Gets the left hand side of the binary operation. + #[inline] + #[must_use] + pub const fn lhs(&self) -> &PrivateName { + &self.lhs + } + + /// Gets the right hand side of the binary operation. + #[inline] + #[must_use] + pub const fn rhs(&self) -> &Expression { + &self.rhs + } +} + +impl ToInternedString for BinaryInPrivate { + #[inline] + fn to_interned_string(&self, interner: &Interner) -> String { + format!( + "#{} in {}", + interner.resolve_expect(self.lhs.description()), + self.rhs.to_interned_string(interner) + ) + } +} + +impl From for Expression { + #[inline] + fn from(op: BinaryInPrivate) -> Self { + Self::BinaryInPrivate(op) + } +} + +impl VisitWith for BinaryInPrivate { + fn visit_with<'a, V>(&'a self, visitor: &mut V) -> ControlFlow + where + V: Visitor<'a>, + { + try_break!(visitor.visit_private_name(&self.lhs)); + visitor.visit_expression(&self.rhs) + } + + fn visit_with_mut<'a, V>(&'a mut self, visitor: &mut V) -> ControlFlow + where + V: VisitorMut<'a>, + { + try_break!(visitor.visit_private_name_mut(&mut self.lhs)); + visitor.visit_expression_mut(&mut self.rhs) + } +} diff --git a/boa_ast/src/expression/operator/mod.rs b/boa_ast/src/expression/operator/mod.rs index ef8d1453de..d397757d23 100644 --- a/boa_ast/src/expression/operator/mod.rs +++ b/boa_ast/src/expression/operator/mod.rs @@ -20,5 +20,9 @@ pub mod unary; pub mod update; pub use self::{ - assign::Assign, binary::Binary, conditional::Conditional, unary::Unary, update::Update, + assign::Assign, + binary::{Binary, BinaryInPrivate}, + conditional::Conditional, + unary::Unary, + update::Update, }; diff --git a/boa_ast/src/visitor.rs b/boa_ast/src/visitor.rs index ac7b6ed109..806579ed92 100644 --- a/boa_ast/src/visitor.rs +++ b/boa_ast/src/visitor.rs @@ -17,7 +17,7 @@ use crate::{ literal::{ArrayLiteral, Literal, ObjectLiteral, TemplateElement, TemplateLiteral}, operator::{ assign::{Assign, AssignTarget}, - Binary, Conditional, Unary, Update, + Binary, BinaryInPrivate, Conditional, Unary, Update, }, Await, Call, Expression, Identifier, New, Optional, OptionalOperation, OptionalOperationKind, Spread, SuperCall, TaggedTemplate, Yield, @@ -167,6 +167,7 @@ node_ref! { Unary, Update, Binary, + BinaryInPrivate, Conditional, Await, Yield, @@ -254,6 +255,7 @@ pub trait Visitor<'ast>: Sized { define_visit!(visit_unary, Unary); define_visit!(visit_update, Update); define_visit!(visit_binary, Binary); + define_visit!(visit_binary_in_private, BinaryInPrivate); define_visit!(visit_conditional, Conditional); define_visit!(visit_await, Await); define_visit!(visit_yield, Yield); @@ -338,6 +340,7 @@ pub trait Visitor<'ast>: Sized { NodeRef::Unary(n) => self.visit_unary(n), NodeRef::Update(n) => self.visit_update(n), NodeRef::Binary(n) => self.visit_binary(n), + NodeRef::BinaryInPrivate(n) => self.visit_binary_in_private(n), NodeRef::Conditional(n) => self.visit_conditional(n), NodeRef::Await(n) => self.visit_await(n), NodeRef::Yield(n) => self.visit_yield(n), @@ -427,6 +430,7 @@ pub trait VisitorMut<'ast>: Sized { define_visit_mut!(visit_unary_mut, Unary); define_visit_mut!(visit_update_mut, Update); define_visit_mut!(visit_binary_mut, Binary); + define_visit_mut!(visit_binary_in_private_mut, BinaryInPrivate); define_visit_mut!(visit_conditional_mut, Conditional); define_visit_mut!(visit_await_mut, Await); define_visit_mut!(visit_yield_mut, Yield); @@ -511,6 +515,7 @@ pub trait VisitorMut<'ast>: Sized { NodeRefMut::Unary(n) => self.visit_unary_mut(n), NodeRefMut::Update(n) => self.visit_update_mut(n), NodeRefMut::Binary(n) => self.visit_binary_mut(n), + NodeRefMut::BinaryInPrivate(n) => self.visit_binary_in_private_mut(n), NodeRefMut::Conditional(n) => self.visit_conditional_mut(n), NodeRefMut::Await(n) => self.visit_await_mut(n), NodeRefMut::Yield(n) => self.visit_yield_mut(n), diff --git a/boa_engine/src/bytecompiler/expression/binary.rs b/boa_engine/src/bytecompiler/expression/binary.rs index 6a802e2fc9..44b0615a68 100644 --- a/boa_engine/src/bytecompiler/expression/binary.rs +++ b/boa_engine/src/bytecompiler/expression/binary.rs @@ -1,6 +1,6 @@ use boa_ast::expression::operator::{ binary::{ArithmeticOp, BinaryOp, BitwiseOp, LogicalOp, RelationalOp}, - Binary, + Binary, BinaryInPrivate, }; use crate::{bytecompiler::ByteCompiler, vm::Opcode, JsResult}; @@ -95,4 +95,20 @@ impl ByteCompiler<'_, '_> { Ok(()) } + + pub(crate) fn compile_binary_in_private( + &mut self, + binary: &BinaryInPrivate, + use_expr: bool, + ) -> JsResult<()> { + let index = self.get_or_insert_private_name(*binary.lhs()); + self.compile_expr(binary.rhs(), true)?; + self.emit(Opcode::InPrivate, &[index]); + + if !use_expr { + self.emit_opcode(Opcode::Pop); + } + + Ok(()) + } } diff --git a/boa_engine/src/bytecompiler/expression/mod.rs b/boa_engine/src/bytecompiler/expression/mod.rs index 4ce10f7942..89558959bf 100644 --- a/boa_engine/src/bytecompiler/expression/mod.rs +++ b/boa_engine/src/bytecompiler/expression/mod.rs @@ -91,6 +91,9 @@ impl ByteCompiler<'_, '_> { Expression::Unary(unary) => self.compile_unary(unary, use_expr)?, Expression::Update(update) => self.compile_update(update, use_expr)?, Expression::Binary(binary) => self.compile_binary(binary, use_expr)?, + Expression::BinaryInPrivate(binary) => { + self.compile_binary_in_private(binary, use_expr)?; + } Expression::Assign(assign) => self.compile_assign(assign, use_expr)?, Expression::ObjectLiteral(object) => { self.compile_object_literal(object, use_expr)?; diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index f46326c10e..ffcbf21e29 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -323,7 +323,8 @@ impl CodeBlock { | Opcode::PushClassFieldPrivate | Opcode::PushClassPrivateGetter | Opcode::PushClassPrivateSetter - | Opcode::PushClassPrivateMethod => { + | Opcode::PushClassPrivateMethod + | Opcode::InPrivate => { let operand = self.read::(*pc); *pc += size_of::(); format!( diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index d0a1c2ce04..0b225734d8 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -358,7 +358,8 @@ impl CodeBlock { | Opcode::PushClassFieldPrivate | Opcode::PushClassPrivateGetter | Opcode::PushClassPrivateSetter - | Opcode::PushClassPrivateMethod => { + | Opcode::PushClassPrivateMethod + | Opcode::InPrivate => { let operand = self.read::(pc); pc += size_of::(); let label = format!( diff --git a/boa_engine/src/vm/opcode/binary_ops/mod.rs b/boa_engine/src/vm/opcode/binary_ops/mod.rs index c6809af01c..384bf222ea 100644 --- a/boa_engine/src/vm/opcode/binary_ops/mod.rs +++ b/boa_engine/src/vm/opcode/binary_ops/mod.rs @@ -98,6 +98,40 @@ impl Operation for In { } } +/// `InPrivate` implements the Opcode Operation for `Opcode::InPrivate` +/// +/// Operation: +/// - Binary `in` operation for private names. +#[derive(Debug, Clone, Copy)] +pub(crate) struct InPrivate; + +impl Operation for InPrivate { + const NAME: &'static str = "InPrivate"; + const INSTRUCTION: &'static str = "INST - 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 rhs = context.vm.pop(); + + let Some(rhs) = rhs.as_object() else { + return Err(JsNativeError::typ() + .with_message(format!( + "right-hand side of 'in' should be an object, got `{}`", + rhs.type_of() + )) + .into()); + }; + if rhs.private_element_find(&name, true, true).is_some() { + context.vm.push(true); + } else { + context.vm.push(false); + } + + Ok(ShouldExit::False) + } +} + /// `InstanceOf` implements the Opcode Operation for `Opcode::InstanceOf` /// /// Operation: diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index d4ad0e5a16..3dc25286fe 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -486,6 +486,13 @@ generate_impl! { /// Stack: lhs, rhs **=>** (lhs `in` rhs) In, + /// Binary `in` operator for private names. + /// + /// Operands: private_name_index: `u32` + /// + /// Stack: rhs **=>** (private_name `in` rhs) + InPrivate, + /// Binary `==` operator. /// /// Operands: diff --git a/boa_parser/src/parser/expression/mod.rs b/boa_parser/src/parser/expression/mod.rs index 5e3e3c0d0a..1d8c8148af 100644 --- a/boa_parser/src/parser/expression/mod.rs +++ b/boa_parser/src/parser/expression/mod.rs @@ -32,10 +32,11 @@ use boa_ast::{ expression::{ operator::{ binary::{BinaryOp, LogicalOp}, - Binary, + Binary, BinaryInPrivate, }, Identifier, }, + function::PrivateName, Keyword, Position, Punctuator, }; use boa_interner::{Interner, Sym}; @@ -564,8 +565,44 @@ where fn parse(self, cursor: &mut Cursor, interner: &mut Interner) -> ParseResult { let _timer = Profiler::global().start_event("Relation Expression", "Parsing"); + 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()?; + match token.kind() { + TokenKind::Keyword((Keyword::In, true)) => { + return Err(Error::general( + "Keyword must not contain escaped characters", + token.span().start(), + )); + } + TokenKind::Keyword((Keyword::In, false)) => { + 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)?; + + return Ok(BinaryInPrivate::new(PrivateName::new(identifier), rhs).into()); + } + _ => {} + } + } + } + let mut lhs = ShiftExpression::new(self.name, self.allow_yield, self.allow_await) .parse(cursor, interner)?; + while let Some(tok) = cursor.peek(0, interner)? { match *tok.kind() { TokenKind::Punctuator(op)