From e29d03d4cb93cd630ee6e8cdf75b8f8a11151ced Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Wed, 26 Apr 2023 00:31:56 -0600 Subject: [PATCH 1/9] Make update operations reuse the last found binding locator --- boa_engine/src/builtins/generator/mod.rs | 10 ++- boa_engine/src/builtins/json/mod.rs | 1 - boa_engine/src/bytecompiler/class.rs | 3 +- .../src/bytecompiler/expression/assign.rs | 19 +++-- .../src/bytecompiler/expression/update.rs | 18 ++++- boa_engine/src/bytecompiler/function.rs | 1 - boa_engine/src/bytecompiler/mod.rs | 5 +- .../src/bytecompiler/statement/block.rs | 1 - .../src/bytecompiler/statement/switch.rs | 17 ++-- boa_engine/src/bytecompiler/statement/try.rs | 3 - boa_engine/src/context/mod.rs | 1 - boa_engine/src/environments/compile.rs | 11 +++ boa_engine/src/environments/runtime.rs | 22 +++-- boa_engine/src/vm/code_block.rs | 4 + boa_engine/src/vm/flowgraph/mod.rs | 2 + boa_engine/src/vm/mod.rs | 5 +- boa_engine/src/vm/opcode/get/name.rs | 30 +++++++ boa_engine/src/vm/opcode/mod.rs | 13 +++ boa_engine/src/vm/opcode/set/name.rs | 80 ++++++++++++++----- 19 files changed, 191 insertions(+), 55 deletions(-) diff --git a/boa_engine/src/builtins/generator/mod.rs b/boa_engine/src/builtins/generator/mod.rs index bd177be9937..a54e51ea6c4 100644 --- a/boa_engine/src/builtins/generator/mod.rs +++ b/boa_engine/src/builtins/generator/mod.rs @@ -1,4 +1,4 @@ -//! Boa's implementation of ECMAScript's global `Generator` object. +//! Boa's 3tion of ECMAScript's global `Generator` object. //! //! A Generator is an instance of a generator function and conforms to both the Iterator and Iterable interfaces. //! @@ -12,7 +12,7 @@ use crate::{ builtins::iterable::create_iter_result_object, context::intrinsics::Intrinsics, - environments::DeclarativeEnvironmentStack, + environments::{BindingLocator, DeclarativeEnvironmentStack}, error::JsNativeError, object::{JsObject, CONSTRUCTOR}, property::Attribute, @@ -63,6 +63,7 @@ pub(crate) struct GeneratorContext { pub(crate) active_function: Option, pub(crate) call_frame: Option, pub(crate) realm: Realm, + pub(crate) bindings_stack: Vec, } impl GeneratorContext { @@ -73,6 +74,7 @@ impl GeneratorContext { active_function: Option, call_frame: CallFrame, realm: Realm, + bindings_stack: Vec, ) -> Self { Self { environments, @@ -80,6 +82,7 @@ impl GeneratorContext { active_function, call_frame: Some(call_frame), realm, + bindings_stack, } } @@ -91,6 +94,7 @@ impl GeneratorContext { stack: context.vm.stack.clone(), active_function: context.vm.active_function.clone(), realm: context.realm().clone(), + bindings_stack: context.vm.bindings_stack.clone(), } } @@ -104,6 +108,7 @@ impl GeneratorContext { std::mem::swap(&mut context.vm.environments, &mut self.environments); std::mem::swap(&mut context.vm.stack, &mut self.stack); std::mem::swap(&mut context.vm.active_function, &mut self.active_function); + std::mem::swap(&mut context.vm.bindings_stack, &mut self.bindings_stack); context.swap_realm(&mut self.realm); context .vm @@ -118,6 +123,7 @@ impl GeneratorContext { std::mem::swap(&mut context.vm.environments, &mut self.environments); std::mem::swap(&mut context.vm.stack, &mut self.stack); std::mem::swap(&mut context.vm.active_function, &mut self.active_function); + std::mem::swap(&mut context.vm.bindings_stack, &mut self.bindings_stack); context.swap_realm(&mut self.realm); self.call_frame = context.vm.pop_frame(); assert!(self.call_frame.is_some()); diff --git a/boa_engine/src/builtins/json/mod.rs b/boa_engine/src/builtins/json/mod.rs index 8c07c8e99db..d079d5e7eda 100644 --- a/boa_engine/src/builtins/json/mod.rs +++ b/boa_engine/src/builtins/json/mod.rs @@ -123,7 +123,6 @@ impl Json { context.realm().environment().compile_env(), context, ); - compiler.create_script_decls(&statement_list, false); compiler.compile_statement_list(&statement_list, true, false); Gc::new(compiler.finish()) }; diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index 78d2042ab6a..c9efa579cfb 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/boa_engine/src/bytecompiler/class.rs @@ -78,7 +78,6 @@ impl ByteCompiler<'_, '_> { } else { None }; - compiler.create_script_decls(expr.body(), false); compiler.compile_statement_list(expr.body(), false, false); let env_info = compiler.pop_compile_environment(); @@ -397,7 +396,7 @@ impl ByteCompiler<'_, '_> { compiler.push_compile_environment(false); compiler.create_immutable_binding(class_name.into(), true); compiler.push_compile_environment(true); - compiler.create_script_decls(statement_list, false); + compiler.create_declarations(statement_list, false); compiler.compile_statement_list(statement_list, false, false); let env_info = compiler.pop_compile_environment(); compiler.pop_compile_environment(); diff --git a/boa_engine/src/bytecompiler/expression/assign.rs b/boa_engine/src/bytecompiler/expression/assign.rs index ac9a5fbaecf..dbccafb97ee 100644 --- a/boa_engine/src/bytecompiler/expression/assign.rs +++ b/boa_engine/src/bytecompiler/expression/assign.rs @@ -56,7 +56,13 @@ impl ByteCompiler<'_, '_> { Access::Variable { name } => { let binding = self.get_binding_value(name); let index = self.get_or_insert_binding(binding); - self.emit(Opcode::GetName, &[index]); + let lex = self.current_environment.borrow().is_lex_binding(name); + + if lex { + self.emit(Opcode::GetName, &[index]); + } else { + self.emit(Opcode::GetNameAndLocator, &[index]); + } if short_circuit { early_exit = Some(self.emit_opcode_with_operand(opcode)); @@ -68,10 +74,13 @@ impl ByteCompiler<'_, '_> { if use_expr { self.emit_opcode(Opcode::Dup); } - - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + if lex { + let binding = self.set_mutable_binding(name); + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } else { + self.emit_opcode(Opcode::SetNameByBinding); + } } Access::Property { access } => match access { PropertyAccess::Simple(access) => match access.field() { diff --git a/boa_engine/src/bytecompiler/expression/update.rs b/boa_engine/src/bytecompiler/expression/update.rs index b7becf9a118..8a52d85b0f7 100644 --- a/boa_engine/src/bytecompiler/expression/update.rs +++ b/boa_engine/src/bytecompiler/expression/update.rs @@ -24,7 +24,13 @@ impl ByteCompiler<'_, '_> { Access::Variable { name } => { let binding = self.get_binding_value(name); let index = self.get_or_insert_binding(binding); - self.emit(Opcode::GetName, &[index]); + let lex = self.current_environment.borrow().is_lex_binding(name); + + if lex { + self.emit(Opcode::GetName, &[index]); + } else { + self.emit(Opcode::GetNameAndLocator, &[index]); + } self.emit_opcode(opcode); if post { @@ -33,9 +39,13 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Dup); } - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + if lex { + let binding = self.set_mutable_binding(name); + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } else { + self.emit_opcode(Opcode::SetNameByBinding); + } } Access::Property { access } => match access { PropertyAccess::Simple(access) => match access.field() { diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs index 85ec537ead1..5473d48f568 100644 --- a/boa_engine/src/bytecompiler/function.rs +++ b/boa_engine/src/bytecompiler/function.rs @@ -189,7 +189,6 @@ impl FunctionCompiler { compiler.emit_opcode(Opcode::Yield); } - compiler.create_script_decls(body, false); compiler.compile_statement_list(body, false, false); if let Some(env_label) = env_label { diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index 8aa91c205e3..f081cf80c0f 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -736,6 +736,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { use_expr: bool, configurable_globals: bool, ) { + self.create_declarations(list, configurable_globals); if use_expr { let expr_index = list .statements() @@ -770,7 +771,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { self.push_compile_environment(strict); let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); - self.create_script_decls(list, true); + self.create_declarations(list, true); if use_expr { let expr_index = list @@ -1349,7 +1350,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { } /// Creates the declarations for a script. - pub(crate) fn create_script_decls( + pub(crate) fn create_declarations( &mut self, stmt_list: &StatementList, configurable_globals: bool, diff --git a/boa_engine/src/bytecompiler/statement/block.rs b/boa_engine/src/bytecompiler/statement/block.rs index cd74d73bcab..46a63c8c8af 100644 --- a/boa_engine/src/bytecompiler/statement/block.rs +++ b/boa_engine/src/bytecompiler/statement/block.rs @@ -13,7 +13,6 @@ impl ByteCompiler<'_, '_> { self.push_compile_environment(false); let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); - self.create_script_decls(block.statement_list(), configurable_globals); self.compile_statement_list(block.statement_list(), use_expr, configurable_globals); let env_info = self.pop_compile_environment(); diff --git a/boa_engine/src/bytecompiler/statement/switch.rs b/boa_engine/src/bytecompiler/statement/switch.rs index 31babac24dc..82cc559ca17 100644 --- a/boa_engine/src/bytecompiler/statement/switch.rs +++ b/boa_engine/src/bytecompiler/statement/switch.rs @@ -4,10 +4,15 @@ use boa_ast::statement::Switch; impl ByteCompiler<'_, '_> { /// Compile a [`Switch`] `boa_ast` node pub(crate) fn compile_switch(&mut self, switch: &Switch, configurable_globals: bool) { + self.compile_expr(switch.val(), true); + self.push_compile_environment(false); let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); for case in switch.cases() { - self.create_script_decls(case.body(), configurable_globals); + self.create_declarations(case.body(), configurable_globals); + } + if let Some(body) = switch.default() { + self.create_declarations(body, configurable_globals); } let (start_label, end_label) = self.emit_opcode_with_two_operands(Opcode::LoopStart); @@ -15,7 +20,6 @@ impl ByteCompiler<'_, '_> { self.push_switch_control_info(None, start_address); self.patch_jump_with_target(start_label, start_address); - self.compile_expr(switch.val(), true); let mut labels = Vec::with_capacity(switch.cases().len()); for case in switch.cases() { self.compile_expr(case.condition(), true); @@ -26,13 +30,16 @@ impl ByteCompiler<'_, '_> { for (label, case) in labels.into_iter().zip(switch.cases()) { self.patch_jump(label); - self.compile_statement_list(case.body(), false, configurable_globals); + for item in case.body().statements() { + self.compile_stmt_list_item(item, false, configurable_globals); + } } self.patch_jump(exit); if let Some(body) = switch.default() { - self.create_script_decls(body, configurable_globals); - self.compile_statement_list(body, false, configurable_globals); + for item in body.statements() { + self.compile_stmt_list_item(item, false, configurable_globals); + } } self.pop_switch_control_info(); diff --git a/boa_engine/src/bytecompiler/statement/try.rs b/boa_engine/src/bytecompiler/statement/try.rs index 10b3ac32ee7..f10e7514284 100644 --- a/boa_engine/src/bytecompiler/statement/try.rs +++ b/boa_engine/src/bytecompiler/statement/try.rs @@ -23,7 +23,6 @@ impl ByteCompiler<'_, '_> { self.push_compile_environment(false); let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); - self.create_script_decls(t.block().statement_list(), configurable_globals); self.compile_statement_list(t.block().statement_list(), use_expr, configurable_globals); let env_info = self.pop_compile_environment(); @@ -89,7 +88,6 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::Pop); } - self.create_script_decls(catch.block().statement_list(), configurable_globals); self.compile_statement_list( catch.block().statement_list(), use_expr, @@ -119,7 +117,6 @@ impl ByteCompiler<'_, '_> { self.push_compile_environment(false); let push_env = self.emit_opcode_with_two_operands(Opcode::PushDeclarativeEnvironment); - self.create_script_decls(finally.block().statement_list(), configurable_globals); self.compile_statement_list( finally.block().statement_list(), false, diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index e3127013f6f..e551c6317fd 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -253,7 +253,6 @@ impl<'host> Context<'host> { self.realm.environment().compile_env(), self, ); - compiler.create_script_decls(statement_list, false); compiler.compile_statement_list(statement_list, true, false); Ok(Gc::new(compiler.finish())) } diff --git a/boa_engine/src/environments/compile.rs b/boa_engine/src/environments/compile.rs index 13ecdf2f884..b95bcfc43af 100644 --- a/boa_engine/src/environments/compile.rs +++ b/boa_engine/src/environments/compile.rs @@ -56,6 +56,17 @@ impl CompileTimeEnvironment { .map_or(false, |binding| binding.lex) } + /// Checks if `name` is a lexical binding. + pub(crate) fn is_lex_binding(&self, name: Identifier) -> bool { + if let Some(binding) = self.bindings.get(&name) { + binding.lex + } else if let Some(outer) = &self.outer { + outer.borrow().is_lex_binding(name) + } else { + false + } + } + /// Returns the number of bindings in this environment. pub(crate) fn num_bindings(&self) -> usize { self.bindings.len() diff --git a/boa_engine/src/environments/runtime.rs b/boa_engine/src/environments/runtime.rs index ac7dc8452b5..9136c7ba4d9 100644 --- a/boa_engine/src/environments/runtime.rs +++ b/boa_engine/src/environments/runtime.rs @@ -3,7 +3,7 @@ use crate::{ JsResult, JsString, JsSymbol, JsValue, }; use boa_ast::expression::Identifier; -use boa_gc::{Finalize, Gc, GcRefCell, Trace}; +use boa_gc::{empty_trace, Finalize, Gc, GcRefCell, Trace}; use rustc_hash::FxHashSet; use std::cell::Cell; @@ -663,7 +663,7 @@ impl DeclarativeEnvironmentStack { /// A binding locator contains all information about a binding that is needed to resolve it at runtime. /// /// Binding locators get created at bytecode compile time and are accessible at runtime via the [`crate::vm::CodeBlock`]. -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Finalize)] pub(crate) struct BindingLocator { name: Identifier, environment_index: usize, @@ -673,6 +673,10 @@ pub(crate) struct BindingLocator { silent: bool, } +unsafe impl Trace for BindingLocator { + empty_trace!(); +} + impl BindingLocator { /// Creates a new declarative binding locator that has knows indices. pub(crate) const fn declarative( @@ -691,7 +695,7 @@ impl BindingLocator { } /// Creates a binding locator that indicates that the binding is on the global object. - pub(in crate::environments) const fn global(name: Identifier) -> Self { + pub(crate) const fn global(name: Identifier) -> Self { Self { name, environment_index: 0, @@ -844,7 +848,13 @@ impl Context<'_> { Environment::Declarative(env) => { Ok(env.bindings.borrow()[locator.binding_index].is_some()) } - Environment::Object(_) => Ok(true), + Environment::Object(obj) => { + let key: JsString = self + .interner() + .resolve_expect(locator.name.sym()) + .into_common(false); + obj.clone().has_property(key, self) + } } } } @@ -885,6 +895,8 @@ impl Context<'_> { /// Sets the value of a binding. /// + /// # + /// /// # Panics /// /// Panics if the environment or binding index are out of range. @@ -953,7 +965,7 @@ impl Context<'_> { } /// Return the environment at the given index. Panics if the index is out of range. - fn environment_expect(&self, index: usize) -> &Environment { + pub(crate) fn environment_expect(&self, index: usize) -> &Environment { self.vm .environments .stack diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 2d44baf459e..a066028c83c 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -335,6 +335,7 @@ impl CodeBlock { | Opcode::DefInitLet | Opcode::DefInitConst | Opcode::GetName + | Opcode::GetNameAndLocator | Opcode::GetNameOrUndefined | Opcode::SetName | Opcode::DeleteName => { @@ -495,6 +496,7 @@ impl CodeBlock { | Opcode::SetPrototype | Opcode::PushObjectEnvironment | Opcode::IsObject + | Opcode::SetNameByBinding | Opcode::Nop => String::new(), } } @@ -1099,6 +1101,7 @@ impl JsObject { context.vm.active_function.clone(), call_frame, context.realm().clone(), + context.vm.bindings_stack.clone(), )), queue: VecDeque::new(), }) @@ -1111,6 +1114,7 @@ impl JsObject { context.vm.active_function.clone(), call_frame, context.realm().clone(), + context.vm.bindings_stack.clone(), ), }, }) diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 80c9b1dbeca..157a2e93f3b 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -403,6 +403,7 @@ impl CodeBlock { | Opcode::DefInitLet | Opcode::DefInitConst | Opcode::GetName + | Opcode::GetNameAndLocator | Opcode::GetNameOrUndefined | Opcode::SetName | Opcode::DeleteName => { @@ -566,6 +567,7 @@ impl CodeBlock { | Opcode::SuperCallPrepare | Opcode::SetPrototype | Opcode::IsObject + | Opcode::SetNameByBinding | Opcode::Nop | Opcode::PushObjectEnvironment => { graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index baf40f41353..03033585408 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -6,7 +6,7 @@ use crate::{ builtins::async_generator::{AsyncGenerator, AsyncGeneratorState}, - environments::{DeclarativeEnvironment, DeclarativeEnvironmentStack}, + environments::{BindingLocator, DeclarativeEnvironment, DeclarativeEnvironmentStack}, vm::code_block::Readable, Context, JsError, JsObject, JsResult, JsValue, }; @@ -40,6 +40,7 @@ pub(crate) use { #[cfg(test)] mod tests; + /// Virtual Machine. #[derive(Debug)] pub struct Vm { @@ -51,6 +52,7 @@ pub struct Vm { pub(crate) trace: bool, pub(crate) stack_size_limit: usize, pub(crate) active_function: Option, + pub(crate) bindings_stack: Vec, } impl Vm { @@ -65,6 +67,7 @@ impl Vm { trace: false, stack_size_limit: 1024, active_function: None, + bindings_stack: Vec::default(), } } diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 704731c262c..53dfd7200cd 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -33,6 +33,36 @@ impl Operation for GetName { } } +/// `GetNameAndLocator` implements the Opcode Operation for `Opcode::GetNameAndLocator` +/// +/// Operation: +/// - Find a binding on the environment chain and push its value. +#[derive(Debug, Clone, Copy)] +pub(crate) struct GetNameAndLocator; + +impl Operation for GetNameAndLocator { + const NAME: &'static str = "GetNameAndLocator"; + const INSTRUCTION: &'static str = "INST - 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 + .interner() + .resolve_expect(binding_locator.name().sym()) + .to_string(); + JsNativeError::reference().with_message(format!("{name} is not defined")) + })?; + + context.vm.bindings_stack.push(binding_locator); + context.vm.push(value); + Ok(CompletionType::Normal) + } +} + /// `GetNameOrUndefined` implements the Opcode Operation for `Opcode::GetNameOrUndefined` /// /// Operation: diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index fa24b994317..3e6dd3a9926 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -687,6 +687,14 @@ generate_impl! { /// Stack: **=>** value GetName, + /// Find a binding on the environment chain and push its value, storing the found binding locator + /// in the `current_binding` register. + /// + /// Operands: name_index: `u32` + /// + /// Stack: **=>** value + GetNameAndLocator, + /// Find a binding on the environment chain and push its value. If the binding does not exist push undefined. /// /// Operands: name_index: `u32` @@ -701,6 +709,11 @@ generate_impl! { /// Stack: value **=>** SetName, + /// Assigns a value to the binding pointed by the `current_binding` register. + /// + /// Stack: value **=>** + SetNameByBinding, + /// Deletes a property of the global object. /// /// Operands: name_index: `u32` diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index 1cd843f990c..3b8d332a360 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -1,4 +1,5 @@ use crate::{ + environments::{BindingLocator, Environment}, vm::{opcode::Operation, CompletionType}, Context, JsNativeError, JsResult, }; @@ -25,34 +26,69 @@ impl Operation for SetName { context.find_runtime_binding(&mut binding_locator)?; - if !context.is_initialized_binding(&binding_locator)? { - if binding_locator.is_global() && context.vm.frame().code_block.strict { - let key = context - .interner() - .resolve_expect(binding_locator.name().sym()) - .to_string(); - - return Err(JsNativeError::reference() - .with_message(format!( - "cannot assign to uninitialized global property `{key}`" - )) - .into()); - } + verify_initialized(binding_locator, context)?; - if !binding_locator.is_global() { - let key = context - .interner() - .resolve_expect(binding_locator.name().sym()) - .to_string(); + context.set_binding(binding_locator, value, context.vm.frame().code_block.strict)?; - return Err(JsNativeError::reference() - .with_message(format!("cannot assign to uninitialized binding `{key}`")) - .into()); - } + Ok(CompletionType::Normal) + } +} + +/// `SetNameByBinding` implements the Opcode Operation for `Opcode::SetNameByBinding` +/// +/// Operation: +/// - Assigns a value to the binding pointed by the `current_binding` register. +#[derive(Debug, Clone, Copy)] +pub(crate) struct SetNameByBinding; + +impl Operation for SetNameByBinding { + const NAME: &'static str = "SetNameByBinding"; + const INSTRUCTION: &'static str = "INST - SetNameByBinding"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let binding_locator = context + .vm + .bindings_stack + .pop() + .expect("a binding should have been pushed 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)?; context.set_binding(binding_locator, value, context.vm.frame().code_block.strict)?; Ok(CompletionType::Normal) } } + +/// Checks that the binding pointed by `locator` exists and is initialized. +fn verify_initialized(locator: BindingLocator, context: &mut Context<'_>) -> JsResult<()> { + if !context.is_initialized_binding(&locator)? { + let key = context.interner().resolve_expect(locator.name().sym()); + let strict = context.vm.frame().code_block.strict; + + let message = if locator.is_global() { + strict.then(|| format!("cannot assign to uninitialized global property `{key}`")) + } else { + match context.environment_expect(locator.environment_index()) { + Environment::Declarative(_) => { + Some(format!("cannot assign to uninitialized binding `{key}`")) + } + Environment::Object(_) if strict => { + Some(format!("cannot assign to uninitialized property `{key}`")) + } + Environment::Object(_) => None, + } + }; + + if let Some(message) = message { + return Err(JsNativeError::reference().with_message(message).into()); + } + } + + Ok(()) +} From 6bc275ba48494677bc4341f9efeb8c983fa1ddd1 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 27 Apr 2023 14:25:51 -0600 Subject: [PATCH 2/9] Rename opcode --- boa_engine/src/bytecompiler/expression/assign.rs | 2 +- boa_engine/src/bytecompiler/expression/update.rs | 2 +- boa_engine/src/vm/code_block.rs | 2 +- boa_engine/src/vm/flowgraph/mod.rs | 2 +- boa_engine/src/vm/opcode/mod.rs | 4 ++-- boa_engine/src/vm/opcode/set/name.rs | 12 ++++++------ 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/boa_engine/src/bytecompiler/expression/assign.rs b/boa_engine/src/bytecompiler/expression/assign.rs index dbccafb97ee..a85e732bb95 100644 --- a/boa_engine/src/bytecompiler/expression/assign.rs +++ b/boa_engine/src/bytecompiler/expression/assign.rs @@ -79,7 +79,7 @@ impl ByteCompiler<'_, '_> { let index = self.get_or_insert_binding(binding); self.emit(Opcode::SetName, &[index]); } else { - self.emit_opcode(Opcode::SetNameByBinding); + self.emit_opcode(Opcode::SetNameByLocator); } } Access::Property { access } => match access { diff --git a/boa_engine/src/bytecompiler/expression/update.rs b/boa_engine/src/bytecompiler/expression/update.rs index 8a52d85b0f7..79d7c38d102 100644 --- a/boa_engine/src/bytecompiler/expression/update.rs +++ b/boa_engine/src/bytecompiler/expression/update.rs @@ -44,7 +44,7 @@ impl ByteCompiler<'_, '_> { let index = self.get_or_insert_binding(binding); self.emit(Opcode::SetName, &[index]); } else { - self.emit_opcode(Opcode::SetNameByBinding); + self.emit_opcode(Opcode::SetNameByLocator); } } Access::Property { access } => match access { diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index a066028c83c..30926b7fd99 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -496,7 +496,7 @@ impl CodeBlock { | Opcode::SetPrototype | Opcode::PushObjectEnvironment | Opcode::IsObject - | Opcode::SetNameByBinding + | Opcode::SetNameByLocator | Opcode::Nop => String::new(), } } diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 157a2e93f3b..52e58a35568 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -567,7 +567,7 @@ impl CodeBlock { | Opcode::SuperCallPrepare | Opcode::SetPrototype | Opcode::IsObject - | Opcode::SetNameByBinding + | Opcode::SetNameByLocator | Opcode::Nop | Opcode::PushObjectEnvironment => { graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 3e6dd3a9926..a0ae6e7109a 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -709,10 +709,10 @@ generate_impl! { /// Stack: value **=>** SetName, - /// Assigns a value to the binding pointed by the `current_binding` register. + /// Assigns a value to the binding pointed by the top of the `bindings_stack`. /// /// Stack: value **=>** - SetNameByBinding, + SetNameByLocator, /// Deletes a property of the global object. /// diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index 3b8d332a360..58bd6b2a37c 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -34,16 +34,16 @@ impl Operation for SetName { } } -/// `SetNameByBinding` implements the Opcode Operation for `Opcode::SetNameByBinding` +/// `SetNameByLocator` implements the Opcode Operation for `Opcode::SetNameByLocator` /// /// Operation: -/// - Assigns a value to the binding pointed by the `current_binding` register. +/// - Assigns a value to the binding pointed by the top of the `bindings_stack`. #[derive(Debug, Clone, Copy)] -pub(crate) struct SetNameByBinding; +pub(crate) struct SetNameByLocator; -impl Operation for SetNameByBinding { - const NAME: &'static str = "SetNameByBinding"; - const INSTRUCTION: &'static str = "INST - SetNameByBinding"; +impl Operation for SetNameByLocator { + const NAME: &'static str = "SetNameByLocator"; + const INSTRUCTION: &'static str = "INST - SetNameByLocator"; fn execute(context: &mut Context<'_>) -> JsResult { let binding_locator = context From 12c40c8a48964281414ce47951a68c2462e59c4e Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 27 Apr 2023 14:28:27 -0600 Subject: [PATCH 3/9] Reword opcode comments --- boa_engine/src/vm/opcode/get/name.rs | 3 ++- boa_engine/src/vm/opcode/mod.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 53dfd7200cd..90c702719dd 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -36,7 +36,8 @@ impl Operation for GetName { /// `GetNameAndLocator` implements the Opcode Operation for `Opcode::GetNameAndLocator` /// /// Operation: -/// - Find a binding on the environment chain and push its value. +/// - Find a binding on the environment chain and push its value to the stack and its +/// `BindingLocator` to the `bindings_stack`. #[derive(Debug, Clone, Copy)] pub(crate) struct GetNameAndLocator; diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index a0ae6e7109a..8cc257382c3 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -687,8 +687,8 @@ generate_impl! { /// Stack: **=>** value GetName, - /// Find a binding on the environment chain and push its value, storing the found binding locator - /// in the `current_binding` register. + /// Find a binding on the environment chain and push its value to the stack and its + /// `BindingLocator` to the `bindings_stack`. /// /// Operands: name_index: `u32` /// From fbbe59a33cf54cc5393ce3e598aa09898a6814b2 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 27 Apr 2023 14:31:33 -0600 Subject: [PATCH 4/9] Change capacity of `bindings_stack` --- boa_engine/src/vm/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index 03033585408..fdc8e2404f5 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -67,7 +67,7 @@ impl Vm { trace: false, stack_size_limit: 1024, active_function: None, - bindings_stack: Vec::default(), + bindings_stack: Vec::with_capacity(16), } } From 45d97ec606d174744562fa5d71326e517e441b9f Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 27 Apr 2023 14:43:09 -0600 Subject: [PATCH 5/9] Use the callframe stack to store the current binding --- boa_engine/src/builtins/generator/mod.rs | 8 +------- boa_engine/src/vm/call_frame/mod.rs | 10 +++++++++- boa_engine/src/vm/code_block.rs | 2 -- boa_engine/src/vm/mod.rs | 4 +--- boa_engine/src/vm/opcode/get/name.rs | 6 +++--- boa_engine/src/vm/opcode/set/name.rs | 8 ++------ 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/boa_engine/src/builtins/generator/mod.rs b/boa_engine/src/builtins/generator/mod.rs index a54e51ea6c4..2c314d0e03d 100644 --- a/boa_engine/src/builtins/generator/mod.rs +++ b/boa_engine/src/builtins/generator/mod.rs @@ -12,7 +12,7 @@ use crate::{ builtins::iterable::create_iter_result_object, context::intrinsics::Intrinsics, - environments::{BindingLocator, DeclarativeEnvironmentStack}, + environments::DeclarativeEnvironmentStack, error::JsNativeError, object::{JsObject, CONSTRUCTOR}, property::Attribute, @@ -63,7 +63,6 @@ pub(crate) struct GeneratorContext { pub(crate) active_function: Option, pub(crate) call_frame: Option, pub(crate) realm: Realm, - pub(crate) bindings_stack: Vec, } impl GeneratorContext { @@ -74,7 +73,6 @@ impl GeneratorContext { active_function: Option, call_frame: CallFrame, realm: Realm, - bindings_stack: Vec, ) -> Self { Self { environments, @@ -82,7 +80,6 @@ impl GeneratorContext { active_function, call_frame: Some(call_frame), realm, - bindings_stack, } } @@ -94,7 +91,6 @@ impl GeneratorContext { stack: context.vm.stack.clone(), active_function: context.vm.active_function.clone(), realm: context.realm().clone(), - bindings_stack: context.vm.bindings_stack.clone(), } } @@ -108,7 +104,6 @@ impl GeneratorContext { std::mem::swap(&mut context.vm.environments, &mut self.environments); std::mem::swap(&mut context.vm.stack, &mut self.stack); std::mem::swap(&mut context.vm.active_function, &mut self.active_function); - std::mem::swap(&mut context.vm.bindings_stack, &mut self.bindings_stack); context.swap_realm(&mut self.realm); context .vm @@ -123,7 +118,6 @@ impl GeneratorContext { std::mem::swap(&mut context.vm.environments, &mut self.environments); std::mem::swap(&mut context.vm.stack, &mut self.stack); std::mem::swap(&mut context.vm.active_function, &mut self.active_function); - std::mem::swap(&mut context.vm.bindings_stack, &mut self.bindings_stack); context.swap_realm(&mut self.realm); self.call_frame = context.vm.pop_frame(); assert!(self.call_frame.is_some()); diff --git a/boa_engine/src/vm/call_frame/mod.rs b/boa_engine/src/vm/call_frame/mod.rs index f8dc16d9fd3..ce969a479fd 100644 --- a/boa_engine/src/vm/call_frame/mod.rs +++ b/boa_engine/src/vm/call_frame/mod.rs @@ -5,8 +5,12 @@ mod abrupt_record; mod env_stack; -use crate::{builtins::promise::PromiseCapability, object::JsObject, vm::CodeBlock}; +use crate::{ + builtins::promise::PromiseCapability, environments::BindingLocator, object::JsObject, + vm::CodeBlock, +}; use boa_gc::{Finalize, Gc, Trace}; +use boa_interner::Sym; use thin_vec::ThinVec; pub(crate) use abrupt_record::AbruptCompletionRecord; @@ -39,6 +43,9 @@ pub struct CallFrame { // Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown. pub(crate) iterators: ThinVec<(JsObject, bool)>, + + // The current binding being updated. + pub(crate) current_binding: BindingLocator, } /// ---- `CallFrame` public API ---- @@ -69,6 +76,7 @@ impl CallFrame { promise_capability: None, async_generator: None, iterators: ThinVec::new(), + current_binding: BindingLocator::global(Sym::EMPTY_STRING.into()), } } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 30926b7fd99..651cce90b6e 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -1101,7 +1101,6 @@ impl JsObject { context.vm.active_function.clone(), call_frame, context.realm().clone(), - context.vm.bindings_stack.clone(), )), queue: VecDeque::new(), }) @@ -1114,7 +1113,6 @@ impl JsObject { context.vm.active_function.clone(), call_frame, context.realm().clone(), - context.vm.bindings_stack.clone(), ), }, }) diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index fdc8e2404f5..5df7f5ce48b 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -6,7 +6,7 @@ use crate::{ builtins::async_generator::{AsyncGenerator, AsyncGeneratorState}, - environments::{BindingLocator, DeclarativeEnvironment, DeclarativeEnvironmentStack}, + environments::{DeclarativeEnvironment, DeclarativeEnvironmentStack}, vm::code_block::Readable, Context, JsError, JsObject, JsResult, JsValue, }; @@ -52,7 +52,6 @@ pub struct Vm { pub(crate) trace: bool, pub(crate) stack_size_limit: usize, pub(crate) active_function: Option, - pub(crate) bindings_stack: Vec, } impl Vm { @@ -67,7 +66,6 @@ impl Vm { trace: false, stack_size_limit: 1024, active_function: None, - bindings_stack: Vec::with_capacity(16), } } diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 90c702719dd..c3cf96f439e 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -36,8 +36,8 @@ impl Operation for GetName { /// `GetNameAndLocator` implements the Opcode Operation for `Opcode::GetNameAndLocator` /// /// Operation: -/// - Find a binding on the environment chain and push its value to the stack and its -/// `BindingLocator` to the `bindings_stack`. +/// - Find a binding on the environment chain and push its value to the stack, setting the +/// `current_binding` of the current frame. #[derive(Debug, Clone, Copy)] pub(crate) struct GetNameAndLocator; @@ -58,7 +58,7 @@ impl Operation for GetNameAndLocator { JsNativeError::reference().with_message(format!("{name} is not defined")) })?; - context.vm.bindings_stack.push(binding_locator); + context.vm.frame_mut().current_binding = binding_locator; context.vm.push(value); Ok(CompletionType::Normal) } diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index 58bd6b2a37c..4edd314073c 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -37,7 +37,7 @@ impl Operation for SetName { /// `SetNameByLocator` implements the Opcode Operation for `Opcode::SetNameByLocator` /// /// Operation: -/// - Assigns a value to the binding pointed by the top of the `bindings_stack`. +/// - Assigns a value to the binding pointed by the `current_binding` of the current frame. #[derive(Debug, Clone, Copy)] pub(crate) struct SetNameByLocator; @@ -46,11 +46,7 @@ impl Operation for SetNameByLocator { const INSTRUCTION: &'static str = "INST - SetNameByLocator"; fn execute(context: &mut Context<'_>) -> JsResult { - let binding_locator = context - .vm - .bindings_stack - .pop() - .expect("a binding should have been pushed before"); + let binding_locator = context.vm.frame().current_binding; let value = context.vm.pop(); if binding_locator.is_silent() { return Ok(CompletionType::Normal); From ba053ef54748630e9b7b0c527492781b8dffab94 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Thu, 27 Apr 2023 14:46:32 -0600 Subject: [PATCH 6/9] Fix typo --- boa_engine/src/builtins/generator/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/boa_engine/src/builtins/generator/mod.rs b/boa_engine/src/builtins/generator/mod.rs index 2c314d0e03d..bd177be9937 100644 --- a/boa_engine/src/builtins/generator/mod.rs +++ b/boa_engine/src/builtins/generator/mod.rs @@ -1,4 +1,4 @@ -//! Boa's 3tion of ECMAScript's global `Generator` object. +//! Boa's implementation of ECMAScript's global `Generator` object. //! //! A Generator is an instance of a generator function and conforms to both the Iterator and Iterable interfaces. //! From adf48c57e3aad28373e8505c0911082f67bc201f Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Mon, 1 May 2023 21:01:23 -0600 Subject: [PATCH 7/9] Reuse locators on assignment deletions --- boa_engine/src/bytecompiler/mod.rs | 19 ++++++++++++++++--- boa_engine/src/vm/code_block.rs | 1 + boa_engine/src/vm/flowgraph/mod.rs | 1 + boa_engine/src/vm/opcode/get/name.rs | 22 ++++++++++++++++++++++ boa_engine/src/vm/opcode/mod.rs | 7 +++++++ 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index f081cf80c0f..3e02ea1ac24 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -631,13 +631,26 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { { match access { Access::Variable { name } => { + let binding = self.get_binding_value(name); + let index = self.get_or_insert_binding(binding); + let lex = self.current_environment.borrow().is_lex_binding(name); + + if !lex { + self.emit(Opcode::GetLocator, &[index]); + } + expr_fn(self, 0); if use_expr { self.emit(Opcode::Dup, &[]); } - let binding = self.set_mutable_binding(name); - let index = self.get_or_insert_binding(binding); - self.emit(Opcode::SetName, &[index]); + + if lex { + let binding = self.set_mutable_binding(name); + let index = self.get_or_insert_binding(binding); + self.emit(Opcode::SetName, &[index]); + } else { + self.emit_opcode(Opcode::SetNameByLocator); + } } Access::Property { access } => match access { PropertyAccess::Simple(access) => match access.field() { diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 651cce90b6e..4e4da6e7228 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -335,6 +335,7 @@ impl CodeBlock { | Opcode::DefInitLet | Opcode::DefInitConst | Opcode::GetName + | Opcode::GetLocator | Opcode::GetNameAndLocator | Opcode::GetNameOrUndefined | Opcode::SetName diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 52e58a35568..24b17a14216 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -403,6 +403,7 @@ impl CodeBlock { | Opcode::DefInitLet | Opcode::DefInitConst | Opcode::GetName + | Opcode::GetLocator | Opcode::GetNameAndLocator | Opcode::GetNameOrUndefined | Opcode::SetName diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index c3cf96f439e..7dae765ccb7 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -33,6 +33,28 @@ impl Operation for GetName { } } +/// `GetLocator` implements the Opcode Operation for `Opcode::GetLocator` +/// +/// Operation: +/// - Find a binding on the environment and set the `current_binding` of the current frame. +#[derive(Debug, Clone, Copy)] +pub(crate) struct GetLocator; + +impl Operation for GetLocator { + const NAME: &'static str = "GetLocator"; + const INSTRUCTION: &'static str = "INST - GetLocator"; + + fn execute(context: &mut Context<'_>) -> JsResult { + let index = context.vm.read::(); + let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; + context.find_runtime_binding(&mut binding_locator)?; + + context.vm.frame_mut().current_binding = binding_locator; + + Ok(CompletionType::Normal) + } +} + /// `GetNameAndLocator` implements the Opcode Operation for `Opcode::GetNameAndLocator` /// /// Operation: diff --git a/boa_engine/src/vm/opcode/mod.rs b/boa_engine/src/vm/opcode/mod.rs index 8cc257382c3..a646ef02515 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -687,6 +687,13 @@ generate_impl! { /// Stack: **=>** value GetName, + /// Find a binding on the environment and set the `current_binding` of the current frame. + /// + /// Operands: name_index: `u32` + /// + /// Stack: **=>** + GetLocator, + /// Find a binding on the environment chain and push its value to the stack and its /// `BindingLocator` to the `bindings_stack`. /// From 1cad14ef344a0b7b07922cffe770b2eae5fd62a6 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Mon, 1 May 2023 21:28:06 -0600 Subject: [PATCH 8/9] Fix binding bug --- boa_engine/src/environments/runtime.rs | 2 +- boa_engine/src/vm/call_frame/mod.rs | 7 +++---- boa_engine/src/vm/opcode/get/name.rs | 4 ++-- boa_engine/src/vm/opcode/set/name.rs | 7 ++++++- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/boa_engine/src/environments/runtime.rs b/boa_engine/src/environments/runtime.rs index 9136c7ba4d9..ccbe4e60233 100644 --- a/boa_engine/src/environments/runtime.rs +++ b/boa_engine/src/environments/runtime.rs @@ -695,7 +695,7 @@ impl BindingLocator { } /// Creates a binding locator that indicates that the binding is on the global object. - pub(crate) const fn global(name: Identifier) -> Self { + pub(super) const fn global(name: Identifier) -> Self { Self { name, environment_index: 0, diff --git a/boa_engine/src/vm/call_frame/mod.rs b/boa_engine/src/vm/call_frame/mod.rs index ce969a479fd..40f97aa8440 100644 --- a/boa_engine/src/vm/call_frame/mod.rs +++ b/boa_engine/src/vm/call_frame/mod.rs @@ -10,7 +10,6 @@ use crate::{ vm::CodeBlock, }; use boa_gc::{Finalize, Gc, Trace}; -use boa_interner::Sym; use thin_vec::ThinVec; pub(crate) use abrupt_record::AbruptCompletionRecord; @@ -44,8 +43,8 @@ pub struct CallFrame { // Iterators and their `[[Done]]` flags that must be closed when an abrupt completion is thrown. pub(crate) iterators: ThinVec<(JsObject, bool)>, - // The current binding being updated. - pub(crate) current_binding: BindingLocator, + // The stack of bindings being updated. + pub(crate) binding_stack: Vec, } /// ---- `CallFrame` public API ---- @@ -76,7 +75,7 @@ impl CallFrame { promise_capability: None, async_generator: None, iterators: ThinVec::new(), - current_binding: BindingLocator::global(Sym::EMPTY_STRING.into()), + binding_stack: Vec::new(), } } diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 7dae765ccb7..f3b165d31d3 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -49,7 +49,7 @@ impl Operation for GetLocator { let mut binding_locator = context.vm.frame().code_block.bindings[index as usize]; context.find_runtime_binding(&mut binding_locator)?; - context.vm.frame_mut().current_binding = binding_locator; + context.vm.frame_mut().binding_stack.push(binding_locator); Ok(CompletionType::Normal) } @@ -80,7 +80,7 @@ impl Operation for GetNameAndLocator { JsNativeError::reference().with_message(format!("{name} is not defined")) })?; - context.vm.frame_mut().current_binding = binding_locator; + context.vm.frame_mut().binding_stack.push(binding_locator); context.vm.push(value); Ok(CompletionType::Normal) } diff --git a/boa_engine/src/vm/opcode/set/name.rs b/boa_engine/src/vm/opcode/set/name.rs index 4edd314073c..9e0431776cc 100644 --- a/boa_engine/src/vm/opcode/set/name.rs +++ b/boa_engine/src/vm/opcode/set/name.rs @@ -46,7 +46,12 @@ impl Operation for SetNameByLocator { const INSTRUCTION: &'static str = "INST - SetNameByLocator"; fn execute(context: &mut Context<'_>) -> JsResult { - let binding_locator = context.vm.frame().current_binding; + let binding_locator = context + .vm + .frame_mut() + .binding_stack + .pop() + .expect("locator should have been popped before"); let value = context.vm.pop(); if binding_locator.is_silent() { return Ok(CompletionType::Normal); From e671ada61fa037b47e8b0032df067f15263034e5 Mon Sep 17 00:00:00 2001 From: jedel1043 Date: Tue, 2 May 2023 02:14:52 -0600 Subject: [PATCH 9/9] Remove leftover comment --- boa_engine/src/environments/runtime.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/boa_engine/src/environments/runtime.rs b/boa_engine/src/environments/runtime.rs index ccbe4e60233..0521ff172e6 100644 --- a/boa_engine/src/environments/runtime.rs +++ b/boa_engine/src/environments/runtime.rs @@ -895,8 +895,6 @@ impl Context<'_> { /// Sets the value of a binding. /// - /// # - /// /// # Panics /// /// Panics if the environment or binding index are out of range.