From ab0854f76b06e480c8681b840b717cdf05251a6d Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:11:07 +0100 Subject: [PATCH] Refactor environment stack to remove some panics (#3893) * Refactor environment stack to remove some panics * Fix formatting and enum size difference * fix typo --- core/engine/src/builtins/eval/mod.rs | 2 +- core/engine/src/builtins/function/mod.rs | 23 +- core/engine/src/environments/mod.rs | 4 +- .../runtime/declarative/function.rs | 4 +- .../runtime/declarative/global.rs | 22 +- .../environments/runtime/declarative/mod.rs | 14 +- .../runtime/declarative/module.rs | 4 +- core/engine/src/environments/runtime/mod.rs | 348 ++++++++++-------- core/engine/src/module/source.rs | 13 +- core/engine/src/module/synthetic.rs | 5 +- core/engine/src/realm.rs | 2 +- core/engine/src/vm/opcode/arguments.rs | 8 +- .../src/vm/opcode/control_flow/return.rs | 6 +- core/engine/src/vm/opcode/define/mod.rs | 4 +- core/engine/src/vm/opcode/environment/mod.rs | 6 +- core/engine/src/vm/opcode/set/name.rs | 24 +- 16 files changed, 257 insertions(+), 232 deletions(-) diff --git a/core/engine/src/builtins/eval/mod.rs b/core/engine/src/builtins/eval/mod.rs index 7c592d6cfb6..527bd8e7869 100644 --- a/core/engine/src/builtins/eval/mod.rs +++ b/core/engine/src/builtins/eval/mod.rs @@ -224,7 +224,7 @@ impl Eval { let context = &mut context.guard(move |ctx| match action { EnvStackAction::Truncate(len) => ctx.vm.environments.truncate(len), EnvStackAction::Restore(envs) => { - ctx.vm.environments.truncate(1); + ctx.vm.environments.truncate(0); ctx.vm.environments.extend(envs); } }); diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index 8970df508a2..cd1c0ffa4fc 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -17,7 +17,10 @@ use crate::{ }, bytecompiler::FunctionCompiler, context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors}, - environments::{EnvironmentStack, FunctionSlots, PrivateEnvironment, ThisBindingStatus}, + environments::{ + BindingLocatorEnvironment, EnvironmentStack, FunctionSlots, PrivateEnvironment, + ThisBindingStatus, + }, error::JsNativeError, js_string, native_function::NativeFunctionObject, @@ -1003,10 +1006,11 @@ pub(crate) fn function_call( .vm .environments .push_lexical(code.constant_compile_time_environment(last_env)); - context - .vm - .environments - .put_lexical_value(index, 0, function_object.clone().into()); + context.vm.environments.put_lexical_value( + BindingLocatorEnvironment::Stack(index), + 0, + function_object.clone().into(), + ); last_env += 1; } @@ -1095,10 +1099,11 @@ fn function_construct( .vm .environments .push_lexical(code.constant_compile_time_environment(last_env)); - context - .vm - .environments - .put_lexical_value(index, 0, this_function_object.clone().into()); + context.vm.environments.put_lexical_value( + BindingLocatorEnvironment::Stack(index), + 0, + this_function_object.clone().into(), + ); last_env += 1; } diff --git a/core/engine/src/environments/mod.rs b/core/engine/src/environments/mod.rs index 55b7259307f..fd8819a08e2 100644 --- a/core/engine/src/environments/mod.rs +++ b/core/engine/src/environments/mod.rs @@ -30,8 +30,8 @@ mod runtime; pub(crate) use { compile::CompileTimeEnvironment, runtime::{ - BindingLocator, BindingLocatorError, DeclarativeEnvironment, Environment, EnvironmentStack, - FunctionSlots, PrivateEnvironment, ThisBindingStatus, + BindingLocator, BindingLocatorEnvironment, BindingLocatorError, DeclarativeEnvironment, + Environment, EnvironmentStack, FunctionSlots, PrivateEnvironment, ThisBindingStatus, }, }; diff --git a/core/engine/src/environments/runtime/declarative/function.rs b/core/engine/src/environments/runtime/declarative/function.rs index 43276b03900..b1a7275c978 100644 --- a/core/engine/src/environments/runtime/declarative/function.rs +++ b/core/engine/src/environments/runtime/declarative/function.rs @@ -7,7 +7,7 @@ use super::PoisonableEnvironment; #[derive(Debug, Trace, Finalize)] pub(crate) struct FunctionEnvironment { inner: PoisonableEnvironment, - slots: FunctionSlots, + slots: Box, } impl FunctionEnvironment { @@ -15,7 +15,7 @@ impl FunctionEnvironment { pub(crate) fn new(bindings: u32, poisoned: bool, with: bool, slots: FunctionSlots) -> Self { Self { inner: PoisonableEnvironment::new(bindings, poisoned, with), - slots, + slots: Box::new(slots), } } diff --git a/core/engine/src/environments/runtime/declarative/global.rs b/core/engine/src/environments/runtime/declarative/global.rs index 34183a51ada..e432f516c67 100644 --- a/core/engine/src/environments/runtime/declarative/global.rs +++ b/core/engine/src/environments/runtime/declarative/global.rs @@ -1,21 +1,17 @@ -use boa_gc::{Finalize, Trace}; - -use crate::{JsObject, JsValue}; - use super::PoisonableEnvironment; +use crate::JsValue; +use boa_gc::{Finalize, Trace}; #[derive(Debug, Trace, Finalize)] pub(crate) struct GlobalEnvironment { inner: PoisonableEnvironment, - global_this: JsObject, } impl GlobalEnvironment { /// Creates a new `GlobalEnvironment`. - pub(crate) fn new(global_this: JsObject) -> Self { + pub(crate) fn new() -> Self { Self { inner: PoisonableEnvironment::new(0, false, false), - global_this, } } @@ -43,16 +39,4 @@ impl GlobalEnvironment { pub(crate) fn set(&self, index: u32, value: JsValue) { self.inner.set(index, value); } - - /// `GetThisBinding` - /// - /// Returns the `this` binding on the global environment. - /// - /// More information: - /// - [ECMAScript specification][spec] - /// - /// [spec]: https://tc39.es/ecma262/#sec-function-environment-records-getthisbinding - pub(crate) fn get_this_binding(&self) -> JsObject { - self.global_this.clone() - } } diff --git a/core/engine/src/environments/runtime/declarative/mod.rs b/core/engine/src/environments/runtime/declarative/mod.rs index 8da2a01a9ab..32cfc53469f 100644 --- a/core/engine/src/environments/runtime/declarative/mod.rs +++ b/core/engine/src/environments/runtime/declarative/mod.rs @@ -3,15 +3,14 @@ mod global; mod lexical; mod module; -use std::{cell::Cell, rc::Rc}; - -use boa_gc::{Finalize, GcRefCell, Trace}; pub(crate) use function::{FunctionEnvironment, FunctionSlots, ThisBindingStatus}; pub(crate) use global::GlobalEnvironment; pub(crate) use lexical::LexicalEnvironment; pub(crate) use module::ModuleEnvironment; -use crate::{environments::CompileTimeEnvironment, JsObject, JsResult, JsValue}; +use crate::{environments::CompileTimeEnvironment, JsResult, JsValue}; +use boa_gc::{Finalize, GcRefCell, Trace}; +use std::{cell::Cell, rc::Rc}; /// A declarative environment holds binding values at runtime. /// @@ -44,9 +43,9 @@ pub(crate) struct DeclarativeEnvironment { impl DeclarativeEnvironment { /// Creates a new global `DeclarativeEnvironment`. - pub(crate) fn global(global_this: JsObject) -> Self { + pub(crate) fn global() -> Self { Self { - kind: DeclarativeEnvironmentKind::Global(GlobalEnvironment::new(global_this)), + kind: DeclarativeEnvironmentKind::Global(GlobalEnvironment::new()), compile: Rc::new(CompileTimeEnvironment::new_global()), } } @@ -221,8 +220,7 @@ impl DeclarativeEnvironmentKind { /// [spec]: https://tc39.es/ecma262/#sec-function-environment-records-getthisbinding pub(crate) fn get_this_binding(&self) -> JsResult> { match self { - Self::Lexical(_) => Ok(None), - Self::Global(g) => Ok(Some(g.get_this_binding().into())), + Self::Lexical(_) | Self::Global(_) => Ok(None), Self::Function(f) => f.get_this_binding(), Self::Module(_) => Ok(Some(JsValue::undefined())), } diff --git a/core/engine/src/environments/runtime/declarative/module.rs b/core/engine/src/environments/runtime/declarative/module.rs index 0de1e45989c..6ffd88cb45b 100644 --- a/core/engine/src/environments/runtime/declarative/module.rs +++ b/core/engine/src/environments/runtime/declarative/module.rs @@ -67,9 +67,9 @@ impl ModuleEnvironment { .get_binding(name) .expect("linking must ensure the binding exists"); - let value = env.get(index.binding_index)?; + let value = env.get(index.binding_index())?; - *accessor.borrow_mut() = BindingAccessor::Index(index.binding_index); + *accessor.borrow_mut() = BindingAccessor::Index(index.binding_index()); Some(value) } diff --git a/core/engine/src/environments/runtime/mod.rs b/core/engine/src/environments/runtime/mod.rs index ed67f32f7a6..8e13398cfc1 100644 --- a/core/engine/src/environments/runtime/mod.rs +++ b/core/engine/src/environments/runtime/mod.rs @@ -2,7 +2,7 @@ use std::rc::Rc; use crate::{ environments::CompileTimeEnvironment, - object::{internal_methods::InternalMethodContext, JsObject, PrivateName}, + object::{JsObject, PrivateName}, Context, JsResult, JsString, JsSymbol, JsValue, }; use boa_gc::{empty_trace, Finalize, Gc, Trace}; @@ -26,7 +26,7 @@ pub(crate) use self::{ #[derive(Clone, Debug, Trace, Finalize)] pub(crate) struct EnvironmentStack { stack: Vec, - + global: Gc, private_stack: Vec>, } @@ -45,13 +45,6 @@ impl Environment { Self::Object(_) => None, } } - - /// Returns the declarative environment and panic if it is not one. - #[track_caller] - pub(crate) fn declarative_expect(&self) -> &Gc { - self.as_declarative() - .expect("environment must be declarative") - } } impl EnvironmentStack { @@ -62,7 +55,8 @@ impl EnvironmentStack { DeclarativeEnvironmentKind::Global(_) )); Self { - stack: vec![Environment::Declarative(global)], + stack: Vec::new(), + global, private_stack: Vec::new(), } } @@ -73,19 +67,12 @@ impl EnvironmentStack { global.kind(), DeclarativeEnvironmentKind::Global(_) )); - self.stack[0] = Environment::Declarative(global); + self.global = global; } /// Gets the current global environment. pub(crate) fn global(&self) -> &Gc { - let env = &self.stack[0]; - - match env { - Environment::Declarative(ref env) => env, - Environment::Object(_) => { - unreachable!("first environment should be the global environment") - } - } + &self.global } /// Gets the next outer function environment. @@ -105,7 +92,9 @@ impl EnvironmentStack { /// Pop all current environments except the global environment. pub(crate) fn pop_to_global(&mut self) -> Vec { - self.stack.split_off(1) + let mut envs = Vec::new(); + std::mem::swap(&mut envs, &mut self.stack); + envs } /// Get the number of current environments. @@ -131,10 +120,6 @@ impl EnvironmentStack { /// - [ECMAScript specification][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-getthisenvironment - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. pub(crate) fn get_this_environment(&self) -> &DeclarativeEnvironmentKind { for env in self.stack.iter().rev() { if let Some(decl) = env.as_declarative().filter(|decl| decl.has_this_binding()) { @@ -142,59 +127,53 @@ impl EnvironmentStack { } } - panic!("global environment must exist"); + self.global().kind() } /// `GetThisBinding` /// /// Returns the current `this` binding of the environment. + /// Note: If the current environment is the global environment, this function returns `Ok(None)`. /// /// More information: /// - [ECMAScript specification][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-function-environment-records-getthisbinding - pub(crate) fn get_this_binding(&self) -> JsResult { + pub(crate) fn get_this_binding(&self) -> JsResult> { for env in self.stack.iter().rev() { if let Environment::Declarative(decl) = env { if let Some(this) = decl.get_this_binding()? { - return Ok(this); + return Ok(Some(this)); } } } - panic!("global environment must exist"); + Ok(None) } - /// Push a new object environment on the environments stack and return it's index. - pub(crate) fn push_object(&mut self, object: JsObject) -> usize { - let index = self.stack.len(); + /// Push a new object environment on the environments stack. + pub(crate) fn push_object(&mut self, object: JsObject) { self.stack.push(Environment::Object(object)); - index } /// Push a lexical environment on the environments stack and return it's index. - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. - #[track_caller] pub(crate) fn push_lexical(&mut self, compile_environment: Rc) -> u32 { let num_bindings = compile_environment.num_bindings(); let (poisoned, with) = { - let with = self - .stack - .last() - .expect("global environment must always exist") - .as_declarative() - .is_none(); + // Check if the outer environment is a declarative environment. + let with = if let Some(env) = self.stack.last() { + env.as_declarative().is_none() + } else { + false + }; let environment = self .stack .iter() .rev() .find_map(Environment::as_declarative) - .expect("global environment must always exist"); + .unwrap_or(self.global()); (environment.poisoned(), with || environment.with()) }; @@ -215,11 +194,6 @@ impl EnvironmentStack { } /// Push a function environment on the environments stack. - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. - #[track_caller] pub(crate) fn push_function( &mut self, compile_environment: Rc, @@ -228,19 +202,19 @@ impl EnvironmentStack { let num_bindings = compile_environment.num_bindings(); let (poisoned, with) = { - let with = self - .stack - .last() - .expect("global environment must always exist") - .as_declarative() - .is_none(); + // Check if the outer environment is a declarative environment. + let with = if let Some(env) = self.stack.last() { + env.as_declarative().is_none() + } else { + false + }; let environment = self .stack .iter() .rev() .find_map(Environment::as_declarative) - .expect("global environment must always exist"); + .unwrap_or(self.global()); (environment.poisoned(), with || environment.with()) }; @@ -258,11 +232,6 @@ impl EnvironmentStack { } /// Push a module environment on the environments stack. - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. - #[track_caller] pub(crate) fn push_module(&mut self, compile_environment: Rc) { let num_bindings = compile_environment.num_bindings(); self.stack.push(Environment::Declarative(Gc::new( @@ -275,37 +244,28 @@ impl EnvironmentStack { /// Pop environment from the environments stack. #[track_caller] - pub(crate) fn pop(&mut self) -> Environment { - debug_assert!(self.stack.len() > 1); - self.stack - .pop() - .expect("environment stack is cannot be empty") + pub(crate) fn pop(&mut self) { + debug_assert!(!self.stack.is_empty()); + self.stack.pop(); } /// Get the most outer environment. - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. - #[track_caller] - pub(crate) fn current_ref(&self) -> &Environment { - self.stack - .last() - .expect("global environment must always exist") + pub(crate) fn current_declarative_ref(&self) -> Option<&Gc> { + if let Some(env) = self.stack.last() { + env.as_declarative() + } else { + Some(self.global()) + } } /// Get the compile environment for the current runtime environment. - /// - /// # Panics - /// - /// Panics if no environment exists on the stack. pub(crate) fn current_compile_environment(&self) -> Rc { self.stack .iter() .filter_map(Environment::as_declarative) .last() - .expect("global environment must always exist") - .compile_env() + .map(|env| env.compile_env()) + .unwrap_or(self.global().compile_env()) } /// Mark that there may be added bindings from the current environment to the next function @@ -322,6 +282,7 @@ impl EnvironmentStack { return; } } + self.global().poison(); } /// Set the value of a lexical binding. @@ -332,15 +293,19 @@ impl EnvironmentStack { #[track_caller] pub(crate) fn put_lexical_value( &mut self, - environment_index: u32, + environment: BindingLocatorEnvironment, binding_index: u32, value: JsValue, ) { - let env = self - .stack - .get(environment_index as usize) - .expect("environment index must be in range") - .declarative_expect(); + let env = match environment { + BindingLocatorEnvironment::GlobalObject + | BindingLocatorEnvironment::GlobalDeclarative => self.global(), + BindingLocatorEnvironment::Stack(index) => self + .stack + .get(index as usize) + .and_then(Environment::as_declarative) + .expect("must be declarative environment"), + }; env.set(binding_index, value); } @@ -352,15 +317,19 @@ impl EnvironmentStack { #[track_caller] pub(crate) fn put_value_if_uninitialized( &mut self, - environment_index: u32, + environment: BindingLocatorEnvironment, binding_index: u32, value: JsValue, ) { - let env = self - .stack - .get(environment_index as usize) - .expect("environment index must be in range") - .declarative_expect(); + let env = match environment { + BindingLocatorEnvironment::GlobalObject + | BindingLocatorEnvironment::GlobalDeclarative => self.global(), + BindingLocatorEnvironment::Stack(index) => self + .stack + .get(index as usize) + .and_then(Environment::as_declarative) + .expect("must be declarative environment"), + }; if env.get(binding_index).is_none() { env.set(binding_index, value); } @@ -424,10 +393,17 @@ impl EnvironmentStack { /// Binding locators get created at bytecode compile time and are accessible at runtime via the [`crate::vm::CodeBlock`]. #[derive(Clone, Debug, Eq, Hash, PartialEq, Finalize)] pub(crate) struct BindingLocator { + /// Name of the binding. name: JsString, - environment_index: u32, + + /// Environment of the binding. + /// - 0: Global object + /// - 1: Global declarative environment + /// - n: Stack environment at index n - 2 + environment: u32, + + /// Index of the binding in the environment. binding_index: u32, - global: bool, } unsafe impl Trace for BindingLocator { @@ -443,9 +419,8 @@ impl BindingLocator { ) -> Self { Self { name, - environment_index, + environment: environment_index + 1, binding_index, - global: false, } } @@ -453,9 +428,8 @@ impl BindingLocator { pub(super) const fn global(name: JsString) -> Self { Self { name, - environment_index: 0, + environment: 0, binding_index: 0, - global: true, } } @@ -466,12 +440,25 @@ impl BindingLocator { /// Returns if the binding is located on the global object. pub(crate) const fn is_global(&self) -> bool { - self.global + self.environment == 0 + } + + /// Returns the environment of the binding. + pub(crate) fn environment(&self) -> BindingLocatorEnvironment { + match self.environment { + 0 => BindingLocatorEnvironment::GlobalObject, + 1 => BindingLocatorEnvironment::GlobalDeclarative, + n => BindingLocatorEnvironment::Stack(n - 2), + } } - /// Returns the environment index of the binding. - pub(crate) const fn environment_index(&self) -> u32 { - self.environment_index + /// Sets the environment of the binding. + fn set_environment(&mut self, environment: BindingLocatorEnvironment) { + self.environment = match environment { + BindingLocatorEnvironment::GlobalObject => 0, + BindingLocatorEnvironment::GlobalDeclarative => 1, + BindingLocatorEnvironment::Stack(index) => index + 2, + }; } /// Returns the binding index of the binding. @@ -490,6 +477,14 @@ pub(crate) enum BindingLocatorError { Silent, } +/// The environment in which a binding is located. +#[derive(Clone, Copy, Debug)] +pub(crate) enum BindingLocatorEnvironment { + GlobalObject, + GlobalDeclarative, + Stack(u32), +} + impl Context { /// Gets the corresponding runtime binding of the provided `BindingLocator`, modifying /// its indexes in place. @@ -501,29 +496,33 @@ impl Context { /// are completely removed of runtime checks because the specification guarantees that runtime /// semantics cannot add or remove lexical bindings. pub(crate) fn find_runtime_binding(&mut self, locator: &mut BindingLocator) -> JsResult<()> { - let current = self.vm.environments.current_ref(); - if let Some(env) = current.as_declarative() { + if let Some(env) = self.vm.environments.current_declarative_ref() { if !env.with() && !env.poisoned() { return Ok(()); } } - for env_index in (locator.environment_index..self.vm.environments.stack.len() as u32).rev() - { - match self.environment_expect(env_index) { + let (global, min_index) = match locator.environment() { + BindingLocatorEnvironment::GlobalObject + | BindingLocatorEnvironment::GlobalDeclarative => (true, 0), + BindingLocatorEnvironment::Stack(index) => (false, index), + }; + let max_index = self.vm.environments.stack.len() as u32; + + for index in (min_index..max_index).rev() { + match self.environment_expect(index) { Environment::Declarative(env) => { if env.poisoned() { let compile = env.compile_env(); if compile.is_function() { if let Some(b) = compile.get_binding(locator.name()) { - locator.environment_index = b.environment_index; - locator.binding_index = b.binding_index; - locator.global = false; - break; + locator.set_environment(b.environment()); + locator.binding_index = b.binding_index(); + return Ok(()); } } } else if !env.with() { - break; + return Ok(()); } } Environment::Object(o) => { @@ -536,14 +535,24 @@ impl Context { continue; } } - locator.environment_index = env_index; - locator.global = false; - break; + locator.set_environment(BindingLocatorEnvironment::Stack(index)); + return Ok(()); } } } } + if global { + let env = self.vm.environments.global(); + if env.poisoned() { + let compile = env.compile_env(); + if let Some(b) = compile.get_binding(locator.name()) { + locator.set_environment(b.environment()); + locator.binding_index = b.binding_index(); + } + } + } + Ok(()) } @@ -552,16 +561,21 @@ impl Context { &mut self, locator: &BindingLocator, ) -> JsResult> { - let current = self.vm.environments.current_ref(); - if let Some(env) = current.as_declarative() { + if let Some(env) = self.vm.environments.current_declarative_ref() { if !env.with() { return Ok(None); } } - for env_index in (locator.environment_index..self.vm.environments.stack.len() as u32).rev() - { - match self.environment_expect(env_index) { + let min_index = match locator.environment() { + BindingLocatorEnvironment::GlobalObject + | BindingLocatorEnvironment::GlobalDeclarative => 0, + BindingLocatorEnvironment::Stack(index) => index, + }; + let max_index = self.vm.environments.stack.len() as u32; + + for index in (min_index..max_index).rev() { + match self.environment_expect(index) { Environment::Declarative(env) => { if env.poisoned() { let compile = env.compile_env(); @@ -597,17 +611,24 @@ impl Context { /// /// Panics if the environment or binding index are out of range. pub(crate) fn is_initialized_binding(&mut self, locator: &BindingLocator) -> JsResult { - if locator.global { - let key = locator.name().clone(); - self.global_object().has_property(key, self) - } else { - match self.environment_expect(locator.environment_index) { - Environment::Declarative(env) => Ok(env.get(locator.binding_index).is_some()), + match locator.environment() { + BindingLocatorEnvironment::GlobalObject => { + let key = locator.name().clone(); + let obj = self.global_object(); + obj.has_property(key, self) + } + BindingLocatorEnvironment::GlobalDeclarative => { + let env = self.vm.environments.global(); + Ok(env.get(locator.binding_index()).is_some()) + } + BindingLocatorEnvironment::Stack(index) => match self.environment_expect(index) { + Environment::Declarative(env) => Ok(env.get(locator.binding_index()).is_some()), Environment::Object(obj) => { let key = locator.name().clone(); - obj.clone().has_property(key, self) + let obj = obj.clone(); + obj.has_property(key, self) } - } + }, } } @@ -617,19 +638,24 @@ impl Context { /// /// Panics if the environment or binding index are out of range. pub(crate) fn get_binding(&mut self, locator: &BindingLocator) -> JsResult> { - if locator.global { - let global = self.global_object(); - let key = locator.name().clone(); - global.try_get(key, self) - } else { - match self.environment_expect(locator.environment_index) { - Environment::Declarative(env) => Ok(env.get(locator.binding_index)), + match locator.environment() { + BindingLocatorEnvironment::GlobalObject => { + let key = locator.name().clone(); + let obj = self.global_object(); + obj.try_get(key, self) + } + BindingLocatorEnvironment::GlobalDeclarative => { + let env = self.vm.environments.global(); + Ok(env.get(locator.binding_index())) + } + BindingLocatorEnvironment::Stack(index) => match self.environment_expect(index) { + Environment::Declarative(env) => Ok(env.get(locator.binding_index())), Environment::Object(obj) => { - let obj = obj.clone(); let key = locator.name().clone(); + let obj = obj.clone(); obj.get(key, self).map(Some) } - } + }, } } @@ -645,24 +671,27 @@ impl Context { value: JsValue, strict: bool, ) -> JsResult<()> { - if locator.global { - let key = locator.name().clone(); - - self.global_object().set(key, value, strict, self)?; - } else { - match self.environment_expect(locator.environment_index) { + match locator.environment() { + BindingLocatorEnvironment::GlobalObject => { + let key = locator.name().clone(); + let obj = self.global_object(); + obj.set(key, value, strict, self)?; + } + BindingLocatorEnvironment::GlobalDeclarative => { + let env = self.vm.environments.global(); + env.set(locator.binding_index(), value); + } + BindingLocatorEnvironment::Stack(index) => match self.environment_expect(index) { Environment::Declarative(decl) => { - decl.set(locator.binding_index, value); + decl.set(locator.binding_index(), value); } Environment::Object(obj) => { - let obj = obj.clone(); let key = locator.name().clone(); - + let obj = obj.clone(); obj.set(key, value, strict, self)?; } - } + }, } - Ok(()) } @@ -674,20 +703,21 @@ impl Context { /// /// Panics if the environment or binding index are out of range. pub(crate) fn delete_binding(&mut self, locator: &BindingLocator) -> JsResult { - if locator.is_global() { - let key = locator.name().clone(); - self.global_object() - .__delete__(&key.into(), &mut self.into()) - } else { - match self.environment_expect(locator.environment_index) { + match locator.environment() { + BindingLocatorEnvironment::GlobalObject => { + let key = locator.name().clone(); + let obj = self.global_object(); + obj.__delete__(&key.into(), &mut self.into()) + } + BindingLocatorEnvironment::GlobalDeclarative => Ok(false), + BindingLocatorEnvironment::Stack(index) => match self.environment_expect(index) { Environment::Declarative(_) => Ok(false), Environment::Object(obj) => { - let obj = obj.clone(); let key = locator.name().clone(); - - obj.__delete__(&key.into(), &mut InternalMethodContext::new(self)) + let obj = obj.clone(); + obj.__delete__(&key.into(), &mut self.into()) } - } + }, } } diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index ceadec0c037..0231686b8d1 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -1653,7 +1653,7 @@ impl SourceTextModule { // i. Let namespace be GetModuleNamespace(importedModule). let namespace = module.namespace(context); context.vm.environments.put_lexical_value( - locator.environment_index(), + locator.environment(), locator.binding_index(), namespace.into(), ); @@ -1665,8 +1665,8 @@ impl SourceTextModule { BindingName::Name(name) => context .vm .environments - .current_ref() - .declarative_expect() + .current_declarative_ref() + .expect("must be declarative") .kind() .as_module() .expect("last environment should be the module env") @@ -1674,7 +1674,7 @@ impl SourceTextModule { BindingName::Namespace => { let namespace = export_locator.module.namespace(context); context.vm.environments.put_lexical_value( - locator.environment_index(), + locator.environment(), locator.binding_index(), namespace.into(), ); @@ -1690,7 +1690,7 @@ impl SourceTextModule { let function = create_function_object_fast(code, context); context.vm.environments.put_lexical_value( - locator.environment_index(), + locator.environment(), locator.binding_index(), function.into(), ); @@ -1704,8 +1704,7 @@ impl SourceTextModule { let env = frame .environments - .current_ref() - .as_declarative() + .current_declarative_ref() .cloned() .expect("frame must have a declarative environment"); diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index 330b57995c5..20f60a8ff77 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -308,15 +308,14 @@ impl SyntheticModule { for locator in exports { // b. Perform ! env.InitializeBinding(exportName, undefined). envs.put_lexical_value( - locator.environment_index(), + locator.environment(), locator.binding_index(), JsValue::undefined(), ); } let env = envs - .current_ref() - .as_declarative() + .current_declarative_ref() .cloned() .expect("should have the module environment"); diff --git a/core/engine/src/realm.rs b/core/engine/src/realm.rs index bbf3dce18b9..255d7130244 100644 --- a/core/engine/src/realm.rs +++ b/core/engine/src/realm.rs @@ -78,7 +78,7 @@ impl Realm { let global_this = hooks .create_global_this(&intrinsics) .unwrap_or_else(|| global_object.clone()); - let environment = Gc::new(DeclarativeEnvironment::global(global_this.clone())); + let environment = Gc::new(DeclarativeEnvironment::global()); let realm = Self { inner: Gc::new(Inner { diff --git a/core/engine/src/vm/opcode/arguments.rs b/core/engine/src/vm/opcode/arguments.rs index fb4fe7fb625..957d30b1dc7 100644 --- a/core/engine/src/vm/opcode/arguments.rs +++ b/core/engine/src/vm/opcode/arguments.rs @@ -27,12 +27,16 @@ impl Operation for CreateMappedArgumentsObject { let code = frame.code_block().clone(); let args = frame.arguments(&context.vm).to_vec(); - let env = context.vm.environments.current_ref(); + let env = context + .vm + .environments + .current_declarative_ref() + .expect("must be declarative"); let arguments = MappedArguments::new( &function_object, &code.mapped_arguments_binding_indices, &args, - env.declarative_expect(), + env, context, ); context.vm.push(arguments); diff --git a/core/engine/src/vm/opcode/control_flow/return.rs b/core/engine/src/vm/opcode/control_flow/return.rs index 93e502b8321..6b3ffa4b94b 100644 --- a/core/engine/src/vm/opcode/control_flow/return.rs +++ b/core/engine/src/vm/opcode/control_flow/return.rs @@ -59,15 +59,15 @@ impl Operation for CheckReturn { this } else { let realm = frame.realm.clone(); - let this = context.vm.environments.get_this_binding(); - match this { + match context.vm.environments.get_this_binding() { Err(err) => { let err = err.inject_realm(realm); context.vm.pending_exception = Some(err); return Ok(CompletionType::Throw); } - Ok(this) => this, + Ok(Some(this)) => this, + Ok(None) => context.realm().global_this().clone().into(), } } }; diff --git a/core/engine/src/vm/opcode/define/mod.rs b/core/engine/src/vm/opcode/define/mod.rs index 4af7365af40..71f5a20f205 100644 --- a/core/engine/src/vm/opcode/define/mod.rs +++ b/core/engine/src/vm/opcode/define/mod.rs @@ -23,7 +23,7 @@ impl DefVar { let binding_locator = context.vm.frame().code_block.bindings[index].clone(); context.vm.environments.put_value_if_uninitialized( - binding_locator.environment_index(), + binding_locator.environment(), binding_locator.binding_index(), JsValue::undefined(), ); @@ -106,7 +106,7 @@ impl PutLexicalValue { let value = context.vm.pop(); let binding_locator = context.vm.frame().code_block.bindings[index].clone(); context.vm.environments.put_lexical_value( - binding_locator.environment_index(), + binding_locator.environment(), binding_locator.binding_index(), value, ); diff --git a/core/engine/src/vm/opcode/environment/mod.rs b/core/engine/src/vm/opcode/environment/mod.rs index 31f61ce8f1b..659f1400ac7 100644 --- a/core/engine/src/vm/opcode/environment/mod.rs +++ b/core/engine/src/vm/opcode/environment/mod.rs @@ -27,7 +27,11 @@ impl Operation for This { return Ok(CompletionType::Normal); } - let this = context.vm.environments.get_this_binding()?; + let this = context + .vm + .environments + .get_this_binding()? + .unwrap_or(context.realm().global_this().clone().into()); context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED; context.vm.stack[this_index as usize] = this.clone(); context.vm.push(this); diff --git a/core/engine/src/vm/opcode/set/name.rs b/core/engine/src/vm/opcode/set/name.rs index 421412bbc68..244c25e6c64 100644 --- a/core/engine/src/vm/opcode/set/name.rs +++ b/core/engine/src/vm/opcode/set/name.rs @@ -1,5 +1,5 @@ use crate::{ - environments::{BindingLocator, Environment}, + environments::{BindingLocator, BindingLocatorEnvironment, Environment}, vm::{opcode::Operation, CompletionType}, Context, JsNativeError, JsResult, }; @@ -125,15 +125,17 @@ fn verify_initialized(locator: &BindingLocator, context: &mut Context) -> JsResu let key = locator.name(); let strict = context.vm.frame().code_block.strict(); - let message = if locator.is_global() { - strict.then(|| { - format!( - "cannot assign to uninitialized global property `{}`", - key.to_std_string_escaped() - ) - }) - } else { - match context.environment_expect(locator.environment_index()) { + let message = match locator.environment() { + BindingLocatorEnvironment::GlobalObject if strict => Some(format!( + "cannot assign to uninitialized global property `{}`", + key.to_std_string_escaped() + )), + BindingLocatorEnvironment::GlobalObject => None, + BindingLocatorEnvironment::GlobalDeclarative => Some(format!( + "cannot assign to uninitialized binding `{}`", + key.to_std_string_escaped() + )), + BindingLocatorEnvironment::Stack(index) => match context.environment_expect(index) { Environment::Declarative(_) => Some(format!( "cannot assign to uninitialized binding `{}`", key.to_std_string_escaped() @@ -143,7 +145,7 @@ fn verify_initialized(locator: &BindingLocator, context: &mut Context) -> JsResu key.to_std_string_escaped() )), Environment::Object(_) => None, - } + }, }; if let Some(message) = message {