fix eval delete bindings

core/engine/src/builtins/eval/mod.rs

@@ -316,6 +316,24 @@ impl Eval {
             )
             .map_err(|e| JsNativeError::syntax().with_message(e))?;
 
+        let deletable_binding_indices =
+            bindings
+                .new_annex_b_function_names
+                .iter()
+                .map(|binding| binding.locator().binding_index())
+                .chain(bindings.new_function_names.values().filter_map(
+                    |(binding, binding_exists)| {
+                        (!*binding_exists).then_some(binding.locator().binding_index())
+                    },
+                ))
+                .chain(
+                    bindings
+                        .new_var_names
+                        .iter()
+                        .map(|binding| binding.locator().binding_index()),
+                )
+                .collect::<Vec<_>>();
+
         compiler.eval_declaration_instantiation(&body, strict, &variable_scope, bindings);
 
         compiler.compile_statement_list(body.statements(), true, false);
@@ -326,6 +344,7 @@ impl Eval {
         // function environment before evaluating.
         if !strict {
             var_environment.extend_from_compile();
+            var_environment.mark_deletable_bindings(deletable_binding_indices);
         }
 
         let env_fp = context.vm.frame().environments.len() as u32;

core/engine/src/environments/runtime/declarative/function.rs

@@ -1,8 +1,16 @@
 use boa_ast::scope::Scope;
 use boa_gc::{Finalize, GcRefCell, Trace, custom_trace};
+use rustc_hash::FxHashSet;
+use std::cell::RefCell;
 
 use crate::{JsNativeError, JsObject, JsResult, JsValue, builtins::function::OrdinaryFunction};
 
+#[derive(Debug, Default)]
+struct BindingDeletionState {
+    deletable_bindings: FxHashSet<u32>,
+    deleted_bindings: FxHashSet<u32>,
+}
+
 #[derive(Debug, Trace, Finalize)]
 pub(crate) struct FunctionEnvironment {
     bindings: GcRefCell<Vec<Option<JsValue>>>,
@@ -11,6 +19,9 @@ pub(crate) struct FunctionEnvironment {
     // Safety: Nothing in `Scope` needs tracing.
     #[unsafe_ignore_trace]
     scope: Scope,
+
+    #[unsafe_ignore_trace]
+    binding_deletion_state: Box<RefCell<BindingDeletionState>>,
 }
 
 impl FunctionEnvironment {
@@ -20,6 +31,7 @@ impl FunctionEnvironment {
             bindings: GcRefCell::new(vec![None; bindings_count as usize]),
             slots: Box::new(slots),
             scope,
+            binding_deletion_state: Box::default(),
         }
     }
 
@@ -58,6 +70,41 @@ impl FunctionEnvironment {
         &self.bindings
     }
 
+    pub(crate) fn mark_deletable_bindings<I>(&self, bindings: I)
+    where
+        I: IntoIterator<Item = u32>,
+    {
+        self.binding_deletion_state
+            .borrow_mut()
+            .deletable_bindings
+            .extend(bindings);
+    }
+
+    pub(crate) fn is_deleted_binding(&self, index: u32) -> bool {
+        self.binding_deletion_state
+            .borrow()
+            .deleted_bindings
+            .contains(&index)
+    }
+
+    #[track_caller]
+    pub(crate) fn delete_binding(&self, index: u32) -> bool {
+        if !self
+            .binding_deletion_state
+            .borrow()
+            .deletable_bindings
+            .contains(&index)
+        {
+            return false;
+        }
+        self.bindings.borrow_mut()[index as usize] = None;
+        self.binding_deletion_state
+            .borrow_mut()
+            .deleted_bindings
+            .insert(index);
+        true
+    }
+
     /// `BindThisValue`
     ///
     /// Sets the given value as the `this` binding of the environment.

core/engine/src/environments/runtime/declarative/mod.rs

@@ -90,6 +90,16 @@ impl DeclarativeEnvironment {
         self.kind.set(index, value);
     }
 
+    #[track_caller]
+    pub(crate) fn delete_binding(&self, index: u32) -> bool {
+        self.kind.delete_binding(index)
+    }
+
+    #[track_caller]
+    pub(crate) fn is_deleted_binding(&self, index: u32) -> bool {
+        self.kind.is_deleted_binding(index)
+    }
+
     /// `GetThisBinding`
     ///
     /// Returns the `this` binding of this environment.
@@ -139,6 +149,15 @@ impl DeclarativeEnvironment {
             }
         }
     }
+
+    pub(crate) fn mark_deletable_bindings<I>(&self, bindings: I)
+    where
+        I: IntoIterator<Item = u32>,
+    {
+        if let Some(env) = self.kind().as_function() {
+            env.mark_deletable_bindings(bindings);
+        }
+    }
 }
 
 /// The kind of the declarative environment.
@@ -212,6 +231,22 @@ impl DeclarativeEnvironmentKind {
         }
     }
 
+    #[track_caller]
+    pub(crate) fn delete_binding(&self, index: u32) -> bool {
+        match self {
+            Self::Function(inner) => inner.delete_binding(index),
+            Self::Lexical(_) | Self::Global(_) | Self::Module(_) => false,
+        }
+    }
+
+    #[track_caller]
+    pub(crate) fn is_deleted_binding(&self, index: u32) -> bool {
+        match self {
+            Self::Function(inner) => inner.is_deleted_binding(index),
+            Self::Lexical(_) | Self::Global(_) | Self::Module(_) => false,
+        }
+    }
+
     /// `GetThisBinding`
     ///
     /// Returns the `this` binding of this environment.

core/engine/src/environments/runtime/mod.rs

@@ -460,27 +460,45 @@ 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 deleted_binding = match locator.scope() {
+            BindingLocatorScope::Stack(index) => matches!(
+                self.environment_expect(index),
+                Environment::Declarative(env) if env.is_deleted_binding(locator.binding_index())
+            ),
+            BindingLocatorScope::GlobalObject | BindingLocatorScope::GlobalDeclarative => false,
+        };
+
         let global = self.vm.frame().realm.environment();
         if let Some(env) = self.vm.frame().environments.current_declarative_ref(global)
             && !env.with()
             && !env.poisoned()
+            && !deleted_binding
         {
             return Ok(());
         }
 
         let (global, min_index) = match locator.scope() {
             BindingLocatorScope::GlobalObject | BindingLocatorScope::GlobalDeclarative => (true, 0),
+            BindingLocatorScope::Stack(_) if deleted_binding => (false, 0),
             BindingLocatorScope::Stack(index) => (false, index),
         };
         let max_index = self.vm.frame().environments.len() as u32;
 
         for index in (min_index..max_index).rev() {
             match self.environment_expect(index) {
                 Environment::Declarative(env) => {
-                    if env.poisoned() {
+                    if env.poisoned() || deleted_binding {
                         if let Some(env) = env.kind().as_function()
                             && let Some(b) = env.compile().get_binding(locator.name())
                         {
+                            if deleted_binding
+                                && self
+                                    .environment_expect(index)
+                                    .as_declarative()
+                                    .is_some_and(|env| env.is_deleted_binding(b.binding_index()))
+                            {
+                                continue;
+                            }
                             locator.set_scope(b.scope());
                             locator.set_binding_index(b.binding_index());
                             return Ok(());
@@ -505,6 +523,26 @@ impl Context {
             }
         }
 
+        if deleted_binding
+            && let BindingLocatorScope::Stack(index) = locator.scope()
+            && let Environment::Declarative(env) = self.environment_expect(index)
+            && let Some(function_env) = env.kind().as_function()
+        {
+            let mut scope = function_env.compile().outer();
+            while let Some(current_scope) = scope {
+                if let Some(binding) = current_scope.get_binding(locator.name()) {
+                    locator.set_scope(binding.scope());
+                    locator.set_binding_index(binding.binding_index());
+                    return Ok(());
+                }
+                scope = current_scope.outer();
+            }
+
+            locator.set_scope(BindingLocatorScope::GlobalObject);
+            locator.set_binding_index(0);
+            return Ok(());
+        }
+
         if global
             && self.realm().environment().poisoned()
             && let Some(b) = self.realm().scope().get_binding(locator.name())
@@ -672,7 +710,7 @@ impl Context {
             }
             BindingLocatorScope::GlobalDeclarative => Ok(false),
             BindingLocatorScope::Stack(index) => match self.environment_expect(index) {
-                Environment::Declarative(_) => Ok(false),
+                Environment::Declarative(env) => Ok(env.delete_binding(locator.binding_index())),
                 Environment::Object(obj) => {
                     let key = locator.name().clone();
                     let obj = obj.clone();

core/engine/src/tests/env.rs

@@ -94,3 +94,53 @@ fn indirect_eval_function_var_binding_4350() {
         js_str!("[object Object],[object Object],[object Object]"),
     )]);
 }
+
+#[test]
+// https://github.com/boa-dev/boa/issues/5333
+fn eval_created_bindings_can_be_deleted_5333() {
+    run_test_actions([
+        TestAction::assert_eq(
+            indoc! {r#"
+                (function() {
+                    var initial = null;
+                    var deleted = null;
+                    var postDeletion;
+                    eval('initial = x; deleted = delete x; postDeletion = function() { x; }; var x;');
+                    try {
+                        postDeletion();
+                        return 'no throw';
+                    } catch (e) {
+                        return String(initial) + ':' + String(deleted) + ':' + e.name;
+                    }
+                }());
+            "#},
+            js_str!("undefined:true:ReferenceError"),
+        ),
+        TestAction::assert_eq(
+            indoc! {r#"
+                (function() {
+                    var initial;
+                    var deleted = null;
+                    var postDeletion;
+                    eval('initial = f; deleted = delete f; postDeletion = function() { f; }; function f() { return 33; }');
+                    try {
+                        postDeletion();
+                        return 'no throw';
+                    } catch (e) {
+                        return typeof initial + ':' + String(initial()) + ':' + String(deleted) + ':' + e.name;
+                    }
+                }());
+            "#},
+            js_str!("function:33:true:ReferenceError"),
+        ),
+        TestAction::assert_eq(
+            indoc! {r#"
+                (function() {
+                    eval('delete x; var x = 1;');
+                    return typeof globalThis.x + ':' + String(globalThis.x);
+                }());
+            "#},
+            js_str!("number:1"),
+        ),
+    ]);
+}

core/engine/src/vm/opcode/define/mod.rs

@@ -1,5 +1,6 @@
 use super::{IndexOperand, RegisterOperand};
 use crate::{Context, JsResult, JsValue, vm::opcode::Operation};
+use boa_ast::scope::BindingLocatorScope;
 
 pub(crate) mod class;
 pub(crate) mod own_property;
@@ -20,6 +21,14 @@ impl DefVar {
         // TODO: spec specifies to return `empty` on empty vars, but we're trying to initialize.
         let binding_locator = context.vm.frame().code_block.bindings[usize::from(index)].clone();
 
+        if let BindingLocatorScope::Stack(index) = binding_locator.scope()
+            && let crate::environments::Environment::Declarative(env) =
+                context.environment_expect(index)
+            && env.is_deleted_binding(binding_locator.binding_index())
+        {
+            return;
+        }
+
         {
             let frame = context.vm.frame_mut();
             let global = frame.realm.environment();