Skip to content
9 changes: 9 additions & 0 deletions crates/monty/src/bytecode/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 26 additions & 10 deletions crates/monty/src/bytecode/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 { .. } => {}
}
Expand Down Expand Up @@ -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);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not clear why this is necessary?

}
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);
}
}
}
Expand Down
25 changes: 21 additions & 4 deletions crates/monty/src/bytecode/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8> for Opcode {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
Expand All @@ -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);
Expand Down
Loading
Loading