diff --git a/crates/monty/src/bytecode/builder.rs b/crates/monty/src/bytecode/builder.rs index db4bb751..d0f03414 100644 --- a/crates/monty/src/bytecode/builder.rs +++ b/crates/monty/src/bytecode/builder.rs @@ -439,6 +439,15 @@ impl CodeBuilder { } } + /// Emits `DeleteLocal`, using the wide variant for slots above 255. + pub fn emit_delete_local(&mut self, slot: u16) { + if let Ok(s) = u8::try_from(slot) { + self.emit_u8(Opcode::DeleteLocal, s); + } else { + self.emit_u16(Opcode::DeleteLocalW, slot); + } + } + /// Adds a constant to the pool, returning its index. /// /// # Panics diff --git a/crates/monty/src/bytecode/compiler.rs b/crates/monty/src/bytecode/compiler.rs index 7bea26ec..43937830 100644 --- a/crates/monty/src/bytecode/compiler.rs +++ b/crates/monty/src/bytecode/compiler.rs @@ -21,8 +21,8 @@ use crate::{ exception_private::ExcType, exception_public::{MontyException, StackFrame}, expressions::{ - Callable, CmpOperator, Comprehension, DictItem, Expr, ExprLoc, Identifier, Literal, NameScope, Node, Operator, - PreparedFunctionDef, PreparedNode, SequenceItem, UnpackTarget, + Callable, CmpOperator, Comprehension, DeleteTarget, DictItem, Expr, ExprLoc, Identifier, Literal, NameScope, + Node, Operator, PreparedFunctionDef, PreparedNode, SequenceItem, UnpackTarget, }, fstring::{ConversionFlag, FStringPart, FormatSpec, ParsedFormatSpec, encode_format_spec}, function::Function, @@ -408,6 +408,21 @@ impl<'a> Compiler<'a> { } => self.compile_import_from(*module_name, names, *position), Node::Break { position } => self.compile_break(*position)?, Node::Continue { position } => self.compile_continue(*position)?, + Node::Delete(targets) => { + for target in targets { + match target { + DeleteTarget::Name(ident) => { + self.compile_delete(ident); + } + DeleteTarget::Subscript(sub) => { + self.compile_expr(&sub.target)?; + self.compile_expr(&sub.index)?; + self.code.set_location(sub.position, None); + self.code.emit(Opcode::DeleteSubscr); + } + } + } + } // These are handled during the prepare phase and produce no bytecode Node::Pass | Node::Global { .. } | Node::Nonlocal { .. } => {} } @@ -3088,25 +3103,26 @@ impl<'a> Compiler<'a> { /// because module-level locals live in the globals array. fn compile_delete(&mut self, target: &Identifier) { let slot = u16::try_from(target.namespace_id().index()).expect("local slot exceeds u16"); + self.code.set_location(target.position, None); match target.scope { NameScope::Local | NameScope::LocalUnassigned => { + self.code.register_local_name(slot, target.name_id); if self.is_module_scope { self.code.emit_u16(Opcode::DeleteGlobal, slot); - } else if let Ok(s) = u8::try_from(slot) { - self.code.emit_u8(Opcode::DeleteLocal, s); } else { - // Wide variant not implemented yet - todo!("DeleteLocalW for slot > 255"); + if matches!(target.scope, NameScope::Local) { + self.code.register_assigned_local(slot); + } + self.code.emit_delete_local(slot); } } NameScope::Global => { + self.code.register_local_name(slot, target.name_id); self.code.emit_u16(Opcode::DeleteGlobal, slot); } NameScope::Cell => { - // Delete cell not commonly needed - // For now, just store None - self.code.emit(Opcode::LoadNone); - self.compile_store(target); + self.code.register_local_name(slot, target.name_id); + self.code.emit_u16(Opcode::DeleteCell, slot); } } } diff --git a/crates/monty/src/bytecode/op.rs b/crates/monty/src/bytecode/op.rs index c3ff85fc..85a7ed8c 100644 --- a/crates/monty/src/bytecode/op.rs +++ b/crates/monty/src/bytecode/op.rs @@ -440,6 +440,18 @@ pub enum Opcode { /// Pops iterable (TOS), adds each item to set at stack position `len - 2 - depth`. /// Raises `TypeError` if iterable is not iterable. SetExtend, + /// Delete a subscript item (e.g., `del d['key']`, `del lst[0]`). + /// + /// Pops index (TOS) then container, calls `__delitem__` on the container. + /// Raises `TypeError` if the type doesn't support item deletion. + DeleteSubscr, + /// Delete local variable (wide, slot > 255). Operand: u16 slot. + DeleteLocalW, + /// Delete closure cell contents. Operand: u16 slot. + /// + /// The slot contains `Value::Ref(cell_id)` in the frame's local namespace. + /// Deleting the cell marks it unbound by storing `Value::Undefined`. + DeleteCell, } impl TryFrom for Opcode { @@ -477,7 +489,7 @@ impl Opcode { LoadLocal | LoadLocalW | LoadLocalCallable | LoadLocalCallableW | LoadGlobal | LoadGlobalCallable | LoadCell => 1, StoreLocal | StoreLocalW | StoreGlobal | StoreCell => -1, - DeleteLocal | DeleteGlobal => 0, // doesn't affect stack + DeleteLocal | DeleteLocalW | DeleteCell | DeleteGlobal => 0, // doesn't affect stack // Binary operations: pop 2, push 1 = -1 BinaryAdd | BinarySub | BinaryMul | BinaryDiv | BinaryFloorDiv | BinaryMod | BinaryPow | BinaryAnd @@ -553,6 +565,8 @@ impl Opcode { DictUpdate => -1, // SetExtend: pop iterable, add all items to set below = -1 SetExtend => -1, + // DeleteSubscr: pop index + container, push nothing = -2 + DeleteSubscr => -2, // Special Nop => 0, @@ -584,8 +598,8 @@ mod tests { #[test] fn test_opcode_roundtrip() { - // Verify that all opcodes from 0 to DeleteGlobal (last opcode) can be converted to u8 and back. - for byte in 0..=Opcode::DeleteGlobal as u8 { + // Verify that all opcodes from 0 to DeleteCell (last opcode) can be converted to u8 and back. + for byte in 0..=Opcode::DeleteCell as u8 { let opcode = Opcode::try_from(byte).unwrap(); assert_eq!(opcode as u8, byte, "opcode {opcode:?} has wrong discriminant"); } @@ -601,12 +615,15 @@ mod tests { assert_eq!(Opcode::DeleteGlobal as u8, 112); assert_eq!(Opcode::DictUpdate as u8, 113); assert_eq!(Opcode::SetExtend as u8, 114); + assert_eq!(Opcode::DeleteSubscr as u8, 115); + assert_eq!(Opcode::DeleteLocalW as u8, 116); + assert_eq!(Opcode::DeleteCell as u8, 117); } #[test] fn test_invalid_opcode() { // Byte just after the last valid opcode should fail - let result = Opcode::try_from(Opcode::SetExtend as u8 + 1); + let result = Opcode::try_from(Opcode::DeleteCell as u8 + 1); assert!(result.is_err()); // 255 should also fail let result = Opcode::try_from(255u8); diff --git a/crates/monty/src/bytecode/vm/mod.rs b/crates/monty/src/bytecode/vm/mod.rs index 333c49f5..8b59ce40 100644 --- a/crates/monty/src/bytecode/vm/mod.rs +++ b/crates/monty/src/bytecode/vm/mod.rs @@ -13,10 +13,11 @@ mod exceptions; mod format; mod scheduler; -use std::{cmp::Ordering, mem}; +use std::{cmp::Ordering, collections::HashSet, mem}; pub(crate) use call::CallResult; use scheduler::Scheduler; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::{ MontyObject, @@ -480,6 +481,10 @@ impl CallFrame<'_> { /// Note: This struct does not implement `Clone` because `Value` uses manual /// reference counting. Snapshots transfer ownership - they are not copied. #[derive(Debug, serde::Serialize, serde::Deserialize)] +#[expect( + clippy::box_collection, + reason = "Option> keeps struct small (8B vs 48B) — del is rare" +)] pub struct VMSnapshot { /// Operand stack — locals and operands interleaved per frame. /// @@ -507,6 +512,58 @@ pub struct VMSnapshot { /// /// Contains call ID counter, task state, pending calls, and resolved futures. scheduler: Scheduler, + + /// Global slots that have been explicitly deleted via `del`. + /// + /// Preserved across snapshots so that resumed execution raises `NameError` + /// (not `UnboundLocalError`) for previously deleted globals. + #[serde( + serialize_with = "serialize_deleted_globals", + deserialize_with = "deserialize_deleted_globals" + )] + deleted_globals: Option>>, +} + +/// Serializes deleted global slots using the legacy on-wire `HashSet` shape. +/// +/// Keeping the wire format stable avoids breaking persisted snapshots while still +/// allowing the in-memory VM representation to use `Option>` for footprint. +#[expect( + clippy::box_collection, + reason = "matches the compact in-memory VMSnapshot field we are serializing" +)] +#[expect( + clippy::ref_option, + reason = "serde serialize_with requires a shared reference to the field type" +)] +fn serialize_deleted_globals(deleted_globals: &Option>>, serializer: S) -> Result +where + S: Serializer, +{ + match deleted_globals { + Some(set) => set.serialize(serializer), + None => HashSet::::new().serialize(serializer), + } +} + +/// Deserializes deleted global slots from the legacy `HashSet` wire format. +/// +/// Empty sets map back to `None` so the VM keeps the compact "no deleted globals" +/// representation after loading a snapshot. +#[expect( + clippy::box_collection, + reason = "matches the compact in-memory VMSnapshot field we are reconstructing" +)] +fn deserialize_deleted_globals<'de, D>(deserializer: D) -> Result>>, D::Error> +where + D: Deserializer<'de>, +{ + let deleted_globals = HashSet::::deserialize(deserializer)?; + if deleted_globals.is_empty() { + Ok(None) + } else { + Ok(Some(Box::new(deleted_globals))) + } } // ============================================================================ @@ -522,6 +579,10 @@ pub struct VMSnapshot { /// # Lifetimes /// * `'a` - Lifetime of the heap, namespaces, and interns /// * `'p` - Lifetime of the print writer's internal references +#[expect( + clippy::box_collection, + reason = "Option> keeps struct small (8B vs 48B) — del is rare" +)] pub struct VM<'h, 'a, T: ResourceTracker> { /// Operand stack — locals and operands interleaved per frame. /// @@ -583,6 +644,17 @@ pub struct VM<'h, 'a, T: ResourceTracker> { /// back to a `NameError`, so the traceback points to the name reference rather than /// the call expression. ext_function_load_ip: Option, + + /// Global slots that have been explicitly deleted via `del`. + /// + /// Used by `load_global` to distinguish "deleted by `del`" (→ `NameError`) from + /// "unbound comprehension variable" (→ `UnboundLocalError`). Both appear as + /// `Value::Undefined` with `is_assigned_local` set, but CPython raises different + /// exception types for each case. + /// + /// Boxed and wrapped in `Option` to keep the VM struct small (8 bytes vs 48) + /// since `del` on globals is rare and the VM is accessed on every instruction. + deleted_globals: Option>>, } impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { @@ -605,6 +677,7 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { scheduler: Scheduler::new(), ext_function_load_ip: None, // Set by LoadGlobalCallable/LoadLocalCallable module_code: None, + deleted_globals: None, } } @@ -666,6 +739,7 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { scheduler: snapshot.scheduler, module_code: Some(module_code), ext_function_load_ip: None, + deleted_globals: snapshot.deleted_globals, } } /// Consumes the VM and creates a snapshot for pause/resume. @@ -686,6 +760,7 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { exception_stack: self.exception_stack, instruction_ip: self.instruction_ip, scheduler: self.scheduler, + deleted_globals: self.deleted_globals, } } @@ -857,11 +932,15 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { } Opcode::DeleteLocal => { let slot = u16::from(fetch_u8!(cached_frame)); - self.delete_local(&cached_frame, slot); + try_catch_sync!(self, cached_frame, self.delete_local(&cached_frame, slot)); + } + Opcode::DeleteLocalW => { + let slot = fetch_u16!(cached_frame); + try_catch_sync!(self, cached_frame, self.delete_local(&cached_frame, slot)); } Opcode::DeleteGlobal => { let slot = fetch_u16!(cached_frame); - self.delete_global(slot); + try_catch_sync!(self, cached_frame, self.delete_global(slot)); } // Variables - Callable-context Local Loads Opcode::LoadLocalCallable => { @@ -897,6 +976,10 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { let slot = fetch_u16!(cached_frame); self.store_cell(&cached_frame, slot); } + Opcode::DeleteCell => { + let slot = fetch_u16!(cached_frame); + try_catch_sync!(self, cached_frame, self.delete_cell(&cached_frame, slot)); + } // Binary Operations - route through exception handling for tracebacks Opcode::BinaryAdd => try_catch_sync!(self, cached_frame, self.binary_add()), Opcode::BinarySub => try_catch_sync!(self, cached_frame, self.binary_sub()), @@ -1147,6 +1230,15 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { catch_sync!(self, cached_frame, e); } } + Opcode::DeleteSubscr => { + let index = self.pop(); + let mut obj = self.pop(); + let result = obj.py_delitem(index, self); + obj.drop_with_heap(self); + if let Err(e) = result { + catch_sync!(self, cached_frame, e); + } + } Opcode::LoadAttr => { let name_idx = fetch_u16!(cached_frame); let name_id = StringId::from_index(name_idx); @@ -1828,11 +1920,23 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { old_value.drop_with_heap(self); } - /// Deletes a local variable (sets it to Undefined). - fn delete_local(&mut self, cached_frame: &CachedFrame<'a>, slot: u16) { - let target = &mut self.stack[cached_frame.stack_base + slot as usize]; - let old_value = mem::replace(target, Value::Undefined); + /// Deletes a local variable, raising the same error family as a load when the + /// target is already unbound. + fn delete_local(&mut self, cached_frame: &CachedFrame<'a>, slot: u16) -> RunResult<()> { + let is_assigned = cached_frame.code.is_assigned_local(slot); + let name = cached_frame.code.local_name(slot); + let slot_index = cached_frame.stack_base + slot as usize; + if matches!(self.stack.get(slot_index), Some(Value::Undefined)) { + return Err(if is_assigned { + self.unbound_local_error(slot, name) + } else { + self.name_error(slot, name) + }); + } + + let old_value = mem::replace(&mut self.stack[slot_index], Value::Undefined); old_value.drop_with_heap(self); + Ok(()) } /// Loads a global variable and pushes it onto the stack. @@ -1847,11 +1951,17 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { if matches!(value, Value::Undefined) { let name = self.current_frame().code.local_name(slot); - // If the name is registered as an assigned local (e.g. a module-level - // variable or comprehension loop variable), raise UnboundLocalError - // immediately rather than yielding NameLookup. + // If the name was explicitly deleted via `del`, raise NameError + // (CPython never raises UnboundLocalError for globals). + // Otherwise, if the name is registered as an assigned local (e.g. + // a comprehension loop variable), raise UnboundLocalError. if self.current_frame().code.is_assigned_local(slot) { - return Err(self.unbound_local_error(slot, name)); + let was_deleted = self.deleted_globals.as_ref().is_some_and(|set| set.contains(&slot)); + return Err(if was_deleted { + self.name_error(slot, name) + } else { + self.unbound_local_error(slot, name) + }); } let Some(name_id) = name else { @@ -1870,23 +1980,46 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { } /// Pops the top of stack and stores it in a global variable. + /// + /// Re-assigning a previously deleted global clears the deleted marker so that + /// a subsequent load raises `UnboundLocalError` (not `NameError`) if the slot + /// becomes undefined again through a different path. fn store_global(&mut self, slot: u16) { let value = self.pop(); let old_value = mem::replace(&mut self.globals[slot as usize], value); old_value.drop_with_heap(self); + if let Some(set) = &mut self.deleted_globals { + set.remove(&slot); + } } - /// Deletes a global variable (sets it to Undefined). - fn delete_global(&mut self, slot: u16) { - let old_value = mem::replace(&mut self.globals[slot as usize], Value::Undefined); + /// Deletes a global variable, raising `NameError` if the binding does not exist. + /// + /// Marks the slot as explicitly deleted so that `load_global` raises `NameError` + /// (not `UnboundLocalError`) on subsequent access — matching CPython's behaviour + /// for module-level `del`. + fn delete_global(&mut self, slot: u16) -> RunResult<()> { + let name = self.current_frame().code.local_name(slot); + let slot_index = slot as usize; + if matches!(self.globals.get(slot_index), Some(Value::Undefined)) { + return Err(self.name_error(slot, name)); + } + + let old_value = mem::replace(&mut self.globals[slot_index], Value::Undefined); old_value.drop_with_heap(self); + self.deleted_globals + .get_or_insert_with(|| Box::new(HashSet::new())) + .insert(slot); + Ok(()) } /// Loads from a closure cell and pushes onto the stack. /// /// The cell `HeapId` is read from the frame's local variable slot on the stack /// (cells are stored as `Value::Ref(cell_id)` at known positions in the locals region). - /// Returns a `NameError` if the cell value is undefined (free variable not bound). + /// Returns `UnboundLocalError` for the current function's own deleted/uninitialized + /// cell variables, and the free-variable `NameError` for captured cells from an + /// enclosing scope. fn load_cell(&mut self, cached_frame: &CachedFrame<'a>, slot: u16) -> RunResult<()> { let cell_id = self.cell_id_from_local(cached_frame, slot); let value = match self.heap.get(cell_id) { @@ -1894,11 +2027,15 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { _ => panic!("LoadCell: entry is not a Cell"), }; - // Check for undefined value - raise NameError for unbound free variable + // Check for undefined value and distinguish owned cell vars from free vars if matches!(value, Value::Undefined) { value.drop_with_heap(self); let name = cached_frame.code.local_name(slot); - return Err(self.free_var_error(name)); + return Err(if self.is_owned_cell_slot(slot) { + self.unbound_local_error(slot, name) + } else { + self.free_var_error(name) + }); } self.push(value); @@ -1924,6 +2061,20 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { ExcType::name_error_free_variable(&name_str).into() } + /// Returns whether a cell slot belongs to the current function rather than an + /// enclosing scope captured as a free variable. + /// + /// Slot layout: `[params][cell_vars][free_vars]`. Slots below + /// `total_slots() + cell_var_count` are owned by this function. + fn is_owned_cell_slot(&self, slot: u16) -> bool { + let Some(function_id) = self.current_frame().function_id else { + return false; + }; + let function = self.interns.get_function(function_id); + let first_free_var_slot = function.signature.total_slots() + function.cell_var_count; + usize::from(slot) < first_free_var_slot + } + /// Pops the top of stack and stores it in a closure cell. /// /// The cell `HeapId` is read from the frame's local variable slot on the stack. @@ -1939,6 +2090,31 @@ impl<'h, 'a, T: ResourceTracker> VM<'h, 'a, T> { }; mem::swap(&mut cell.get_mut(this.heap).0, value); } + + /// Deletes the value stored in a closure cell. + /// + /// Cell variables owned by the current function raise `UnboundLocalError` when + /// already unbound; captured free variables raise the free-variable `NameError`. + fn delete_cell(&mut self, cached_frame: &CachedFrame<'_>, slot: u16) -> RunResult<()> { + let is_owned_cell = self.is_owned_cell_slot(slot); + let name = cached_frame.code.local_name(slot); + let cell_id = self.cell_id_from_local(cached_frame, slot); + let HeapReadOutput::Cell(mut cell) = self.heap.read(cell_id) else { + panic!("DeleteCell: entry is not a Cell") + }; + + if matches!(&cell.get(self.heap).0, Value::Undefined) { + return Err(if is_owned_cell { + self.unbound_local_error(slot, name) + } else { + self.free_var_error(name) + }); + } + + let old_value = mem::replace(&mut cell.get_mut(self.heap).0, Value::Undefined); + old_value.drop_with_heap(self); + Ok(()) + } } // `heap` is not a public field on VM, so this implementation needs to go here rather than in `heap.rs` @@ -1951,3 +2127,63 @@ impl ContainsHeap for VM<'_, '_, T> { self.heap } } + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + + use serde::{Deserialize, Serialize}; + + use super::{deserialize_deleted_globals, serialize_deleted_globals}; + + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] + struct LegacyDeletedGlobals { + deleted_globals: HashSet, + } + + #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] + #[expect( + clippy::box_collection, + reason = "mirrors the production field shape for wire-format compatibility testing" + )] + struct CurrentDeletedGlobals { + #[serde( + serialize_with = "serialize_deleted_globals", + deserialize_with = "deserialize_deleted_globals" + )] + deleted_globals: Option>>, + } + + #[test] + fn deleted_globals_wire_format_matches_legacy_hashset() { + let mut deleted_globals = HashSet::new(); + deleted_globals.insert(3); + deleted_globals.insert(7); + + let legacy = LegacyDeletedGlobals { + deleted_globals: deleted_globals.clone(), + }; + let current = CurrentDeletedGlobals { + deleted_globals: Some(Box::new(deleted_globals)), + }; + + let legacy_bytes = postcard::to_allocvec(&legacy).expect("legacy snapshot should serialize"); + let current_bytes = postcard::to_allocvec(¤t).expect("current snapshot should serialize"); + assert_eq!( + current_bytes, legacy_bytes, + "wire format should remain backwards compatible" + ); + } + + #[test] + fn deleted_globals_empty_legacy_hashset_restores_to_none() { + let legacy = LegacyDeletedGlobals { + deleted_globals: HashSet::new(), + }; + let bytes = postcard::to_allocvec(&legacy).expect("legacy snapshot should serialize"); + let current: CurrentDeletedGlobals = + postcard::from_bytes(&bytes).expect("current snapshot should deserialize legacy bytes"); + + assert_eq!(current.deleted_globals, None); + } +} diff --git a/crates/monty/src/exception_private.rs b/crates/monty/src/exception_private.rs index 40f0c3b1..bb94f9fe 100644 --- a/crates/monty/src/exception_private.rs +++ b/crates/monty/src/exception_private.rs @@ -297,6 +297,18 @@ impl ExcType { .into() } + /// Creates a TypeError for item deletion on types that don't support it. + /// + /// Matches CPython's format: `TypeError: '{type}' object doesn't support item deletion` + #[must_use] + pub(crate) fn type_error_not_sub_deletion(type_: Type) -> RunError { + SimpleException::new_msg( + Self::TypeError, + format!("'{type_}' object doesn't support item deletion"), + ) + .into() + } + /// Creates a TypeError for unhashable types when calling `hash()`. /// /// This matches Python 3.14's error message: `TypeError: unhashable type: 'list'` diff --git a/crates/monty/src/expressions.rs b/crates/monty/src/expressions.rs index ef3199f6..e17fcb0a 100644 --- a/crates/monty/src/expressions.rs +++ b/crates/monty/src/expressions.rs @@ -369,6 +369,32 @@ pub enum UnpackTarget { Starred(Identifier), } +/// Target for the `del` statement. +/// +/// Each target specifies what to delete: a variable binding, a subscript item, +/// or (in the future) an attribute. `del x, d['key']` produces two targets. +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub enum DeleteTarget { + /// Delete a variable binding: `del x` + Name(Identifier), + /// Delete a subscript item: `del d['key']` or `del lst[0]` + Subscript(Box), +} + +/// Data for a subscript deletion target (`del d['key']`). +/// +/// Boxed inside `DeleteTarget::Subscript` to avoid a large enum variant size +/// difference (Identifier is 36 bytes vs ~276 bytes for two `ExprLoc`s + `CodeRange`). +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct DeleteSubscriptTarget { + /// The container expression (e.g., `d` in `del d['key']`) + pub target: ExprLoc, + /// The index/key expression (e.g., `'key'` in `del d['key']`) + pub index: ExprLoc, + /// Source position of the full subscript expression for traceback carets + pub position: CodeRange, +} + /// A generator clause in a comprehension: `for target in iter [if cond1] [if cond2]...` /// /// Represents one `for` clause with zero or more `if` filters. Multiple generators @@ -622,6 +648,11 @@ pub enum Node { /// Source position for error reporting. position: CodeRange, }, + /// Delete statement (e.g., `del x`, `del d['key']`, `del x, y`). + /// + /// Removes variable bindings or container items. Multiple targets in + /// `del a, b` are stored as separate entries and executed left-to-right. + Delete(Vec), } /// A prepared function definition with resolved names and scope information. diff --git a/crates/monty/src/heap_data.rs b/crates/monty/src/heap_data.rs index a7bf53c6..1eb9c600 100644 --- a/crates/monty/src/heap_data.rs +++ b/crates/monty/src/heap_data.rs @@ -889,6 +889,17 @@ impl<'h> PyTrait<'h> for HeapReadOutput<'h> { } } + fn py_delitem(&mut self, key: Value, vm: &mut VM<'h, '_, impl ResourceTracker>) -> RunResult<()> { + match self { + Self::List(l) => l.py_delitem(key, vm), + Self::Dict(d) => d.py_delitem(key, vm), + _ => { + key.drop_with_heap(vm); + Err(ExcType::type_error_not_sub_deletion(self.py_type(vm))) + } + } + } + fn py_getattr(&self, attr: &EitherStr, vm: &mut VM<'h, '_, impl ResourceTracker>) -> RunResult> { match self { Self::Str(s) => s.py_getattr(attr, vm), diff --git a/crates/monty/src/parse.rs b/crates/monty/src/parse.rs index 53a27bf4..796eee7a 100644 --- a/crates/monty/src/parse.rs +++ b/crates/monty/src/parse.rs @@ -16,8 +16,8 @@ use crate::{ exception_private::ExcType, exception_public::{CodeLoc, MontyException}, expressions::{ - Callable, CmpOperator, Comprehension, DictItem, Expr, ExprLoc, Identifier, ImportName, Literal, Node, Operator, - SequenceItem, UnpackTarget, + Callable, CmpOperator, Comprehension, DeleteSubscriptTarget, DeleteTarget, DictItem, Expr, ExprLoc, Identifier, + ImportName, Literal, Node, Operator, SequenceItem, UnpackTarget, }, fstring::{ConversionFlag, FStringPart, FormatSpec}, intern::{InternerBuilder, StringId}, @@ -286,10 +286,14 @@ impl<'a> Parser<'a> { Some(value) => Ok(Node::Return(self.parse_expression(*value)?)), None => Ok(Node::ReturnNone), }, - Stmt::Delete(d) => Err(ParseError::not_implemented( - "the 'del' statement", - self.convert_range(d.range), - )), + Stmt::Delete(d) => { + let targets = d + .targets + .into_iter() + .map(|t| self.parse_delete_target(t)) + .collect::, _>>()?; + Ok(Node::Delete(targets)) + } Stmt::TypeAlias(t) => Err(ParseError::not_implemented("type aliases", self.convert_range(t.range))), Stmt::Assign(ast::StmtAssign { targets, value, range, .. @@ -591,6 +595,28 @@ impl<'a> Parser<'a> { } } + /// Parses a single target of a `del` statement. + /// + /// Supports variable names (`del x`) and subscripts (`del d['key']`, `del lst[1:3]`). + /// Attribute deletion (`del obj.attr`) is not yet supported since Monty + /// does not have user-defined classes. + fn parse_delete_target(&mut self, target: AstExpr) -> Result { + match target { + AstExpr::Name(ast::ExprName { id, range, .. }) => Ok(DeleteTarget::Name(self.identifier(&id, range))), + AstExpr::Subscript(ast::ExprSubscript { + value, slice, range, .. + }) => Ok(DeleteTarget::Subscript(Box::new(DeleteSubscriptTarget { + target: self.parse_expression(*value)?, + index: self.parse_expression(*slice)?, + position: self.convert_range(range), + }))), + other => Err(ParseError::not_implemented( + "del with attribute targets", + self.convert_range(other.range()), + )), + } + } + /// Parses an expression from the ruff AST into Monty's ExprLoc representation. /// /// Includes depth tracking to prevent stack overflow from deeply nested structures. diff --git a/crates/monty/src/prepare.rs b/crates/monty/src/prepare.rs index 055eef96..51d32905 100644 --- a/crates/monty/src/prepare.rs +++ b/crates/monty/src/prepare.rs @@ -6,8 +6,8 @@ use crate::{ args::{ArgExprs, CallArg, CallKwarg}, builtins::Builtins, expressions::{ - Callable, CmpOperator, Comprehension, DictItem, Expr, ExprLoc, Identifier, ImportName, Literal, NameScope, - Node, Operator, PreparedFunctionDef, PreparedNode, SequenceItem, UnpackTarget, + Callable, CmpOperator, Comprehension, DeleteSubscriptTarget, DeleteTarget, DictItem, Expr, ExprLoc, Identifier, + ImportName, Literal, NameScope, Node, Operator, PreparedFunctionDef, PreparedNode, SequenceItem, UnpackTarget, }, fstring::{FStringPart, FormatSpec}, intern::{InternerBuilder, StringId}, @@ -613,6 +613,25 @@ impl<'i> Prepare<'i> { position, }); } + Node::Delete(targets) => { + let resolved = targets + .into_iter() + .map(|t| match t { + DeleteTarget::Name(ident) => { + let (resolved, _) = self.get_id(ident); + Ok(DeleteTarget::Name(resolved)) + } + DeleteTarget::Subscript(sub) => { + Ok(DeleteTarget::Subscript(Box::new(DeleteSubscriptTarget { + target: self.prepare_expression(sub.target)?, + index: self.prepare_expression(sub.index)?, + position: sub.position, + }))) + } + }) + .collect::, _>>()?; + new_nodes.push(Node::Delete(resolved)); + } } } Ok(new_nodes) @@ -2107,6 +2126,20 @@ fn collect_scope_info_from_node( collect_assigned_names_from_expr(m, assigned_names, interner); } } + // del marks variable names as assigned (same as CPython scoping) + Node::Delete(targets) => { + for target in targets { + match target { + DeleteTarget::Name(ident) => { + assigned_names.insert(interner.get_str(ident.name_id).to_string()); + } + DeleteTarget::Subscript(sub) => { + collect_assigned_names_from_expr(&sub.target, assigned_names, interner); + collect_assigned_names_from_expr(&sub.index, assigned_names, interner); + } + } + } + } // These don't create new names Node::Pass | Node::ReturnNone | Node::Raise(None) | Node::Break { .. } | Node::Continue { .. } => {} } @@ -2424,6 +2457,14 @@ fn collect_cell_vars_from_node( collect_cell_vars_from_expr(object, our_locals, cell_vars, interner); collect_cell_vars_from_expr(value, our_locals, cell_vars, interner); } + Node::Delete(targets) => { + for target in targets { + if let DeleteTarget::Subscript(sub) = target { + collect_cell_vars_from_expr(&sub.target, our_locals, cell_vars, interner); + collect_cell_vars_from_expr(&sub.index, our_locals, cell_vars, interner); + } + } + } // Other nodes don't contain nested function definitions or lambdas _ => {} } @@ -2754,6 +2795,14 @@ fn collect_referenced_names_from_node(node: &ParseNode, referenced: &mut AHashSe collect_referenced_names_from_node(n, referenced, interner); } } + Node::Delete(targets) => { + for target in targets { + if let DeleteTarget::Subscript(sub) = target { + collect_referenced_names_from_expr(&sub.target, referenced, interner); + collect_referenced_names_from_expr(&sub.index, referenced, interner); + } + } + } // Imports create bindings but don't reference names Node::Import { .. } | Node::ImportFrom { .. } => {} Node::Pass diff --git a/crates/monty/src/types/dict.rs b/crates/monty/src/types/dict.rs index 867c6d30..5d9e592f 100644 --- a/crates/monty/src/types/dict.rs +++ b/crates/monty/src/types/dict.rs @@ -628,6 +628,18 @@ impl<'h> PyTrait<'h> for HeapRead<'h, Dict> { Ok(()) } + fn py_delitem(&mut self, key: Value, vm: &mut VM<'h, '_, impl ResourceTracker>) -> RunResult<()> { + defer_drop!(key, vm); + match self.pop(key, vm)? { + Some((k, v)) => { + k.drop_with_heap(vm); + v.drop_with_heap(vm); + Ok(()) + } + None => Err(ExcType::key_error(key, vm)), + } + } + fn py_call_attr( &mut self, self_id: HeapId, diff --git a/crates/monty/src/types/list.rs b/crates/monty/src/types/list.rs index ffdd4a81..5eb05ec4 100644 --- a/crates/monty/src/types/list.rs +++ b/crates/monty/src/types/list.rs @@ -201,6 +201,28 @@ impl<'h> HeapRead<'h, List> { Ok(Value::Ref(heap_id)) } + /// Deletes a slice of items from the list (e.g., `del lst[1:4]`). + /// + /// Collects matching indices, then removes them in reverse order so that + /// earlier indices remain valid as later ones are removed. + fn delitem_slice(&mut self, slice: &super::Slice, vm: &mut VM<'h, '_, impl ResourceTracker>) -> RunResult<()> { + let len = self.get(vm.heap).items.len(); + let (start, stop, step) = slice + .indices(len) + .map_err(|()| ExcType::value_error_slice_step_zero())?; + + let mut indices = collect_slice_indices(start, stop, step, len); + + // Sort descending so we remove from highest index first, preserving + // lower indices as we go. + indices.sort_unstable_by(|a, b| b.cmp(a)); + for &idx in &indices { + let removed = self.get_mut(vm.heap).items.remove(idx); + removed.drop_with_heap(vm); + } + Ok(()) + } + /// Clones the item at the given index with proper refcount management. pub(crate) fn clone_item(&self, index: usize, vm: &mut VM<'h, '_, impl ResourceTracker>) -> Value { self.get(vm.heap).items[index].clone_with_heap(vm.heap) @@ -306,6 +328,32 @@ impl<'h> PyTrait<'h> for HeapRead<'h, List> { Ok(()) } + fn py_delitem(&mut self, key: Value, vm: &mut VM<'h, '_, impl ResourceTracker>) -> RunResult<()> { + defer_drop!(key, vm); + + // Slice deletion: `del lst[1:4]` or `del lst[::2]` + // Copy the slice data to release the immutable heap borrow before mutating. + if let Value::Ref(id) = *key + && let HeapData::Slice(slice) = vm.heap.get(id) + { + let slice = slice.clone(); + return self.delitem_slice(&slice, vm); + } + + let index = key.as_index(vm, Type::List)?; + let len = i64::try_from(self.get(vm.heap).len()).expect("list length exceeds i64::MAX"); + let normalized = if index < 0 { index + len } else { index }; + + if normalized < 0 || normalized >= len { + return Err(ExcType::list_assignment_index_error()); + } + + let idx = usize::try_from(normalized).expect("list index validated non-negative"); + let removed = self.get_mut(vm.heap).items.remove(idx); + removed.drop_with_heap(vm); + Ok(()) + } + fn py_eq(&self, other: &Self, vm: &mut VM<'h, '_, impl ResourceTracker>) -> Result { let a_len = self.get(vm.heap).items.len(); if a_len != other.get(vm.heap).items.len() { @@ -864,6 +912,38 @@ pub(crate) fn get_slice_items( Ok(result) } +/// Collects the indices selected by a slice, in forward order. +/// +/// Uses `i64` arithmetic (matching `get_slice_items`) so the sentinel +/// `stop > len` value for negative steps that mean "go to the beginning" +/// is handled correctly. +fn collect_slice_indices(start: usize, stop: usize, step: i64, len: usize) -> Vec { + let mut indices = Vec::new(); + if let Ok(step_usize) = usize::try_from(step) { + let mut i = start; + while i < stop && i < len { + indices.push(i); + i += step_usize; + } + } else { + let step_abs = i64::try_from(step.unsigned_abs()).expect("step magnitude fits i64"); + let mut i = i64::try_from(start).expect("start fits i64"); + let stop_i64 = if stop > len { + -1 + } else { + i64::try_from(stop).expect("stop fits i64") + }; + while let Ok(i_usize) = usize::try_from(i) { + if i_usize >= len || i <= stop_i64 { + break; + } + indices.push(i_usize); + i -= step_abs; + } + } + indices +} + #[cfg(test)] mod tests { use num_bigint::BigInt; diff --git a/crates/monty/src/types/py_trait.rs b/crates/monty/src/types/py_trait.rs index 75563fcb..88ec3fb5 100644 --- a/crates/monty/src/types/py_trait.rs +++ b/crates/monty/src/types/py_trait.rs @@ -320,6 +320,17 @@ pub trait PyTrait<'h> { .into()) } + /// Python subscript delete operation (`__delitem__`), e.g., `del d[key]`. + /// + /// Removes the item associated with the key, or returns an error if the key + /// doesn't exist or the type doesn't support item deletion. + /// + /// Default implementation returns TypeError. + fn py_delitem(&mut self, key: Value, vm: &mut VM<'h, '_, impl ResourceTracker>) -> RunResult<()> { + key.drop_with_heap(vm); + Err(ExcType::type_error_not_sub_deletion(self.py_type(vm))) + } + /// Python attribute get operation (`__getattr__`), e.g., `obj.attr`. /// /// Returns the value associated with the attribute (owned), or `Ok(None)` if the type diff --git a/crates/monty/src/value.rs b/crates/monty/src/value.rs index 06b394cb..f8e32a95 100644 --- a/crates/monty/src/value.rs +++ b/crates/monty/src/value.rs @@ -1362,6 +1362,15 @@ impl PyTrait<'_> for Value { ))), } } + + fn py_delitem(&mut self, key: Self, vm: &mut VM<'_, '_, impl ResourceTracker>) -> RunResult<()> { + if let Self::Ref(id) = self { + vm.heap.read(*id).py_delitem(key, vm) + } else { + key.drop_with_heap(vm); + Err(ExcType::type_error_not_sub_deletion(self.py_type(vm))) + } + } } impl Value { diff --git a/crates/monty/test_cases/del__all.py b/crates/monty/test_cases/del__all.py new file mode 100644 index 00000000..f628cf5d --- /dev/null +++ b/crates/monty/test_cases/del__all.py @@ -0,0 +1,230 @@ +# === del variable (becomes unbound local) === +x = 42 +del x +try: + x + assert False, 'expected error after del' +except NameError as exc: + assert str(exc) == "name 'x' is not defined", f'wrong msg: {exc}' + +# === del dict key === +d = {'a': 1, 'b': 2, 'c': 3} +del d['b'] +assert d == {'a': 1, 'c': 3}, f'dict after del: {d}' + +# === del dict key (variable index) === +d2 = {'x': 10, 'y': 20} +key = 'x' +del d2[key] +assert d2 == {'y': 20}, f'dict after del with var key: {d2}' + +# === del list item === +lst = [1, 2, 3, 4, 5] +del lst[1] +assert lst == [1, 3, 4, 5], f'list after del: {lst}' + +# === del list negative index === +lst2 = [10, 20, 30] +del lst2[-1] +assert lst2 == [10, 20], f'list after del[-1]: {lst2}' + +# === del multiple targets === +a = 1 +b = 2 +c = 3 +del a, b +try: + a + assert False, 'expected error for a' +except NameError as exc: + assert str(exc) == "name 'a' is not defined", f'wrong msg: {exc}' +try: + b + assert False, 'expected error for b' +except NameError as exc: + assert str(exc) == "name 'b' is not defined", f'wrong msg: {exc}' +assert c == 3, 'c should be unchanged' + +# === del missing global raises NameError === +try: + del missing + assert False, 'expected NameError for missing' +except NameError as exc: + assert str(exc) == "name 'missing' is not defined", f'wrong msg: {exc}' + +# === del duplicated target raises on second delete === +dup = 1 +try: + del dup, dup + assert False, 'expected NameError for repeated delete target' +except NameError as exc: + assert str(exc) == "name 'dup' is not defined", f'wrong msg: {exc}' + +try: + dup + assert False, 'expected dup to remain deleted' +except NameError as exc: + assert str(exc) == "name 'dup' is not defined", f'wrong msg: {exc}' + + +# === del unbound local raises UnboundLocalError === +def delete_unbound_local(): + try: + del local_name + assert False, 'expected UnboundLocalError for local delete' + except UnboundLocalError as exc: + return str(exc) + + local_name = 1 + + +assert delete_unbound_local() == ( + "cannot access local variable 'local_name' where it is not associated with a value" +), 'wrong unbound-local delete message' + + +# === del captured local keeps cell unbound === +def delete_captured_local(): + x = 1 + + def inner(): + return x + + del x + + try: + x + assert False, 'expected UnboundLocalError after deleting captured local' + except UnboundLocalError as exc: + outer_msg = str(exc) + + try: + inner() + assert False, 'expected NameError from closure after delete' + except NameError as exc: + inner_msg = str(exc) + + return outer_msg, inner_msg + + +assert delete_captured_local() == ( + "cannot access local variable 'x' where it is not associated with a value", + "cannot access free variable 'x' where it is not associated with a value in enclosing scope", +), 'wrong messages after deleting captured local' + + +# === del nonlocal keeps outer cell unbound === +def delete_nonlocal_binding(): + x = 1 + + def delete_x(): + nonlocal x + del x + + def read_x(): + return x + + delete_x() + + try: + x + assert False, 'expected UnboundLocalError in outer scope after nonlocal del' + except UnboundLocalError as exc: + outer_msg = str(exc) + + try: + read_x() + assert False, 'expected NameError in sibling closure after nonlocal del' + except NameError as exc: + inner_msg = str(exc) + + return outer_msg, inner_msg + + +assert delete_nonlocal_binding() == ( + "cannot access local variable 'x' where it is not associated with a value", + "cannot access free variable 'x' where it is not associated with a value in enclosing scope", +), 'wrong messages after deleting nonlocal binding' + +# === del and re-assign === +val = 'hello' +del val +val = 'world' +assert val == 'world', f'reassigned value: {val}' + +# === del in loop === +d3 = {'a': 1, 'b': 2, 'c': 3} +keys_to_remove = ['a', 'c'] +for k in keys_to_remove: + del d3[k] +assert d3 == {'b': 2}, f'dict after loop del: {d3}' + +# === del list first element repeatedly === +lst3 = [1, 2, 3] +del lst3[0] +del lst3[0] +assert lst3 == [3], f'list after repeated del[0]: {lst3}' + +# === del dict integer key === +d4 = {1: 'one', 2: 'two', 3: 'three'} +del d4[2] +assert d4 == {1: 'one', 3: 'three'}, f'dict int key del: {d4}' + +# === del list with len check === +lst4 = [10, 20, 30, 40] +assert len(lst4) == 4, 'initial len' +del lst4[2] +assert len(lst4) == 3, 'len after del' +assert lst4 == [10, 20, 40], f'list after del[2]: {lst4}' + +# === del dict missing key raises KeyError === +d5 = {'a': 1} +try: + del d5['b'] + assert False, 'expected KeyError' +except KeyError: + pass + +# === del list out of range raises IndexError === +lst5 = [1, 2] +try: + del lst5[10] + assert False, 'expected IndexError' +except IndexError: + pass + +# === del list slice (contiguous) === +lst6 = [0, 1, 2, 3, 4, 5] +del lst6[1:4] +assert lst6 == [0, 4, 5], f'list after del[1:4]: {lst6}' + +# === del list slice (step) === +lst7 = [0, 1, 2, 3, 4, 5] +del lst7[::2] +assert lst7 == [1, 3, 5], f'list after del[::2]: {lst7}' + +# === del list slice (reverse) === +lst8 = [0, 1, 2, 3, 4, 5] +del lst8[::-1] +assert lst8 == [], f'list after del[::-1]: {lst8}' + +# === del list slice (empty range) === +lst9 = [0, 1, 2] +del lst9[1:1] +assert lst9 == [0, 1, 2], f'list after del[1:1]: {lst9}' + +# === del list slice step zero raises ValueError === +lst10 = [0, 1, 2] +try: + del lst10[::0] + assert False, 'expected ValueError' +except ValueError as e: + assert str(e) == 'slice step cannot be zero', f'wrong msg: {e}' + +# === del on non-subscriptable type raises TypeError === +t = (1, 2, 3) +try: + del t[0] + assert False, 'expected TypeError' +except TypeError as e: + assert str(e) == "'tuple' object doesn't support item deletion", f'wrong msg: {e}' diff --git a/crates/monty/test_cases/del__list_index_error.py b/crates/monty/test_cases/del__list_index_error.py new file mode 100644 index 00000000..a3dcf36b --- /dev/null +++ b/crates/monty/test_cases/del__list_index_error.py @@ -0,0 +1,10 @@ +lst = [1, 2, 3] +del lst[10] +""" +TRACEBACK: +Traceback (most recent call last): + File "del__list_index_error.py", line 2, in + del lst[10] + ~~~~~~~ +IndexError: list assignment index out of range +""" diff --git a/crates/monty/test_cases/del__type_error.py b/crates/monty/test_cases/del__type_error.py new file mode 100644 index 00000000..797dc70c --- /dev/null +++ b/crates/monty/test_cases/del__type_error.py @@ -0,0 +1,10 @@ +t = (1, 2, 3) +del t[0] +""" +TRACEBACK: +Traceback (most recent call last): + File "del__type_error.py", line 2, in + del t[0] + ~~~~ +TypeError: 'tuple' object doesn't support item deletion +""" diff --git a/crates/monty/tests/bytecode_limits.rs b/crates/monty/tests/bytecode_limits.rs index a4717af7..a23fc47e 100644 --- a/crates/monty/tests/bytecode_limits.rs +++ b/crates/monty/tests/bytecode_limits.rs @@ -24,6 +24,20 @@ fn generate_many_locals(count: usize) -> String { code } +/// Generates Python code that deletes the highest-index local variable. +/// +/// Creates: `def f(): v0=0; ...; v{n-1}={n-1}; del v{n-1}; return v{n-2}\nf()` +fn generate_many_locals_with_delete(count: usize) -> String { + let mut code = String::from("def f():\n"); + for i in 0..count { + writeln!(code, " v{i} = {i}").unwrap(); + } + writeln!(code, " del v{}", count - 1).unwrap(); + writeln!(code, " return v{}", count - 2).unwrap(); + code.push_str("f()"); + code +} + /// Generates Python code calling a function with N positional arguments. /// /// Creates: `def f(*args): return len(args)\nf(0, 1, 2, ..., n-1)` @@ -96,6 +110,8 @@ fn assert_syntax_error(result: Result, expected } mod local_variable_limits { + use monty::MontyObject; + use super::*; #[test] @@ -148,6 +164,14 @@ mod local_variable_limits { let result = run.run_no_limits(vec![]); assert!(result.is_ok(), "300 locals should run successfully"); } + + #[test] + fn delete_local_above_u8_limit_uses_wide_instruction() { + let code = generate_many_locals_with_delete(300); + let run = MontyRun::new(code, "test.py", vec![]).expect("300 locals with delete should compile"); + let result = run.run_no_limits(vec![]).expect("wide delete should run successfully"); + assert_eq!(result.as_ref(), &MontyObject::Int(298)); + } } mod function_argument_limits { diff --git a/crates/monty/tests/delete.rs b/crates/monty/tests/delete.rs new file mode 100644 index 00000000..09a3d9b3 --- /dev/null +++ b/crates/monty/tests/delete.rs @@ -0,0 +1,55 @@ +use monty::{ExcType, MontyException, MontyRun}; + +/// Runs code and asserts that execution fails with the expected exception type and message. +fn assert_runtime_error(code: &str, expected_type: ExcType, expected_message: &str) { + let run = MontyRun::new(code.to_owned(), "test.py", vec![]).expect("code should compile"); + let err = run.run_no_limits(vec![]).expect_err("code should raise"); + assert_exception(&err, expected_type, expected_message); +} + +/// Asserts the exception type and message without caring whether it came from parse or runtime. +fn assert_exception(err: &MontyException, expected_type: ExcType, expected_message: &str) { + assert_eq!(err.exc_type(), expected_type); + assert_eq!(err.message(), Some(expected_message)); +} + +#[test] +fn deleting_missing_global_raises_name_error() { + assert_runtime_error("del missing", ExcType::NameError, "name 'missing' is not defined"); +} + +#[test] +fn deleting_unbound_local_raises_unbound_local_error() { + assert_runtime_error( + "def f():\n del x\n x = 1\nf()", + ExcType::UnboundLocalError, + "cannot access local variable 'x' where it is not associated with a value", + ); +} + +#[test] +fn deleting_captured_local_keeps_outer_name_unbound() { + assert_runtime_error( + "def outer():\n x = 1\n def inner():\n return x\n del x\n return x\nouter()", + ExcType::UnboundLocalError, + "cannot access local variable 'x' where it is not associated with a value", + ); +} + +#[test] +fn deleting_captured_local_keeps_inner_closure_unbound() { + assert_runtime_error( + "def outer():\n x = 1\n def inner():\n return x\n del x\n return inner()\nouter()", + ExcType::NameError, + "cannot access free variable 'x' where it is not associated with a value in enclosing scope", + ); +} + +#[test] +fn deleting_nonlocal_keeps_outer_cell_unbound() { + assert_runtime_error( + "def outer():\n x = 1\n def delete_x():\n nonlocal x\n del x\n delete_x()\n return x\nouter()", + ExcType::UnboundLocalError, + "cannot access local variable 'x' where it is not associated with a value", + ); +} diff --git a/crates/monty/tests/parse_errors.rs b/crates/monty/tests/parse_errors.rs index 1f62b006..b0ff0455 100644 --- a/crates/monty/tests/parse_errors.rs +++ b/crates/monty/tests/parse_errors.rs @@ -434,8 +434,9 @@ fn matrix_multiplication_augmented_assignment_has_descriptive_message() { } #[test] -fn del_statement_returns_not_implemented_error() { - // The del statement is not supported at parse time - let result = MontyRun::new("x = 1\ndel x".to_owned(), "test.py", vec![]); - assert_eq!(get_exc_type(result), ExcType::NotImplementedError); +fn del_statement_compiles_and_runs() { + let run = + MontyRun::new("x = 1\ndel x".to_owned(), "test.py", vec![]).expect("del statement should compile successfully"); + let result = run.run_no_limits(vec![]); + assert!(result.is_ok(), "del statement should execute successfully"); }