Remove `mutate_immutable` and `silent` checks from Opcodes (#3024)

* Generate specialized opcode for binding errors

* Remove `mutate_immutable` and `silent` checks from Opcodes

* rust-fmt

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).

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<BindingLocator, BindingLocatorError> {
         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<BindingLocator, BindingLocatorError> {
         self.current_environment
             .borrow()
             .set_mutable_binding_var_recursive(name)

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);
                     }

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);
                 }

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),

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(

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<BindingLocator, BindingLocatorError> {
+        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<BindingLocator, BindingLocatorError> {
         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.

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,
     },
 };
 

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<'_> {

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::<u32>(*pc);
                 *pc += size_of::<u32>();
                 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"),
         }
     }
 }

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::<u32>(pc);
                     pc += size_of::<u32>();
                     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"),
             }
         }
 

boa_engine/src/vm/opcode/define/mod.rs

@@ -53,10 +53,6 @@ impl Operation for DefInitVar {
         let index = context.vm.read::<u32>();
         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)?;
 

boa_engine/src/vm/opcode/delete/mod.rs

@@ -74,7 +74,6 @@ impl Operation for DeleteName {
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
         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)?;
 

boa_engine/src/vm/opcode/get/name.rs

@@ -18,7 +18,6 @@ impl Operation for GetName {
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let index = context.vm.read::<u32>();
         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<CompletionType> {
         let index = context.vm.read::<u32>();
         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<CompletionType> {
         let index = context.vm.read::<u32>();
         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();
 

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,
     }
 }
 

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<CompletionType> {
+        let index = context.vm.read::<u32>();
+        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::<u32>();
         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)?;