Refactor environment stack to remove some panics (#3893)

* Refactor environment stack to remove some panics * Fix formatting and enum size difference * fix typo
boa-dev · Jul 10, 2024 · ab0854f · ab0854f
1 parent f80c532
commit ab0854f
Show file tree

Hide file tree

Showing 16 changed files with 257 additions and 232 deletions.
diff --git 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
@@ -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
@@ -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
@@ -7,15 +7,15 @@ use super::PoisonableEnvironment;
 #[derive(Debug, Trace, Finalize)]
 pub(crate) struct FunctionEnvironment {
     inner: PoisonableEnvironment,
-    slots: FunctionSlots,
+    slots: Box<FunctionSlots>,
 }
 
 impl FunctionEnvironment {
     /// Creates a new `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
@@ -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
@@ -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<Option<JsValue>> {
         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
@@ -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)
                     }