diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index d95b54df80..2542894aa6 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -1,5 +1,6 @@ use crate::{ bytecompiler::{ByteCompiler, FunctionCompiler, FunctionSpec, Label, NodeKind}, + environments::BindingLocatorError, vm::{ create_function_object_fast, create_generator_function_object, BindingOpcode, CodeBlockFlags, Opcode, @@ -737,9 +738,19 @@ impl ByteCompiler<'_, '_> { // iii. Else, if binding_exists { // 1. Perform ! varEnv.SetMutableBinding(fn, fo, false). - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + match self.set_mutable_binding(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } else { // 1. NOTE: The following invocation cannot return an abrupt completion because of the validation preceding step 14. // 2. Perform ! varEnv.CreateMutableBinding(fn, true). diff --git a/boa_engine/src/bytecompiler/env.rs b/boa_engine/src/bytecompiler/env.rs index 31de3019f2..26aec1e811 100644 --- a/boa_engine/src/bytecompiler/env.rs +++ b/boa_engine/src/bytecompiler/env.rs @@ -1,5 +1,5 @@ use super::ByteCompiler; -use crate::environments::{BindingLocator, CompileTimeEnvironment}; +use crate::environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment}; use boa_ast::expression::Identifier; use boa_gc::{Gc, GcRefCell}; @@ -108,7 +108,10 @@ impl ByteCompiler<'_, '_> { } /// Return the binding locator for a set operation on an existing binding. - pub(crate) fn set_mutable_binding(&self, name: Identifier) -> BindingLocator { + pub(crate) fn set_mutable_binding( + &self, + name: Identifier, + ) -> Result { self.current_environment .borrow() .set_mutable_binding_recursive(name) @@ -116,7 +119,10 @@ impl ByteCompiler<'_, '_> { #[cfg(feature = "annex-b")] /// Return the binding locator for a set operation on an existing var binding. - pub(crate) fn set_mutable_binding_var(&self, name: Identifier) -> BindingLocator { + pub(crate) fn set_mutable_binding_var( + &self, + name: Identifier, + ) -> Result { self.current_environment .borrow() .set_mutable_binding_var_recursive(name) diff --git a/boa_engine/src/bytecompiler/expression/assign.rs b/boa_engine/src/bytecompiler/expression/assign.rs index a85e732bb9..5054b5767b 100644 --- a/boa_engine/src/bytecompiler/expression/assign.rs +++ b/boa_engine/src/bytecompiler/expression/assign.rs @@ -1,5 +1,6 @@ use crate::{ bytecompiler::{Access, ByteCompiler}, + environments::BindingLocatorError, vm::{BindingOpcode, Opcode}, }; use boa_ast::expression::{ @@ -75,9 +76,19 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Dup); } if lex { - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + match self.set_mutable_binding(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } else { self.emit_opcode(Opcode::SetNameByLocator); } diff --git a/boa_engine/src/bytecompiler/expression/update.rs b/boa_engine/src/bytecompiler/expression/update.rs index 79d7c38d10..cbca6a1203 100644 --- a/boa_engine/src/bytecompiler/expression/update.rs +++ b/boa_engine/src/bytecompiler/expression/update.rs @@ -1,5 +1,6 @@ use crate::{ bytecompiler::{Access, ByteCompiler}, + environments::BindingLocatorError, vm::Opcode, }; use boa_ast::expression::{ @@ -40,9 +41,19 @@ impl ByteCompiler<'_, '_> { } if lex { - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + match self.set_mutable_binding(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } else { self.emit_opcode(Opcode::SetNameByLocator); } diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index 3f703ac629..5aae0385c5 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -15,7 +15,7 @@ use std::cell::Cell; use crate::{ builtins::function::ThisMode, - environments::{BindingLocator, CompileTimeEnvironment}, + environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment}, js_string, vm::{BindingOpcode, CodeBlock, CodeBlockFlags, Opcode}, Context, JsBigInt, JsString, JsValue, @@ -367,13 +367,25 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { self.emit(Opcode::DefVar, &[index]); } BindingOpcode::InitVar => { - let binding = if self.has_binding(name) { - self.set_mutable_binding(name) + if self.has_binding(name) { + match self.set_mutable_binding(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::DefInitVar, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } else { - self.initialize_mutable_binding(name, true) + let binding = self.initialize_mutable_binding(name, true); + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::DefInitVar, &[index]); }; - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::DefInitVar, &[index]); } BindingOpcode::InitLet => { let binding = self.initialize_mutable_binding(name, false); @@ -385,11 +397,19 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { let index = self.get_or_insert_binding(binding); self.emit(Opcode::PutLexicalValue, &[index]); } - BindingOpcode::SetName => { - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); - } + BindingOpcode::SetName => match self.set_mutable_binding(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + }, } } @@ -618,9 +638,19 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { } if lex { - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + match self.set_mutable_binding(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } else { self.emit_opcode(Opcode::SetNameByLocator); } @@ -1032,9 +1062,20 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { let binding = self.get_binding_value(name); let index = self.get_or_insert_binding(binding); self.emit(Opcode::GetName, &[index]); - let binding = self.set_mutable_binding_var(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + + match self.set_mutable_binding_var(name) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(name); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } } Declaration::Class(class) => self.class(class, false), diff --git a/boa_engine/src/bytecompiler/statement/loop.rs b/boa_engine/src/bytecompiler/statement/loop.rs index 0c64acb20f..e2aa879bf5 100644 --- a/boa_engine/src/bytecompiler/statement/loop.rs +++ b/boa_engine/src/bytecompiler/statement/loop.rs @@ -10,6 +10,7 @@ use boa_interner::Sym; use crate::{ bytecompiler::{Access, ByteCompiler}, + environments::BindingLocatorError, vm::{BindingOpcode, Opcode}, }; @@ -323,9 +324,19 @@ impl ByteCompiler<'_, '_> { match for_of_loop.initializer() { IterableLoopInitializer::Identifier(ref ident) => { - let binding = self.set_mutable_binding(*ident); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::DefInitVar, &[index]); + match self.set_mutable_binding(*ident) { + Ok(binding) => { + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::DefInitVar, &[index]); + } + Err(BindingLocatorError::MutateImmutable) => { + let index = self.get_or_insert_name(*ident); + self.emit(Opcode::ThrowMutateImmutable, &[index]); + } + Err(BindingLocatorError::Silent) => { + self.emit(Opcode::Pop, &[]); + } + } } IterableLoopInitializer::Access(access) => { self.access_set( diff --git a/boa_engine/src/environments/compile.rs b/boa_engine/src/environments/compile.rs index 02d592099f..8aab0a0607 100644 --- a/boa_engine/src/environments/compile.rs +++ b/boa_engine/src/environments/compile.rs @@ -4,6 +4,8 @@ use boa_gc::{Finalize, Gc, GcRefCell, Trace}; use rustc_hash::FxHashMap; +use super::runtime::BindingLocatorError; + /// A compile time binding represents a binding at bytecode compile time in a [`CompileTimeEnvironment`]. /// /// It contains the binding index and a flag to indicate if this is a mutable binding or not. @@ -245,41 +247,47 @@ impl CompileTimeEnvironment { } /// Return the binding locator for a mutable binding. - pub(crate) fn set_mutable_binding_recursive(&self, name: Identifier) -> BindingLocator { - match self.bindings.get(&name) { + pub(crate) fn set_mutable_binding_recursive( + &self, + name: Identifier, + ) -> Result { + Ok(match self.bindings.get(&name) { Some(binding) if binding.mutable => { BindingLocator::declarative(name, self.environment_index, binding.index) } - Some(binding) if binding.strict => BindingLocator::mutate_immutable(name), - Some(_) => BindingLocator::silent(name), + Some(binding) if binding.strict => return Err(BindingLocatorError::MutateImmutable), + Some(_) => return Err(BindingLocatorError::Silent), None => self.outer.as_ref().map_or_else( - || BindingLocator::global(name), + || Ok(BindingLocator::global(name)), |outer| outer.borrow().set_mutable_binding_recursive(name), - ), - } + )?, + }) } #[cfg(feature = "annex-b")] /// Return the binding locator for a set operation on an existing var binding. - pub(crate) fn set_mutable_binding_var_recursive(&self, name: Identifier) -> BindingLocator { + pub(crate) fn set_mutable_binding_var_recursive( + &self, + name: Identifier, + ) -> Result { if !self.is_function() { return self.outer.as_ref().map_or_else( - || BindingLocator::global(name), + || Ok(BindingLocator::global(name)), |outer| outer.borrow().set_mutable_binding_var_recursive(name), ); } - match self.bindings.get(&name) { + Ok(match self.bindings.get(&name) { Some(binding) if binding.mutable => { BindingLocator::declarative(name, self.environment_index, binding.index) } - Some(binding) if binding.strict => BindingLocator::mutate_immutable(name), - Some(_) => BindingLocator::silent(name), + Some(binding) if binding.strict => return Err(BindingLocatorError::MutateImmutable), + Some(_) => return Err(BindingLocatorError::Silent), None => self.outer.as_ref().map_or_else( - || BindingLocator::global(name), + || Ok(BindingLocator::global(name)), |outer| outer.borrow().set_mutable_binding_var_recursive(name), - ), - } + )?, + }) } /// Gets the outer environment of this environment. diff --git a/boa_engine/src/environments/mod.rs b/boa_engine/src/environments/mod.rs index 458ea04e70..55b7259307 100644 --- a/boa_engine/src/environments/mod.rs +++ b/boa_engine/src/environments/mod.rs @@ -30,8 +30,8 @@ mod runtime; pub(crate) use { compile::CompileTimeEnvironment, runtime::{ - BindingLocator, DeclarativeEnvironment, Environment, EnvironmentStack, FunctionSlots, - PrivateEnvironment, ThisBindingStatus, + BindingLocator, BindingLocatorError, DeclarativeEnvironment, Environment, EnvironmentStack, + FunctionSlots, PrivateEnvironment, ThisBindingStatus, }, }; diff --git a/boa_engine/src/environments/runtime/mod.rs b/boa_engine/src/environments/runtime/mod.rs index 748948125b..2cb196ae59 100644 --- a/boa_engine/src/environments/runtime/mod.rs +++ b/boa_engine/src/environments/runtime/mod.rs @@ -1,6 +1,5 @@ use crate::{ environments::CompileTimeEnvironment, - error::JsNativeError, object::{JsObject, PrivateName}, Context, JsResult, JsString, JsSymbol, JsValue, }; @@ -524,8 +523,6 @@ pub(crate) struct BindingLocator { environment_index: u32, binding_index: u32, global: bool, - mutate_immutable: bool, - silent: bool, } unsafe impl Trace for BindingLocator { @@ -544,8 +541,6 @@ impl BindingLocator { environment_index, binding_index, global: false, - mutate_immutable: false, - silent: false, } } @@ -556,33 +551,6 @@ impl BindingLocator { environment_index: 0, binding_index: 0, global: true, - mutate_immutable: false, - silent: false, - } - } - - /// Creates a binding locator that indicates that it was attempted to mutate an immutable binding. - /// At runtime this should always produce a type error. - pub(in crate::environments) const fn mutate_immutable(name: Identifier) -> Self { - Self { - name, - environment_index: 0, - binding_index: 0, - global: false, - mutate_immutable: true, - silent: false, - } - } - - /// Creates a binding locator that indicates that any action is silently ignored. - pub(in crate::environments) const fn silent(name: Identifier) -> Self { - Self { - name, - environment_index: 0, - binding_index: 0, - global: false, - mutate_immutable: false, - silent: true, } } @@ -605,26 +573,15 @@ impl BindingLocator { pub(crate) const fn binding_index(&self) -> u32 { self.binding_index } +} - /// Returns if the binding is a silent operation. - pub(crate) const fn is_silent(&self) -> bool { - self.silent - } +/// Action that is returned when a fallible binding operation. +pub(crate) enum BindingLocatorError { + /// Trying to mutate immutable binding, + MutateImmutable, - /// Helper method to throws an error if the binding access is illegal. - pub(crate) fn throw_mutate_immutable( - &self, - context: &mut Context<'_>, - ) -> Result<(), JsNativeError> { - if self.mutate_immutable { - Err(JsNativeError::typ().with_message(format!( - "cannot mutate an immutable binding '{}'", - context.interner().resolve_expect(self.name.sym()) - ))) - } else { - Ok(()) - } - } + /// Indicates that any action is silently ignored. + Silent, } impl Context<'_> { diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index f9e77948ba..11bc89ba3f 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -410,7 +410,8 @@ impl CodeBlock { | Opcode::PushClassPrivateGetter | Opcode::PushClassPrivateSetter | Opcode::PushClassPrivateMethod - | Opcode::InPrivate => { + | Opcode::InPrivate + | Opcode::ThrowMutateImmutable => { let operand = self.read::(*pc); *pc += size_of::(); format!( @@ -609,8 +610,7 @@ impl CodeBlock { | Opcode::Reserved48 | Opcode::Reserved49 | Opcode::Reserved50 - | Opcode::Reserved51 - | Opcode::Reserved52 => unreachable!("Reserved opcodes are unrechable"), + | Opcode::Reserved51 => unreachable!("Reserved opcodes are unrechable"), } } } diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 3bd5e589c9..085f8fdd48 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -471,7 +471,8 @@ impl CodeBlock { | Opcode::PushClassPrivateGetter | Opcode::PushClassPrivateSetter | Opcode::PushClassPrivateMethod - | Opcode::InPrivate => { + | Opcode::InPrivate + | Opcode::ThrowMutateImmutable => { let operand = self.read::(pc); pc += size_of::(); let label = format!( @@ -708,8 +709,7 @@ impl CodeBlock { | Opcode::Reserved48 | Opcode::Reserved49 | Opcode::Reserved50 - | Opcode::Reserved51 - | Opcode::Reserved52 => unreachable!("Reserved opcodes are unrechable"), + | Opcode::Reserved51 => unreachable!("Reserved opcodes are unrechable"), } } diff --git a/boa_engine/src/vm/opcode/define/mod.rs b/boa_engine/src/vm/opcode/define/mod.rs index 086a3fd6ec..83baeced5f 100644 --- a/boa_engine/src/vm/opcode/define/mod.rs +++ b/boa_engine/src/vm/opcode/define/mod.rs @@ -53,10 +53,6 @@ impl Operation for DefInitVar { let index = context.vm.read::(); let value = context.vm.pop(); let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; - if binding_locator.is_silent() { - return Ok(CompletionType::Normal); - } - binding_locator.throw_mutate_immutable(context)?; context.find_runtime_binding(&mut binding_locator)?; diff --git a/boa_engine/src/vm/opcode/delete/mod.rs b/boa_engine/src/vm/opcode/delete/mod.rs index 8e412e1bc8..22129fb35b 100644 --- a/boa_engine/src/vm/opcode/delete/mod.rs +++ b/boa_engine/src/vm/opcode/delete/mod.rs @@ -74,7 +74,6 @@ impl Operation for DeleteName { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; - binding_locator.throw_mutate_immutable(context)?; context.find_runtime_binding(&mut binding_locator)?; diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index ad909e3307..3a067b370f 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -18,7 +18,6 @@ impl Operation for GetName { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; - binding_locator.throw_mutate_immutable(context)?; context.find_runtime_binding(&mut binding_locator)?; let value = context.get_binding(binding_locator)?.ok_or_else(|| { let name = context @@ -70,7 +69,6 @@ impl Operation for GetNameAndLocator { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; - binding_locator.throw_mutate_immutable(context)?; context.find_runtime_binding(&mut binding_locator)?; let value = context.get_binding(binding_locator)?.ok_or_else(|| { let name = context @@ -100,7 +98,6 @@ impl Operation for GetNameOrUndefined { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; - binding_locator.throw_mutate_immutable(context)?; let is_global = binding_locator.is_global(); diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 46d5010909..cac4b28b04 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -668,6 +668,13 @@ generate_impl! { /// Stack: value **=>** PutLexicalValue, + /// Throws an error because the binding access is illegal. + /// + /// Operands: binding_index: `u32` + /// + /// Stack: **=>** + ThrowMutateImmutable, + /// Find a binding on the environment chain and push its value. /// /// Operands: name_index: `u32` @@ -1814,8 +1821,6 @@ generate_impl! { Reserved50 => Reserved, /// Reserved [`Opcode`]. Reserved51 => Reserved, - /// Reserved [`Opcode`]. - Reserved52 => Reserved, } } diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index d15e400966..b8aa64eade 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -4,6 +4,30 @@ use crate::{ Context, JsNativeError, JsResult, }; +/// `ThrowMutateImmutable` implements the Opcode Operation for `Opcode::ThrowMutateImmutable` +/// +/// Operation: +/// - Throws an error because the binding access is illegal. +#[derive(Debug, Clone, Copy)] +pub(crate) struct ThrowMutateImmutable; + +impl Operation for ThrowMutateImmutable { + const NAME: &'static str = "ThrowMutateImmutable"; + const INSTRUCTION: &'static str = "INST - ThrowMutateImmutable"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let index = context.vm.read::(); + let name = &context.vm.frame().code_block.names[index as usize]; + + Err(JsNativeError::typ() + .with_message(format!( + "cannot mutate an immutable binding '{}'", + name.to_std_string_escaped() + )) + .into()) + } +} + /// `SetName` implements the Opcode Operation for `Opcode::SetName` /// /// Operation: @@ -19,10 +43,6 @@ impl Operation for SetName { let index = context.vm.read::(); let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; let value = context.vm.pop(); - if binding_locator.is_silent() { - return Ok(CompletionType::Normal); - } - binding_locator.throw_mutate_immutable(context)?; context.find_runtime_binding(&mut binding_locator)?; @@ -57,10 +77,6 @@ impl Operation for SetNameByLocator { .pop() .expect("locator should have been popped before"); let value = context.vm.pop(); - if binding_locator.is_silent() { - return Ok(CompletionType::Normal); - } - binding_locator.throw_mutate_immutable(context)?; verify_initialized(binding_locator, context)?;