diff --git a/boa_engine/src/builtins/json/mod.rs b/boa_engine/src/builtins/json/mod.rs index 8c07c8e99d..d079d5e7ed 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 78d2042ab6..c9efa579cf 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 ac9a5fbaec..a85e732bb9 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::SetNameByLocator); + } } 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 b7becf9a11..79d7c38d10 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::SetNameByLocator); + } } 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 85ec537ead..5473d48f56 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 8aa91c205e..3e02ea1ac2 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() { @@ -736,6 +749,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 +784,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 +1363,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 cd74d73bca..46a63c8c8a 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 31babac24d..82cc559ca1 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 10b3ac32ee..f10e751428 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 e3127013f6..e551c6317f 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 13ecdf2f88..b95bcfc43a 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 ac7dc8452b..0521ff172e 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(super) 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) + } } } } @@ -953,7 +963,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/call_frame/mod.rs b/boa_engine/src/vm/call_frame/mod.rs index f8dc16d9fd..40f97aa844 100644 --- a/boa_engine/src/vm/call_frame/mod.rs +++ b/boa_engine/src/vm/call_frame/mod.rs @@ -5,7 +5,10 @@ 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 thin_vec::ThinVec; @@ -39,6 +42,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 stack of bindings being updated. + pub(crate) binding_stack: Vec, } /// ---- `CallFrame` public API ---- @@ -69,6 +75,7 @@ impl CallFrame { promise_capability: None, async_generator: None, iterators: ThinVec::new(), + binding_stack: Vec::new(), } } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 2d44baf459..4e4da6e722 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -335,6 +335,8 @@ impl CodeBlock { | Opcode::DefInitLet | Opcode::DefInitConst | Opcode::GetName + | Opcode::GetLocator + | Opcode::GetNameAndLocator | Opcode::GetNameOrUndefined | Opcode::SetName | Opcode::DeleteName => { @@ -495,6 +497,7 @@ impl CodeBlock { | Opcode::SetPrototype | Opcode::PushObjectEnvironment | Opcode::IsObject + | 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 80c9b1dbec..24b17a1421 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -403,6 +403,8 @@ impl CodeBlock { | Opcode::DefInitLet | Opcode::DefInitConst | Opcode::GetName + | Opcode::GetLocator + | Opcode::GetNameAndLocator | Opcode::GetNameOrUndefined | Opcode::SetName | Opcode::DeleteName => { @@ -566,6 +568,7 @@ impl CodeBlock { | Opcode::SuperCallPrepare | Opcode::SetPrototype | Opcode::IsObject + | Opcode::SetNameByLocator | 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 baf40f4135..5df7f5ce48 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -40,6 +40,7 @@ pub(crate) use { #[cfg(test)] mod tests; + /// Virtual Machine. #[derive(Debug)] pub struct Vm { diff --git a/boa_engine/src/vm/opcode/get/name.rs b/boa_engine/src/vm/opcode/get/name.rs index 704731c262..f3b165d31d 100644 --- a/boa_engine/src/vm/opcode/get/name.rs +++ b/boa_engine/src/vm/opcode/get/name.rs @@ -33,6 +33,59 @@ 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().binding_stack.push(binding_locator); + + Ok(CompletionType::Normal) + } +} + +/// `GetNameAndLocator` implements the Opcode Operation for `Opcode::GetNameAndLocator` +/// +/// Operation: +/// - 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; + +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.frame_mut().binding_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 fa24b99431..a646ef0251 100644 --- a/boa_engine/src/vm/opcode/mod.rs +++ b/boa_engine/src/vm/opcode/mod.rs @@ -687,6 +687,21 @@ 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`. + /// + /// 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 +716,11 @@ generate_impl! { /// Stack: value **=>** SetName, + /// Assigns a value to the binding pointed by the top of the `bindings_stack`. + /// + /// Stack: value **=>** + SetNameByLocator, + /// 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 1cd843f990..9e0431776c 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,70 @@ 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) + } +} + +/// `SetNameByLocator` implements the Opcode Operation for `Opcode::SetNameByLocator` +/// +/// Operation: +/// - Assigns a value to the binding pointed by the `current_binding` of the current frame. +#[derive(Debug, Clone, Copy)] +pub(crate) struct SetNameByLocator; + +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 + .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); } + 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(()) +}