From 431a358b2fddbb10056f97148e46c86b7448a9e9 Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Mon, 20 Mar 2023 17:04:39 +0000 Subject: [PATCH] Implement `with` and object environments (#2692) This Pull Request changes the following: - Implement `with` statement parsing, ast node, compilation and excution. - Implement object environments that are used in the `with` statement excution. The implementation of object environments can probably be optimized further by using more compile-time information about when object environments can exist. Maybe there could also be a separate environment stack for object environments to reduce the filtering and iteration that is needed with the current implementation. This does not fix all tests in the `test/language/statements/with` suite yet. But for most failing tests that I have looked at we are missing other features / have bugs elsewhere. As a note for the review: The functions in the `impl Context` block in `boa_engine/src/environments/runtime.rs` are mostly copied / moved from the existing functions. The only change there should be the addition of the object environment logic. They had to be moved to `Context` because of borrow semantics. --- boa_ast/src/operations.rs | 1 + boa_ast/src/statement/mod.rs | 9 +- boa_ast/src/statement/with.rs | 81 +++ boa_ast/src/visitor.rs | 4 +- boa_engine/src/builtins/eval/mod.rs | 5 +- boa_engine/src/bytecompiler/statement/mod.rs | 4 +- boa_engine/src/bytecompiler/statement/with.rs | 14 + boa_engine/src/environments/mod.rs | 3 +- boa_engine/src/environments/runtime.rs | 639 +++++++++++++----- boa_engine/src/tests/mod.rs | 2 +- boa_engine/src/vm/code_block.rs | 2 + boa_engine/src/vm/flowgraph/mod.rs | 3 +- boa_engine/src/vm/opcode/define/mod.rs | 16 +- boa_engine/src/vm/opcode/delete/mod.rs | 11 +- boa_engine/src/vm/opcode/get/name.rs | 20 +- boa_engine/src/vm/opcode/mod.rs | 7 + boa_engine/src/vm/opcode/push/environment.rs | 21 + boa_engine/src/vm/opcode/set/name.rs | 10 +- boa_parser/src/parser/statement/mod.rs | 7 + boa_parser/src/parser/statement/with/mod.rs | 89 +++ 20 files changed, 729 insertions(+), 219 deletions(-) create mode 100644 boa_ast/src/statement/with.rs create mode 100644 boa_engine/src/bytecompiler/statement/with.rs create mode 100644 boa_parser/src/parser/statement/with/mod.rs diff --git a/boa_ast/src/operations.rs b/boa_ast/src/operations.rs index 68e9d7997ed..69a054859aa 100644 --- a/boa_ast/src/operations.rs +++ b/boa_ast/src/operations.rs @@ -945,6 +945,7 @@ where Statement::Try(node) => self.visit_try(node), Statement::Continue(node) => self.visit_continue(node), Statement::Break(node) => self.visit_break(node), + Statement::With(with) => self.visit_with(with), } } diff --git a/boa_ast/src/statement/mod.rs b/boa_ast/src/statement/mod.rs index d392dd14ef2..d6c920c4925 100644 --- a/boa_ast/src/statement/mod.rs +++ b/boa_ast/src/statement/mod.rs @@ -14,6 +14,7 @@ mod r#return; mod switch; mod throw; mod r#try; +mod with; pub mod iteration; @@ -26,6 +27,7 @@ pub use self::{ r#try::{Catch, ErrorHandler, Finally, Try}, switch::{Case, Switch}, throw::Throw, + with::With, }; use core::ops::ControlFlow; @@ -92,7 +94,6 @@ pub enum Statement { /// See [`Return`]. Return(Return), - // TODO: Possibly add `with` statements. /// See [`Labelled`]. Labelled(Labelled), @@ -101,6 +102,9 @@ pub enum Statement { /// See [`Try`]. Try(Try), + + /// See [`With`]. + With(With), } impl Statement { @@ -129,6 +133,7 @@ impl Statement { Self::Labelled(labelled) => return labelled.to_interned_string(interner), Self::Throw(throw) => throw.to_interned_string(interner), Self::Try(try_catch) => return try_catch.to_indented_string(interner, indentation), + Self::With(with) => return with.to_interned_string(interner), }; s.push(';'); s @@ -208,6 +213,7 @@ impl VisitWith for Statement { Self::Labelled(l) => visitor.visit_labelled(l), Self::Throw(th) => visitor.visit_throw(th), Self::Try(tr) => visitor.visit_try(tr), + Self::With(with) => visitor.visit_with(with), } } @@ -236,6 +242,7 @@ impl VisitWith for Statement { Self::Labelled(l) => visitor.visit_labelled_mut(l), Self::Throw(th) => visitor.visit_throw_mut(th), Self::Try(tr) => visitor.visit_try_mut(tr), + Self::With(with) => visitor.visit_with_mut(with), } } } diff --git a/boa_ast/src/statement/with.rs b/boa_ast/src/statement/with.rs new file mode 100644 index 00000000000..82b969099b3 --- /dev/null +++ b/boa_ast/src/statement/with.rs @@ -0,0 +1,81 @@ +use crate::{ + expression::Expression, + statement::Statement, + try_break, + visitor::{VisitWith, Visitor, VisitorMut}, +}; +use boa_interner::{Interner, ToInternedString}; +use core::ops::ControlFlow; + +/// The `with` statement extends the scope chain for a statement. +/// +/// More information: +/// - [ECMAScript reference][spec] +/// - [MDN documentation][mdn] +/// +/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/with +/// [spec]: https://tc39.es/ecma262/#prod-WithStatement +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] +#[derive(Clone, Debug, PartialEq)] +pub struct With { + expression: Expression, + statement: Box, +} + +impl With { + /// Creates a `With` AST node. + #[must_use] + pub fn new(expression: Expression, statement: Statement) -> Self { + Self { + expression, + statement: Box::new(statement), + } + } + + /// Gets the expression value of this `With` statement. + #[must_use] + pub const fn expression(&self) -> &Expression { + &self.expression + } + + /// Gets the statement value of this `With` statement. + #[must_use] + pub const fn statement(&self) -> &Statement { + &self.statement + } +} + +impl From for Statement { + fn from(with: With) -> Self { + Self::With(with) + } +} + +impl ToInternedString for With { + fn to_interned_string(&self, interner: &Interner) -> String { + format!( + "with ({}) {{{}}}", + self.expression.to_interned_string(interner), + self.statement.to_interned_string(interner) + ) + } +} + +impl VisitWith for With { + fn visit_with<'a, V>(&'a self, visitor: &mut V) -> ControlFlow + where + V: Visitor<'a>, + { + try_break!(visitor.visit_expression(&self.expression)); + visitor.visit_statement(&self.statement) + } + + fn visit_with_mut<'a, V>(&'a mut self, visitor: &mut V) -> ControlFlow + where + V: VisitorMut<'a>, + { + try_break!(visitor.visit_expression_mut(&mut self.expression)); + visitor.visit_statement_mut(&mut self.statement) + } +} diff --git a/boa_ast/src/visitor.rs b/boa_ast/src/visitor.rs index 3654e57c82b..22fd6ff3ccb 100644 --- a/boa_ast/src/visitor.rs +++ b/boa_ast/src/visitor.rs @@ -36,7 +36,7 @@ use crate::{ IterableLoopInitializer, WhileLoop, }, Block, Case, Catch, Finally, If, Labelled, LabelledItem, Return, Statement, Switch, Throw, - Try, + Try, With, }, ModuleItem, ModuleItemList, StatementList, StatementListItem, }; @@ -241,6 +241,7 @@ pub trait Visitor<'ast>: Sized { define_visit!(visit_labelled, Labelled); define_visit!(visit_throw, Throw); define_visit!(visit_try, Try); + define_visit!(visit_with, With); define_visit!(visit_identifier, Identifier); define_visit!(visit_formal_parameter_list, FormalParameterList); define_visit!(visit_class_element, ClassElement); @@ -434,6 +435,7 @@ pub trait VisitorMut<'ast>: Sized { define_visit_mut!(visit_labelled_mut, Labelled); define_visit_mut!(visit_throw_mut, Throw); define_visit_mut!(visit_try_mut, Try); + define_visit_mut!(visit_with_mut, With); define_visit_mut!(visit_identifier_mut, Identifier); define_visit_mut!(visit_formal_parameter_list_mut, FormalParameterList); define_visit_mut!(visit_class_element_mut, ClassElement); diff --git a/boa_engine/src/builtins/eval/mod.rs b/boa_engine/src/builtins/eval/mod.rs index d246c9c8e8e..981efaf896a 100644 --- a/boa_engine/src/builtins/eval/mod.rs +++ b/boa_engine/src/builtins/eval/mod.rs @@ -10,13 +10,12 @@ //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval use crate::{ - builtins::BuiltInObject, context::intrinsics::Intrinsics, environments::DeclarativeEnvironment, + builtins::BuiltInObject, context::intrinsics::Intrinsics, environments::Environment, error::JsNativeError, object::JsObject, Context, JsArgs, JsResult, JsString, JsValue, }; use boa_ast::operations::{ contains, contains_arguments, top_level_var_declared_names, ContainsSymbol, }; -use boa_gc::Gc; use boa_parser::{Parser, Source}; use boa_profiler::Profiler; @@ -85,7 +84,7 @@ impl Eval { #[derive(Debug)] enum EnvStackAction { Truncate(usize), - Restore(Vec>), + Restore(Vec), } /// Restores the environment after calling `eval` or after throwing an error. diff --git a/boa_engine/src/bytecompiler/statement/mod.rs b/boa_engine/src/bytecompiler/statement/mod.rs index d7b82424bad..36688a2d1fe 100644 --- a/boa_engine/src/bytecompiler/statement/mod.rs +++ b/boa_engine/src/bytecompiler/statement/mod.rs @@ -10,6 +10,7 @@ mod labelled; mod r#loop; mod switch; mod r#try; +mod with; impl ByteCompiler<'_, '_> { /// Compiles a [`Statement`] `boa_ast` node. @@ -56,8 +57,9 @@ impl ByteCompiler<'_, '_> { self.emit(Opcode::Return, &[]); } Statement::Try(t) => self.compile_try(t, use_expr, configurable_globals), - Statement::Empty => {} Statement::Expression(expr) => self.compile_expr(expr, use_expr), + Statement::With(with) => self.compile_with(with, configurable_globals), + Statement::Empty => {} } } } diff --git a/boa_engine/src/bytecompiler/statement/with.rs b/boa_engine/src/bytecompiler/statement/with.rs new file mode 100644 index 00000000000..b8c1ee84cd9 --- /dev/null +++ b/boa_engine/src/bytecompiler/statement/with.rs @@ -0,0 +1,14 @@ +use crate::{bytecompiler::ByteCompiler, vm::Opcode}; +use boa_ast::statement::With; + +impl ByteCompiler<'_, '_> { + /// Compile a [`With`] `boa_ast` node + pub(crate) fn compile_with(&mut self, with: &With, configurable_globals: bool) { + self.compile_expr(with.expression(), true); + self.context.push_compile_time_environment(false); + self.emit_opcode(Opcode::PushObjectEnvironment); + self.compile_stmt(with.statement(), false, configurable_globals); + self.context.pop_compile_time_environment(); + self.emit_opcode(Opcode::PopEnvironment); + } +} diff --git a/boa_engine/src/environments/mod.rs b/boa_engine/src/environments/mod.rs index 612c57a3a81..e9c7dc2761f 100644 --- a/boa_engine/src/environments/mod.rs +++ b/boa_engine/src/environments/mod.rs @@ -30,7 +30,8 @@ mod runtime; pub(crate) use { compile::CompileTimeEnvironment, runtime::{ - BindingLocator, DeclarativeEnvironment, DeclarativeEnvironmentStack, EnvironmentSlots, + BindingLocator, DeclarativeEnvironment, DeclarativeEnvironmentStack, Environment, + EnvironmentSlots, }, }; diff --git a/boa_engine/src/environments/runtime.rs b/boa_engine/src/environments/runtime.rs index 28a54630052..211d9e04a2f 100644 --- a/boa_engine/src/environments/runtime.rs +++ b/boa_engine/src/environments/runtime.rs @@ -1,5 +1,6 @@ use crate::{ - environments::CompileTimeEnvironment, error::JsNativeError, object::JsObject, Context, JsValue, + environments::CompileTimeEnvironment, error::JsNativeError, object::JsObject, Context, + JsResult, JsString, JsSymbol, JsValue, }; use boa_ast::expression::Identifier; use boa_gc::{Finalize, Gc, GcRefCell, Trace}; @@ -32,6 +33,8 @@ pub(crate) struct DeclarativeEnvironment { compile: Gc>, #[unsafe_ignore_trace] poisoned: Cell, + #[unsafe_ignore_trace] + with: Cell, slots: Option, } @@ -214,25 +217,49 @@ impl DeclarativeEnvironment { } } -/// A declarative environment stack holds all declarative environments at runtime. +/// The environment stack holds all environments at runtime. /// /// Environments themselves are garbage collected, /// because they must be preserved for function calls. #[derive(Clone, Debug, Trace, Finalize)] pub struct DeclarativeEnvironmentStack { - stack: Vec>, + stack: Vec, +} + +/// A runtime environment. +#[derive(Clone, Debug, Trace, Finalize)] +pub(crate) enum Environment { + Declarative(Gc), + Object(JsObject), +} + +impl Environment { + /// Returns the declarative environment if it is one. + pub(crate) const fn as_declarative(&self) -> Option<&Gc> { + match self { + Self::Declarative(env) => Some(env), + Self::Object(_) => None, + } + } + + /// Returns the declarative environment and panic if it is not one. + pub(crate) fn declarative_expect(&self) -> &Gc { + self.as_declarative() + .expect("environment must be declarative") + } } impl DeclarativeEnvironmentStack { /// Create a new environment stack with the most outer declarative environment. pub(crate) fn new(global_compile_environment: Gc>) -> Self { Self { - stack: vec![Gc::new(DeclarativeEnvironment { + stack: Vec::from([Environment::Declarative(Gc::new(DeclarativeEnvironment { bindings: GcRefCell::new(Vec::new()), compile: global_compile_environment, poisoned: Cell::new(false), + with: Cell::new(false), slots: Some(EnvironmentSlots::Global), - })], + }))]), } } @@ -240,7 +267,12 @@ impl DeclarativeEnvironmentStack { /// /// This is only useful when compiled bindings are added after the initial compilation (eval). pub(crate) fn extend_outer_function_environment(&mut self) { - for env in self.stack.iter().rev() { + for env in self + .stack + .iter() + .filter_map(Environment::as_declarative) + .rev() + { if let Some(EnvironmentSlots::Function(_)) = env.slots { let compile_bindings_number = env.compile.borrow().num_bindings(); let mut bindings_mut = env.bindings.borrow_mut(); @@ -262,7 +294,12 @@ impl DeclarativeEnvironmentStack { &self, names: &FxHashSet, ) -> Option { - for env in self.stack.iter().rev() { + for env in self + .stack + .iter() + .filter_map(Environment::as_declarative) + .rev() + { let compile = env.compile.borrow(); for name in names { if compile.has_lex_binding(*name) { @@ -277,7 +314,7 @@ impl DeclarativeEnvironmentStack { } /// Pop all current environments except the global environment. - pub(crate) fn pop_to_global(&mut self) -> Vec> { + pub(crate) fn pop_to_global(&mut self) -> Vec { self.stack.split_off(1) } @@ -292,7 +329,7 @@ impl DeclarativeEnvironmentStack { } /// Extend the current environment stack with the given environments. - pub(crate) fn extend(&mut self, other: Vec>) { + pub(crate) fn extend(&mut self, other: Vec) { self.stack.extend(other); } @@ -305,7 +342,8 @@ impl DeclarativeEnvironmentStack { let environment = self .stack .get(0) - .expect("global environment must always exist"); + .and_then(Environment::as_declarative) + .expect("global environment must be declarative and exist"); let mut bindings = environment.bindings.borrow_mut(); if bindings.len() < binding_number { bindings.resize(binding_number, None); @@ -325,7 +363,12 @@ impl DeclarativeEnvironmentStack { /// /// Panics if no environment exists on the stack. pub(crate) fn get_this_environment(&self) -> &EnvironmentSlots { - for env in self.stack.iter().rev() { + for env in self + .stack + .iter() + .filter_map(Environment::as_declarative) + .rev() + { if let Some(slots) = &env.slots { match slots { EnvironmentSlots::Function(function_env) => { @@ -341,6 +384,13 @@ impl DeclarativeEnvironmentStack { panic!("global environment must exist") } + /// Push a new object environment on the environments stack and return it's index. + pub(crate) fn push_object(&mut self, object: JsObject) -> usize { + let index = self.stack.len(); + self.stack.push(Environment::Object(object)); + index + } + /// Push a declarative environment on the environments stack and return it's index. /// /// # Panics @@ -351,21 +401,33 @@ impl DeclarativeEnvironmentStack { num_bindings: usize, compile_environment: Gc>, ) -> usize { - let poisoned = self - .stack - .last() - .expect("global environment must always exist") - .poisoned - .get(); + let (poisoned, with) = { + let with = self + .stack + .last() + .expect("global environment must always exist") + .as_declarative() + .is_none(); + + let environment = self + .stack + .iter() + .filter_map(Environment::as_declarative) + .last() + .expect("global environment must always exist"); + (environment.poisoned.get(), with || environment.with.get()) + }; let index = self.stack.len(); - self.stack.push(Gc::new(DeclarativeEnvironment { - bindings: GcRefCell::new(vec![None; num_bindings]), - compile: compile_environment, - poisoned: Cell::new(poisoned), - slots: None, - })); + self.stack + .push(Environment::Declarative(Gc::new(DeclarativeEnvironment { + bindings: GcRefCell::new(vec![None; num_bindings]), + compile: compile_environment, + poisoned: Cell::new(poisoned), + with: Cell::new(with), + slots: None, + }))); index } @@ -384,12 +446,22 @@ impl DeclarativeEnvironmentStack { new_target: Option, lexical: bool, ) { - let outer = self - .stack - .last() - .expect("global environment must always exist"); - - let poisoned = outer.poisoned.get(); + let (poisoned, with) = { + let with = self + .stack + .last() + .expect("global environment must always exist") + .as_declarative() + .is_none(); + + let environment = self + .stack + .iter() + .filter_map(Environment::as_declarative) + .last() + .expect("global environment must always exist"); + (environment.poisoned.get(), with || environment.with.get()) + }; let this_binding_status = if lexical { ThisBindingStatus::Lexical @@ -401,17 +473,19 @@ impl DeclarativeEnvironmentStack { let this = this.unwrap_or(JsValue::Null); - self.stack.push(Gc::new(DeclarativeEnvironment { - bindings: GcRefCell::new(vec![None; num_bindings]), - compile: compile_environment, - poisoned: Cell::new(poisoned), - slots: Some(EnvironmentSlots::Function(GcRefCell::new(FunctionSlots { - this, - this_binding_status, - function_object, - new_target, - }))), - })); + self.stack + .push(Environment::Declarative(Gc::new(DeclarativeEnvironment { + bindings: GcRefCell::new(vec![None; num_bindings]), + compile: compile_environment, + poisoned: Cell::new(poisoned), + with: Cell::new(with), + slots: Some(EnvironmentSlots::Function(GcRefCell::new(FunctionSlots { + this, + this_binding_status, + function_object, + new_target, + }))), + }))); } /// Push a function environment that inherits it's internal slots from the outer environment. @@ -424,24 +498,39 @@ impl DeclarativeEnvironmentStack { num_bindings: usize, compile_environment: Gc>, ) { - let outer = self - .stack - .last() - .expect("global environment must always exist"); - - let poisoned = outer.poisoned.get(); - let slots = outer.slots.clone(); + let (poisoned, with, slots) = { + let with = self + .stack + .last() + .expect("global environment must always exist") + .as_declarative() + .is_none(); + + let environment = self + .stack + .iter() + .filter_map(Environment::as_declarative) + .last() + .expect("global environment must always exist"); + ( + environment.poisoned.get(), + with || environment.with.get(), + environment.slots.clone(), + ) + }; - self.stack.push(Gc::new(DeclarativeEnvironment { - bindings: GcRefCell::new(vec![None; num_bindings]), - compile: compile_environment, - poisoned: Cell::new(poisoned), - slots, - })); + self.stack + .push(Environment::Declarative(Gc::new(DeclarativeEnvironment { + bindings: GcRefCell::new(vec![None; num_bindings]), + compile: compile_environment, + poisoned: Cell::new(poisoned), + with: Cell::new(with), + slots, + }))); } /// Pop environment from the environments stack. - pub(crate) fn pop(&mut self) -> Gc { + pub(crate) fn pop(&mut self) -> Environment { debug_assert!(self.stack.len() > 1); self.stack .pop() @@ -454,7 +543,12 @@ impl DeclarativeEnvironmentStack { /// /// Panics if no environment exists on the stack. pub(crate) fn current_function_slots(&self) -> &EnvironmentSlots { - for env in self.stack.iter().rev() { + for env in self + .stack + .iter() + .filter_map(Environment::as_declarative) + .rev() + { if let Some(slots) = &env.slots { return slots; } @@ -472,6 +566,7 @@ impl DeclarativeEnvironmentStack { self.stack .last() .expect("global environment must always exist") + .declarative_expect() .clone() } @@ -482,6 +577,8 @@ impl DeclarativeEnvironmentStack { /// Panics if no environment exists on the stack. pub(crate) fn current_compile_environment(&self) -> Gc> { self.stack + .iter() + .filter_map(Environment::as_declarative) .last() .expect("global environment must always exist") .compile @@ -495,6 +592,8 @@ impl DeclarativeEnvironmentStack { /// Panics if no environment exists on the stack. pub(crate) fn poison_current(&mut self) { self.stack + .iter() + .filter_map(Environment::as_declarative) .last() .expect("global environment must always exist") .poisoned @@ -504,14 +603,16 @@ impl DeclarativeEnvironmentStack { /// Mark that there may be added binding in all environments. pub(crate) fn poison_all(&mut self) { for env in &mut self.stack { - if env.poisoned.get() { - return; + if let Some(env) = env.as_declarative() { + if env.poisoned.get() { + return; + } + env.poisoned.set(true); } - env.poisoned.set(true); } } - /// Get the value of a binding. + /// Get the value of a binding. Ignores object environments. /// /// # Panics /// @@ -524,10 +625,12 @@ impl DeclarativeEnvironmentStack { ) -> Option { if environment_index != self.stack.len() - 1 { for env_index in (environment_index + 1..self.stack.len()).rev() { - let env = self + let Environment::Declarative(env) = self .stack .get(env_index) - .expect("environment index must be in range"); + .expect("environment index must be in range") else { + continue; + }; if !env.poisoned.get() { break; } @@ -545,6 +648,7 @@ impl DeclarativeEnvironmentStack { self.stack .get(environment_index) .expect("environment index must be in range") + .declarative_expect() .bindings .borrow() .get(binding_index) @@ -552,33 +656,6 @@ impl DeclarativeEnvironmentStack { .clone() } - /// Get the value of a binding by it's name. - /// - /// This only considers function environments that are poisoned. - /// All other bindings are accessed via indices. - pub(crate) fn get_value_if_global_poisoned(&self, name: Identifier) -> Option { - for env in self.stack.iter().rev() { - if !env.poisoned.get() { - return None; - } - let compile = env.compile.borrow(); - if compile.is_function() { - if let Some(b) = compile.get_binding(name) { - return self - .stack - .get(b.environment_index) - .expect("environment index must be in range") - .bindings - .borrow() - .get(b.binding_index) - .expect("binding index must be in range") - .clone(); - } - } - } - None - } - /// Set the value of a binding. /// /// # Panics @@ -594,6 +671,7 @@ impl DeclarativeEnvironmentStack { .stack .get(environment_index) .expect("environment index must be in range") + .declarative_expect() .bindings .borrow_mut(); let binding = bindings @@ -602,56 +680,6 @@ impl DeclarativeEnvironmentStack { *binding = Some(value); } - /// Set the value of a binding if it is initialized. - /// Return `true` if the value has been set. - /// - /// # Panics - /// - /// Panics if the environment or binding index are out of range. - pub(crate) fn put_value_if_initialized( - &mut self, - mut environment_index: usize, - mut binding_index: usize, - name: Identifier, - value: JsValue, - ) -> bool { - if environment_index != self.stack.len() - 1 { - for env_index in (environment_index + 1..self.stack.len()).rev() { - let env = self - .stack - .get(env_index) - .expect("environment index must be in range"); - if !env.poisoned.get() { - break; - } - let compile = env.compile.borrow(); - if compile.is_function() { - if let Some(b) = compile.get_binding(name) { - environment_index = b.environment_index; - binding_index = b.binding_index; - break; - } - } - } - } - - let mut bindings = self - .stack - .get(environment_index) - .expect("environment index must be in range") - .bindings - .borrow_mut(); - let binding = bindings - .get_mut(binding_index) - .expect("binding index must be in range"); - if binding.is_none() { - false - } else { - *binding = Some(value); - true - } - } - /// Set the value of a binding if it is uninitialized. /// /// # Panics @@ -667,6 +695,7 @@ impl DeclarativeEnvironmentStack { .stack .get(environment_index) .expect("environment index must be in range") + .declarative_expect() .bindings .borrow_mut(); let binding = bindings @@ -677,44 +706,7 @@ impl DeclarativeEnvironmentStack { } } - /// Set the value of a binding by it's name. - /// - /// This only considers function environments that are poisoned. - /// All other bindings are set via indices. - /// - /// # Panics - /// - /// Panics if the environment or binding index are out of range. - pub(crate) fn put_value_if_global_poisoned( - &mut self, - name: Identifier, - value: &JsValue, - ) -> bool { - for env in self.stack.iter().rev() { - if !env.poisoned.get() { - return false; - } - let compile = env.compile.borrow(); - if compile.is_function() { - if let Some(b) = compile.get_binding(name) { - let mut bindings = self - .stack - .get(b.environment_index) - .expect("environment index must be in range") - .bindings - .borrow_mut(); - let binding = bindings - .get_mut(b.binding_index) - .expect("binding index must be in range"); - *binding = Some(value.clone()); - return true; - } - } - } - false - } - - /// Checks if the name only exists as a global property. + /// Check if a binding name does exist in a poisoned environment. /// /// A binding could be marked as `global`, and at the same time, exist in a deeper environment /// context; if the global context is poisoned, an `eval` call could have added a binding that is @@ -723,25 +715,26 @@ impl DeclarativeEnvironmentStack { /// /// # Panics /// - /// Panics if the environment or binding index are out of range. - pub(crate) fn is_only_global_property(&mut self, name: Identifier) -> bool { + /// Panics if the global environment does not exist. + pub(crate) fn binding_in_poisoned_environment(&mut self, name: Identifier) -> bool { for env in self .stack .split_first() .expect("global environment must exist") .1 .iter() + .filter_map(Environment::as_declarative) .rev() { if !env.poisoned.get() { - return true; + return false; } let compile = env.compile.borrow(); if compile.is_function() && compile.get_binding(name).is_some() { - return false; + return true; } } - true + false } } @@ -852,3 +845,287 @@ impl BindingLocator { } } } + +impl Context<'_> { + /// Get the value of a binding. + /// + /// # Panics + /// + /// Panics if the environment or binding index are out of range. + pub(crate) fn get_value_optional( + &mut self, + mut environment_index: usize, + mut binding_index: usize, + name: Identifier, + ) -> JsResult> { + for env_index in (environment_index + 1..self.realm.environments.stack.len()).rev() { + match self.environment_expect(env_index) { + Environment::Declarative(env) => { + if env.poisoned.get() { + let compile = env.compile.borrow(); + if compile.is_function() { + if let Some(b) = compile.get_binding(name) { + environment_index = b.environment_index; + binding_index = b.binding_index; + break; + } + } + } else if !env.with.get() { + break; + } + } + Environment::Object(o) => { + let o = o.clone(); + let key: JsString = self + .interner() + .resolve_expect(name.sym()) + .into_common(false); + if o.has_property(key.clone(), self)? { + if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object() + { + if unscopables.get(key.clone(), self)?.to_boolean() { + continue; + } + } + return o.get(key, self).map(Some); + } + } + } + } + + Ok(self + .environment_expect(environment_index) + .declarative_expect() + .bindings + .borrow() + .get(binding_index) + .expect("binding index must be in range") + .clone()) + } + + /// Get the value of a binding by it's name. + /// + /// This only considers function environments that are poisoned. + /// All other bindings are accessed via indices. + pub(crate) fn get_value_if_global_poisoned( + &mut self, + name: Identifier, + ) -> JsResult> { + for env_index in (0..self.realm.environments.stack.len()).rev() { + let env = self.environment_expect(env_index); + + match env { + Environment::Declarative(env) => { + if env.poisoned.get() { + let compile = env.compile.borrow(); + if compile.is_function() { + if let Some(b) = compile.get_binding(name) { + return Ok(self + .environment_expect(b.environment_index) + .declarative_expect() + .bindings + .borrow() + .get(b.binding_index) + .expect("binding index must be in range") + .clone()); + } + } else if !env.with.get() { + return Ok(None); + } + } + } + Environment::Object(o) => { + let o = o.clone(); + let key: JsString = self + .interner() + .resolve_expect(name.sym()) + .into_common(false); + if o.has_property(key.clone(), self)? { + if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object() + { + if unscopables.get(key.clone(), self)?.to_boolean() { + continue; + } + } + return o.get(key, self).map(Some); + } + } + } + } + + Ok(None) + } + + /// Set the value of a binding if it is initialized. + /// Return `true` if the value has been set. + /// + /// # Panics + /// + /// Panics if the environment or binding index are out of range. + pub(crate) fn put_value_if_initialized( + &mut self, + mut environment_index: usize, + mut binding_index: usize, + name: Identifier, + value: JsValue, + ) -> JsResult { + for env_index in (environment_index + 1..self.realm.environments.stack.len()).rev() { + let env = self.environment_expect(env_index); + + match env { + Environment::Declarative(env) => { + if env.poisoned.get() { + let compile = env.compile.borrow(); + if compile.is_function() { + if let Some(b) = compile.get_binding(name) { + environment_index = b.environment_index; + binding_index = b.binding_index; + break; + } + } + } else if !env.with.get() { + break; + } + } + Environment::Object(o) => { + let o = o.clone(); + let key: JsString = self + .interner() + .resolve_expect(name.sym()) + .into_common(false); + if o.has_property(key.clone(), self)? { + if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object() + { + if unscopables.get(key.clone(), self)?.to_boolean() { + continue; + } + } + return o.set(key, value, true, self); + } + } + } + } + + let mut bindings = self + .environment_expect(environment_index) + .declarative_expect() + .bindings + .borrow_mut(); + let binding = bindings + .get_mut(binding_index) + .expect("binding index must be in range"); + if binding.is_none() { + Ok(false) + } else { + *binding = Some(value); + Ok(true) + } + } + + /// Set the value of a binding by it's name. + /// + /// This only considers function environments that are poisoned. + /// All other bindings are set via indices. + /// + /// # Panics + /// + /// Panics if the environment or binding index are out of range. + pub(crate) fn put_value_if_global_poisoned( + &mut self, + name: Identifier, + value: &JsValue, + ) -> JsResult { + for env_index in (0..self.realm.environments.stack.len()).rev() { + let env = self.environment_expect(env_index); + + match env { + Environment::Declarative(env) => { + if env.poisoned.get() { + let compile = env.compile.borrow(); + if compile.is_function() { + if let Some(b) = compile.get_binding(name) { + let mut bindings = self + .environment_expect(b.environment_index) + .declarative_expect() + .bindings + .borrow_mut(); + let binding = bindings + .get_mut(b.binding_index) + .expect("binding index must be in range"); + *binding = Some(value.clone()); + return Ok(true); + } + } + } else if !env.with.get() { + return Ok(false); + } + } + Environment::Object(o) => { + let o = o.clone(); + let key: JsString = self + .interner() + .resolve_expect(name.sym()) + .into_common(false); + if o.has_property(key.clone(), self)? { + if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object() + { + if unscopables.get(key.clone(), self)?.to_boolean() { + continue; + } + } + return o.set(key, value.clone(), true, self); + } + } + } + } + + Ok(false) + } + + /// Delete a binding form an object environment if it exists. + /// + /// Returns a tuple of `(found, deleted)`. + pub(crate) fn delete_binding_from_objet_environment( + &mut self, + name: Identifier, + ) -> JsResult<(bool, bool)> { + for env_index in (0..self.realm.environments.stack.len()).rev() { + let env = self.environment_expect(env_index); + + match env { + Environment::Object(o) => { + let o = o.clone(); + let key: JsString = self + .interner() + .resolve_expect(name.sym()) + .into_common(false); + if o.has_property(key.clone(), self)? { + if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object() + { + if unscopables.get(key.clone(), self)?.to_boolean() { + continue; + } + } + return Ok((true, o.__delete__(&key.into(), self)?)); + } + } + Environment::Declarative(env) => { + if !env.with.get() { + return Ok((false, false)); + } + } + } + } + + Ok((false, false)) + } + + /// Return the environment at the given index. Panics if the index is out of range. + fn environment_expect(&self, index: usize) -> &Environment { + self.realm + .environments + .stack + .get(index) + .expect("environment index must be in range") + } +} diff --git a/boa_engine/src/tests/mod.rs b/boa_engine/src/tests/mod.rs index ed1020afb1b..8320c30c94d 100644 --- a/boa_engine/src/tests/mod.rs +++ b/boa_engine/src/tests/mod.rs @@ -403,7 +403,7 @@ fn strict_mode_with() { } "#}, ErrorKind::Syntax, - "unexpected token 'with', primary expression at line 3, col 5", + "with statement not allowed in strict mode at line 3, col 5", )]); } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 347ab3976d0..68ab9221308 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -444,6 +444,7 @@ impl CodeBlock { | Opcode::SuperCallSpread | Opcode::ForAwaitOfLoopIterate | Opcode::SetPrototype + | Opcode::PushObjectEnvironment | Opcode::Nop => String::new(), } } @@ -1474,6 +1475,7 @@ impl JsObject { .into()) } else { let function_env = environment + .declarative_expect() .slots() .expect("must be function environment") .as_function_slots() diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index bb959017cb7..d4528d57191 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -515,7 +515,8 @@ impl CodeBlock { | Opcode::SuperCallSpread | Opcode::ForAwaitOfLoopIterate | Opcode::SetPrototype - | Opcode::Nop => { + | Opcode::Nop + | Opcode::PushObjectEnvironment => { 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/define/mod.rs b/boa_engine/src/vm/opcode/define/mod.rs index 234047b45e4..30adcab0665 100644 --- a/boa_engine/src/vm/opcode/define/mod.rs +++ b/boa_engine/src/vm/opcode/define/mod.rs @@ -70,12 +70,16 @@ impl Operation for DefInitVar { binding_locator.throw_mutate_immutable(context)?; if binding_locator.is_global() { - let key = context - .interner() - .resolve_expect(binding_locator.name().sym()) - .into_common::(false) - .into(); - crate::object::internal_methods::global::global_set_no_receiver(&key, value, context)?; + if !context.put_value_if_global_poisoned(binding_locator.name(), &value)? { + let key = context + .interner() + .resolve_expect(binding_locator.name().sym()) + .into_common::(false) + .into(); + crate::object::internal_methods::global::global_set_no_receiver( + &key, value, context, + )?; + } } else { context.realm.environments.put_value( binding_locator.environment_index(), diff --git a/boa_engine/src/vm/opcode/delete/mod.rs b/boa_engine/src/vm/opcode/delete/mod.rs index a89c65b9683..118f56157e3 100644 --- a/boa_engine/src/vm/opcode/delete/mod.rs +++ b/boa_engine/src/vm/opcode/delete/mod.rs @@ -80,11 +80,18 @@ impl Operation for DeleteName { binding_locator.throw_mutate_immutable(context)?; let deleted = if binding_locator.is_global() - && context + && !context .realm .environments - .is_only_global_property(binding_locator.name()) + .binding_in_poisoned_environment(binding_locator.name()) { + let (found, deleted) = + context.delete_binding_from_objet_environment(binding_locator.name())?; + if found { + context.vm.push(deleted); + return Ok(CompletionType::Normal); + } + let key: JsString = context .interner() .resolve_expect(binding_locator.name().sym()) diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 559d2d86d3a..7a128716bb2 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -22,11 +22,7 @@ impl Operation for GetName { binding_locator.throw_mutate_immutable(context)?; let value = if binding_locator.is_global() { - if let Some(value) = context - .realm - .environments - .get_value_if_global_poisoned(binding_locator.name()) - { + if let Some(value) = context.get_value_if_global_poisoned(binding_locator.name())? { value } else { let key: JsString = context @@ -58,11 +54,11 @@ impl Operation for GetName { } } } - } else if let Some(value) = context.realm.environments.get_value_optional( + } else if let Some(value) = context.get_value_optional( binding_locator.environment_index(), binding_locator.binding_index(), binding_locator.name(), - ) { + )? { value } else { let name = context @@ -95,11 +91,7 @@ impl Operation for GetNameOrUndefined { let binding_locator = context.vm.frame().code_block.bindings[index as usize]; binding_locator.throw_mutate_immutable(context)?; let value = if binding_locator.is_global() { - if let Some(value) = context - .realm - .environments - .get_value_if_global_poisoned(binding_locator.name()) - { + if let Some(value) = context.get_value_if_global_poisoned(binding_locator.name())? { value } else { let key: JsString = context @@ -120,11 +112,11 @@ impl Operation for GetNameOrUndefined { _ => JsValue::undefined(), } } - } else if let Some(value) = context.realm.environments.get_value_optional( + } else if let Some(value) = context.get_value_optional( binding_locator.environment_index(), binding_locator.binding_index(), binding_locator.name(), - ) { + )? { value } else { JsValue::undefined() diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 96a738740f9..fd05207b28e 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -1308,6 +1308,13 @@ generate_impl! { /// Stack: **=>** PushDeclarativeEnvironment, + /// Push an object environment. + /// + /// Operands: + /// + /// Stack: object **=>** + PushObjectEnvironment, + /// Push a function environment. /// /// Operands: num_bindings: `u32`, compile_environments_index: `u32` diff --git a/boa_engine/src/vm/opcode/push/environment.rs b/boa_engine/src/vm/opcode/push/environment.rs index 2d1c7ea20ea..47a4f4c53e7 100644 --- a/boa_engine/src/vm/opcode/push/environment.rs +++ b/boa_engine/src/vm/opcode/push/environment.rs @@ -53,3 +53,24 @@ impl Operation for PushFunctionEnvironment { Ok(CompletionType::Normal) } } + +/// `PushObjectEnvironment` implements the Opcode Operation for `Opcode::PushObjectEnvironment` +/// +/// Operation: +/// - Push an object environment +#[derive(Debug, Clone, Copy)] +pub(crate) struct PushObjectEnvironment; + +impl Operation for PushObjectEnvironment { + const NAME: &'static str = "PushObjectEnvironment"; + const INSTRUCTION: &'static str = "INST - PushObjectEnvironment"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let object = context.vm.pop(); + let object = object.to_object(context)?; + + context.realm.environments.push_object(object); + context.vm.frame_mut().inc_frame_env_stack(); + Ok(CompletionType::Normal) + } +} diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index fa98134017a..e3f99f76a29 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -25,11 +25,7 @@ impl Operation for SetName { binding_locator.throw_mutate_immutable(context)?; if binding_locator.is_global() { - if !context - .realm - .environments - .put_value_if_global_poisoned(binding_locator.name(), &value) - { + if !context.put_value_if_global_poisoned(binding_locator.name(), &value)? { let key: JsString = context .interner() .resolve_expect(binding_locator.name().sym()) @@ -60,12 +56,12 @@ impl Operation for SetName { .into()); } } - } else if !context.realm.environments.put_value_if_initialized( + } else if !context.put_value_if_initialized( binding_locator.environment_index(), binding_locator.binding_index(), binding_locator.name(), value, - ) { + )? { return Err(JsNativeError::reference() .with_message(format!( "cannot access '{}' before initialization", diff --git a/boa_parser/src/parser/statement/mod.rs b/boa_parser/src/parser/statement/mod.rs index 5af335883d4..e1752301c93 100644 --- a/boa_parser/src/parser/statement/mod.rs +++ b/boa_parser/src/parser/statement/mod.rs @@ -20,6 +20,7 @@ mod switch; mod throw; mod try_stm; mod variable; +mod with; use self::{ block::BlockStatement, @@ -35,6 +36,7 @@ use self::{ throw::ThrowStatement, try_stm::TryStatement, variable::VariableStatement, + with::WithStatement, }; use crate::{ lexer::{ @@ -124,6 +126,11 @@ where let tok = cursor.peek(0, interner).or_abrupt()?; match tok.kind() { + TokenKind::Keyword((Keyword::With, _)) => { + WithStatement::new(self.allow_yield, self.allow_await, self.allow_return) + .parse(cursor, interner) + .map(ast::Statement::from) + } TokenKind::Keyword((Keyword::If, _)) => { IfStatement::new(self.allow_yield, self.allow_await, self.allow_return) .parse(cursor, interner) diff --git a/boa_parser/src/parser/statement/with/mod.rs b/boa_parser/src/parser/statement/with/mod.rs new file mode 100644 index 00000000000..c991dfdc777 --- /dev/null +++ b/boa_parser/src/parser/statement/with/mod.rs @@ -0,0 +1,89 @@ +//! With statement parsing. + +use crate::{ + parser::{ + cursor::Cursor, expression::Expression, statement::Statement, AllowAwait, AllowReturn, + AllowYield, ParseResult, TokenParser, + }, + Error, +}; +use boa_ast::{statement::With, Keyword, Punctuator}; +use boa_interner::Interner; +use boa_profiler::Profiler; +use std::io::Read; + +/// With statement parsing. +/// +/// More information: +/// - [MDN documentation][mdn] +/// - [ECMAScript specification][spec] +/// +/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/with +/// [spec]: https://tc39.es/ecma262/#prod-WithStatement +#[derive(Debug, Clone, Copy)] +pub(in crate::parser::statement) struct WithStatement { + allow_yield: AllowYield, + allow_await: AllowAwait, + allow_return: AllowReturn, +} + +impl WithStatement { + /// Creates a new `WithStatement` parser. + pub(in crate::parser::statement) fn new( + allow_yield: Y, + allow_await: A, + allow_return: R, + ) -> Self + where + Y: Into, + A: Into, + R: Into, + { + Self { + allow_yield: allow_yield.into(), + allow_await: allow_await.into(), + allow_return: allow_return.into(), + } + } +} + +impl TokenParser for WithStatement +where + R: Read, +{ + type Output = With; + + fn parse(self, cursor: &mut Cursor, interner: &mut Interner) -> ParseResult { + let _timer = Profiler::global().start_event("WithStatement", "Parsing"); + + let position = cursor + .expect((Keyword::With, false), "with statement", interner)? + .span() + .start(); + + // It is a Syntax Error if the source text matched by this production is contained in strict mode code. + if cursor.strict_mode() { + return Err(Error::general( + "with statement not allowed in strict mode", + position, + )); + } + + cursor.expect(Punctuator::OpenParen, "with statement", interner)?; + let expression = Expression::new(None, true, self.allow_yield, self.allow_await) + .parse(cursor, interner)?; + let position = cursor + .expect(Punctuator::CloseParen, "with statement", interner)? + .span() + .end(); + let statement = Statement::new(self.allow_yield, self.allow_await, self.allow_return) + .parse(cursor, interner)?; + + // It is a Syntax Error if IsLabelledFunction(Statement) is true. + if statement.is_labelled_function() { + return Err(Error::wrong_labelled_function_declaration(position)); + } + + Ok(With::new(expression, statement)) + } +}