From 49b348cc7e6291c1e982183a8c76294024b26eca Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Mon, 14 Apr 2025 22:33:55 -0700 Subject: [PATCH 01/10] Remove Instruction::IsOperation --- compiler/codegen/src/compile.rs | 6 ++++-- .../rustpython_codegen__compile__tests__match.snap | 2 +- compiler/core/src/bytecode.rs | 7 +------ vm/src/frame.rs | 8 -------- 4 files changed, 6 insertions(+), 17 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 3c92146c13..8aa968b51a 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -2223,6 +2223,8 @@ impl Compiler<'_> { }); } + use bytecode::TestOperator::*; + // Emit instructions: // 1. Load the new tuple of attribute names. self.emit_load_const(ConstantData::Tuple { @@ -2235,7 +2237,7 @@ impl Compiler<'_> { // 4. Load None. self.emit_load_const(ConstantData::None); // 5. Compare with IS_OP 1. - emit!(self, Instruction::IsOperation(true)); + emit!(self, Instruction::TestOperation { op: IsNot }); // At this point the TOS is a tuple of (nargs + n_attrs) attributes (or None). pc.on_top += 1; @@ -2351,7 +2353,7 @@ impl Compiler<'_> { // emit!(self, Instruction::CopyItem { index: 1_u32 }); // self.emit_load_const(ConstantData::None); // // TODO: should be is - // emit!(self, Instruction::IsOperation(true)); + // emit!(self, Instruction::TestOperation::IsNot); // self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; // // Unpack the tuple of values. diff --git a/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap b/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap index f09f0f5eaf..3f6f4495d7 100644 --- a/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap +++ b/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap @@ -30,7 +30,7 @@ expression: "compile_exec(\"\\\nclass Test:\n pass\n\nt = Test()\nmatch t:\n 14 MatchClass (0) 15 CopyItem (1) 16 LoadConst (None) - 17 IsOperation (true) + 17 TestOperation (IsNot) 18 JumpIfFalse (27) 19 UnpackSequence (0) 20 Pop diff --git a/compiler/core/src/bytecode.rs b/compiler/core/src/bytecode.rs index 81dd591ad1..e00ca28a58 100644 --- a/compiler/core/src/bytecode.rs +++ b/compiler/core/src/bytecode.rs @@ -437,9 +437,6 @@ pub enum Instruction { TestOperation { op: Arg, }, - /// If the argument is true, perform IS NOT. Otherwise perform the IS operation. - // TODO: duplication of TestOperator::{Is,IsNot}. Fix later. - IsOperation(Arg), CompareOperation { op: Arg, }, @@ -1227,8 +1224,7 @@ impl Instruction { BinaryOperation { .. } | BinaryOperationInplace { .. } | TestOperation { .. } - | CompareOperation { .. } - | IsOperation(..) => -1, + | CompareOperation { .. } => -1, BinarySubscript => -1, CopyItem { .. } => 1, Pop => -1, @@ -1436,7 +1432,6 @@ impl Instruction { BinarySubscript => w!(BinarySubscript), LoadAttr { idx } => w!(LoadAttr, name = idx), TestOperation { op } => w!(TestOperation, ?op), - IsOperation(neg) => w!(IsOperation, neg), CompareOperation { op } => w!(CompareOperation, ?op), CopyItem { index } => w!(CopyItem, index), Pop => w!(Pop), diff --git a/vm/src/frame.rs b/vm/src/frame.rs index 6ffb454f54..dbe5cb077a 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -851,14 +851,6 @@ impl ExecutingFrame<'_> { bytecode::Instruction::UnaryOperation { op } => self.execute_unary_op(vm, op.get(arg)), bytecode::Instruction::TestOperation { op } => self.execute_test(vm, op.get(arg)), bytecode::Instruction::CompareOperation { op } => self.execute_compare(vm, op.get(arg)), - bytecode::Instruction::IsOperation(neg) => { - let a = self.pop_value(); - let b = self.pop_value(); - // xor with neg to invert the result if needed - let result = vm.ctx.new_bool(a.is(b.as_ref()) ^ neg.get(arg)); - self.push_value(result.into()); - Ok(None) - } bytecode::Instruction::ReturnValue => { let value = self.pop_value(); self.unwind_blocks(vm, UnwindReason::Returning { value }) From c7042fd8470ad8d9ca4c33246d6b390b74855b85 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 08:39:58 -0700 Subject: [PATCH 02/10] remove unneeded validation --- compiler/codegen/src/compile.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 8aa968b51a..052bd1b290 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -2185,16 +2185,6 @@ impl Compiler<'_> { let nargs = patterns.len(); let n_attrs = kwd_attrs.len(); - let nkwd_patterns = kwd_patterns.len(); - - // Validate that keyword attribute names and patterns match in length. - if n_attrs != nkwd_patterns { - let msg = format!( - "kwd_attrs ({}) / kwd_patterns ({}) length mismatch in class pattern", - n_attrs, nkwd_patterns - ); - unreachable!("{}", msg); - } // Check for too many sub-patterns. if nargs > u32::MAX as usize || (nargs + n_attrs).saturating_sub(1) > i32::MAX as usize { From 456e555e8b173cde5ed16f2d784ee56a4601843c Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 09:25:06 -0700 Subject: [PATCH 03/10] better error --- compiler/codegen/src/ir.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/codegen/src/ir.rs b/compiler/codegen/src/ir.rs index 39857e6fc5..9106d3fdff 100644 --- a/compiler/codegen/src/ir.rs +++ b/compiler/codegen/src/ir.rs @@ -244,7 +244,8 @@ impl CodeInfo { let instr_display = instr.display(display_arg, self); eprint!("{instr_display}: {depth} {effect:+} => "); } - let new_depth = depth.checked_add_signed(effect).unwrap(); + // Either it's less than 0 or more than u32. Option 1 is far more likely unless something is seriously wrong. + let new_depth = depth.checked_add_signed(effect).expect("The stack is detected to have likely been reduced to less than 0 elements, this is a bug."); if DEBUG { eprintln!("{new_depth}"); } From d47944b2fd74db8da80e8965273df2fabcf2ec36 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 10:02:32 -0700 Subject: [PATCH 04/10] error handling --- compiler/codegen/src/ir.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/codegen/src/ir.rs b/compiler/codegen/src/ir.rs index 9106d3fdff..662ba81dbe 100644 --- a/compiler/codegen/src/ir.rs +++ b/compiler/codegen/src/ir.rs @@ -244,8 +244,10 @@ impl CodeInfo { let instr_display = instr.display(display_arg, self); eprint!("{instr_display}: {depth} {effect:+} => "); } - // Either it's less than 0 or more than u32. Option 1 is far more likely unless something is seriously wrong. - let new_depth = depth.checked_add_signed(effect).expect("The stack is detected to have likely been reduced to less than 0 elements, this is a bug."); + if effect < 0 && depth < effect.abs() as u32 { + panic!("The stack will underflow at {depth} with {effect} effect on {instr:?}"); + } + let new_depth = depth.checked_add_signed(effect).unwrap(); if DEBUG { eprintln!("{new_depth}"); } From 09c199a1bab2a292fe4370fcde255d89dbefef51 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 10:03:29 -0700 Subject: [PATCH 05/10] match cleanup --- compiler/codegen/src/compile.rs | 99 ++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 052bd1b290..297feef67a 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -1944,32 +1944,35 @@ impl Compiler<'_> { n: Option<&Identifier>, pc: &mut PatternContext, ) -> CompileResult<()> { - // If no name is provided, simply pop the top of the stack. - if n.is_none() { - emit!(self, Instruction::Pop); - return Ok(()); - } - let name = n.unwrap(); - - // Check if the name is forbidden for storing. - if self.forbidden_name(name.as_str(), NameUsage::Store)? { - return Err(self.compile_error_forbidden_name(name.as_str())); - } + match n { + // If no name is provided, simply pop the top of the stack. + None => { + emit!(self, Instruction::Pop); + return Ok(()); + } + Some(name) => { + // Check if the name is forbidden for storing. + if self.forbidden_name(name.as_str(), NameUsage::Store)? { + return Err(self.compile_error_forbidden_name(name.as_str())); + } - // Ensure we don't store the same name twice. - if pc.stores.contains(&name.to_string()) { - return Err(self.error(CodegenErrorType::DuplicateStore(name.as_str().to_string()))); - } + // Ensure we don't store the same name twice. + // TODO: maybe pc.stores should be a set? + if pc.stores.contains(&name.to_string()) { + return Err( + self.error(CodegenErrorType::DuplicateStore(name.as_str().to_string())) + ); + } - // Calculate how many items to rotate: - // the count is the number of items to preserve on top plus the current stored names, - // plus one for the new value. - let rotations = pc.on_top + pc.stores.len() + 1; - self.pattern_helper_rotate(rotations)?; + // Calculate how many items to rotate: + let rotations = pc.on_top + pc.stores.len() + 1; + self.pattern_helper_rotate(rotations)?; - // Append the name to the captured stores. - pc.stores.push(name.to_string()); - Ok(()) + // Append the name to the captured stores. + pc.stores.push(name.to_string()); + Ok(()) + } + } } fn pattern_unpack_helper(&mut self, elts: &[Pattern]) -> CompileResult<()> { @@ -2245,20 +2248,12 @@ impl Compiler<'_> { pc.on_top -= 1; // Process each sub-pattern. - for i in 0..total { - // Decrement the on_top counter as each sub-pattern is processed. + for subpattern in patterns.iter().chain(kwd_patterns.iter()) { + // Decrement the on_top counter as each sub-pattern is processed + // (on_top should be zero at the end of the algorithm as a sanity check). pc.on_top -= 1; - let subpattern = if i < nargs { - // Positional sub-pattern. - &patterns[i] - } else { - // Keyword sub-pattern. - &kwd_patterns[i - nargs] - }; if subpattern.is_wildcard() { - // For wildcard patterns, simply pop the top of the stack. emit!(self, Instruction::Pop); - continue; } // Compile the subpattern without irrefutability checks. self.compile_pattern_subpattern(subpattern, pc)?; @@ -2479,7 +2474,7 @@ impl Compiler<'_> { self.pattern_helper_rotate(nrots)?; let name = &control.as_ref().unwrap()[i]; // Check for duplicate binding. - if pc.stores.iter().any(|n| n == name) { + if pc.stores.contains(name) { return Err(self.error(CodegenErrorType::DuplicateStore(name.to_string()))); } pc.stores.push(name.clone()); @@ -4607,17 +4602,33 @@ for stop_exc in (StopIteration('spam'), StopAsyncIteration('ham')): #[test] fn test_match() { assert_dis_snapshot!(compile_exec( - "\ -class Test: + r#"\ +class Shape: pass -t = Test() -match t: - case Test(): - assert True - case _: - assert False -" +class Circle(Shape): + def __init__(self, radius): + self.radius = radius + +class Rectangle(Shape): + def __init__(self, width, height): + self.width = width + self.height = height + +def describe_shape(shape): + match shape: + case Circle(radius=r): + return f"A circle with radius {r}" + case Rectangle(width=w, height=h): + return f"A rectangle {w} by {h}" + case _: + return "Unknown shape" + +# Test it out +shapes = [Circle(5), Rectangle(4, 6), "not a shape"] +for s in shapes: + print(describe_shape(s)) +"# )); } } From e949c9aa3f070b4909f81d1ac68ad3442aa9fba5 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 11:41:27 -0700 Subject: [PATCH 06/10] rename --- compiler/codegen/src/compile.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 297feef67a..286f318ccb 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -2467,11 +2467,11 @@ impl Compiler<'_> { self.switch_to_block(end); // Adjust the final captures. - let nstores = control.as_ref().unwrap().len(); - let nrots = nstores + 1 + pc.on_top + pc.stores.len(); - for i in 0..nstores { + let n_stores = control.as_ref().unwrap().len(); + let n_rots = n_stores + 1 + pc.on_top + pc.stores.len(); + for i in 0..n_stores { // Rotate the capture to its proper place. - self.pattern_helper_rotate(nrots)?; + self.pattern_helper_rotate(n_rots)?; let name = &control.as_ref().unwrap()[i]; // Check for duplicate binding. if pc.stores.contains(name) { From 628287c14e293224d5c1890bb343cbe3f763a1a9 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 11:42:13 -0700 Subject: [PATCH 07/10] update snapshot --- ...python_codegen__compile__tests__match.snap | 191 ++++++++++++++---- 1 file changed, 149 insertions(+), 42 deletions(-) diff --git a/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap b/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap index 3f6f4495d7..379d42f18b 100644 --- a/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap +++ b/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap @@ -1,53 +1,160 @@ --- source: compiler/codegen/src/compile.rs -expression: "compile_exec(\"\\\nclass Test:\n pass\n\nt = Test()\nmatch t:\n case Test():\n assert True\n case _:\n assert False\n\")" +expression: "compile_exec(r#\"\\\nclass Shape:\n pass\n\nclass Circle(Shape):\n def __init__(self, radius):\n self.radius = radius\n\nclass Rectangle(Shape):\n def __init__(self, width, height):\n self.width = width\n self.height = height\n\ndef describe_shape(shape):\n match shape:\n case Circle(radius=r):\n return f\"A circle with radius {r}\"\n case Rectangle(width=w, height=h):\n return f\"A rectangle {w} by {h}\"\n case _:\n return \"Unknown shape\"\n\n# Test it out\nshapes = [Circle(5), Rectangle(4, 6), \"not a shape\"]\nfor s in shapes:\n print(describe_shape(s))\n\"#)" --- - 2 0 LoadBuildClass - 1 LoadConst (): 1 0 LoadGlobal (0, __name__) + 3 0 LoadBuildClass + 1 LoadConst (): 2 0 LoadGlobal (0, __name__) 1 StoreLocal (1, __module__) - 2 LoadConst ("Test") + 2 LoadConst ("Shape") 3 StoreLocal (2, __qualname__) 4 LoadConst (None) 5 StoreLocal (3, __doc__) - 2 6 ReturnConst (None) + 3 6 ReturnConst (None) - 2 LoadConst ("Test") + 2 LoadConst ("Shape") 3 MakeFunction (MakeFunctionFlags(0x0)) - 4 LoadConst ("Test") + 4 LoadConst ("Shape") 5 CallFunctionPositional(2) - 6 StoreLocal (0, Test) - - 4 7 LoadNameAny (0, Test) - 8 CallFunctionPositional(0) - 9 StoreLocal (1, t) - - 5 10 LoadNameAny (1, t) - 11 CopyItem (1) - - 6 12 LoadNameAny (0, Test) - 13 LoadConst (()) - 14 MatchClass (0) - 15 CopyItem (1) - 16 LoadConst (None) - 17 TestOperation (IsNot) - 18 JumpIfFalse (27) - 19 UnpackSequence (0) - 20 Pop - - 7 21 LoadConst (True) - 22 JumpIfTrue (26) - 23 LoadGlobal (2, AssertionError) - 24 CallFunctionPositional(0) - 25 Raise (Raise) - >> 26 Jump (35) - >> 27 Pop - 28 Pop - - 9 29 LoadConst (False) - 30 JumpIfTrue (34) - 31 LoadGlobal (2, AssertionError) - 32 CallFunctionPositional(0) - 33 Raise (Raise) - >> 34 Jump (35) - >> 35 ReturnConst (None) + 6 StoreLocal (0, Shape) + + 7 7 LoadBuildClass + 8 LoadConst (): 5 0 LoadGlobal (0, __name__) + 1 StoreLocal (1, __module__) + 2 LoadConst ("Circle") + 3 StoreLocal (2, __qualname__) + 4 LoadConst (None) + 5 StoreLocal (3, __doc__) + + 7 6 LoadConst (): 7 0 LoadFast (1, radius) + 1 LoadFast (0, self) + 2 StoreAttr (0, radius) + 3 ReturnConst (None) + + 7 LoadConst ("Circle.__init__") + 8 MakeFunction (MakeFunctionFlags(0x0)) + 9 StoreLocal (4, __init__) + 10 ReturnConst (None) + + 9 LoadConst ("Circle") + 10 MakeFunction (MakeFunctionFlags(0x0)) + 11 LoadConst ("Circle") + + 5 12 LoadNameAny (0, Shape) + 13 CallFunctionPositional(3) + 14 StoreLocal (1, Circle) + + 12 15 LoadBuildClass + 16 LoadConst (): 9 0 LoadGlobal (0, __name__) + 1 StoreLocal (1, __module__) + 2 LoadConst ("Rectangle") + 3 StoreLocal (2, __qualname__) + 4 LoadConst (None) + 5 StoreLocal (3, __doc__) + + 12 6 LoadConst (): 11 0 LoadFast (1, width) + 1 LoadFast (0, self) + 2 StoreAttr (0, width) + + 12 3 LoadFast (2, height) + 4 LoadFast (0, self) + 5 StoreAttr (1, height) + 6 ReturnConst (None) + + 7 LoadConst ("Rectangle.__init__") + 8 MakeFunction (MakeFunctionFlags(0x0)) + 9 StoreLocal (4, __init__) + 10 ReturnConst (None) + + 17 LoadConst ("Rectangle") + 18 MakeFunction (MakeFunctionFlags(0x0)) + 19 LoadConst ("Rectangle") + + 9 20 LoadNameAny (0, Shape) + 21 CallFunctionPositional(3) + 22 StoreLocal (2, Rectangle) + + 21 23 LoadConst (): 15 0 LoadFast (0, shape) + 1 CopyItem (1) + + 16 2 LoadGlobal (0, Circle) + 3 LoadConst (("radius")) + 4 MatchClass (0) + 5 CopyItem (1) + 6 LoadConst (None) + 7 TestOperation (IsNot) + 8 JumpIfFalse (19) + 9 UnpackSequence (1) + 10 Pop + 11 Pop + + 17 12 LoadConst ("A circle with radius ") + 13 LoadConst ("") + 14 LoadFast (1, r) + 15 FormatValue (None) + 16 BuildString (2) + 17 ReturnValue + 18 Jump (47) + >> 19 Pop + 20 CopyItem (1) + + 18 21 LoadGlobal (1, Rectangle) + 22 LoadConst (("width", "height")) + 23 MatchClass (0) + 24 CopyItem (1) + 25 LoadConst (None) + 26 TestOperation (IsNot) + 27 JumpIfFalse (43) + 28 UnpackSequence (2) + 29 Pop + 30 Pop + 31 Pop + + 19 32 LoadConst ("A rectangle ") + 33 LoadConst ("") + 34 LoadFast (2, w) + 35 FormatValue (None) + 36 LoadConst (" by ") + 37 LoadConst ("") + 38 LoadFast (3, h) + 39 FormatValue (None) + 40 BuildString (4) + 41 ReturnValue + 42 Jump (47) + >> 43 Pop + 44 Pop + + 21 45 ReturnConst ("Unknown shape") + 46 Jump (47) + >> 47 ReturnConst (None) + + 24 LoadConst ("describe_shape") + 25 MakeFunction (MakeFunctionFlags(0x0)) + 26 StoreLocal (3, describe_shape) + + 24 27 LoadNameAny (1, Circle) + 28 LoadConst (5) + 29 CallFunctionPositional(1) + 30 LoadNameAny (2, Rectangle) + 31 LoadConst (4) + 32 LoadConst (6) + 33 CallFunctionPositional(2) + 34 LoadConst ("not a shape") + 35 BuildList (3) + 36 StoreLocal (4, shapes) + + 25 37 SetupLoop + 38 LoadNameAny (4, shapes) + 39 GetIter + >> 40 ForIter (49) + 41 StoreLocal (5, s) + + 26 42 LoadNameAny (6, print) + 43 LoadNameAny (3, describe_shape) + 44 LoadNameAny (5, s) + 45 CallFunctionPositional(1) + 46 CallFunctionPositional(1) + 47 Pop + 48 Jump (40) + >> 49 PopBlock + 50 ReturnConst (None) From d44324d4d0d522df8e6b77151b996c50bdd8d348 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 11:44:04 -0700 Subject: [PATCH 08/10] clippy --- compiler/codegen/src/compile.rs | 2 +- compiler/codegen/src/ir.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 286f318ccb..d32c3f2691 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -1948,7 +1948,7 @@ impl Compiler<'_> { // If no name is provided, simply pop the top of the stack. None => { emit!(self, Instruction::Pop); - return Ok(()); + Ok(()) } Some(name) => { // Check if the name is forbidden for storing. diff --git a/compiler/codegen/src/ir.rs b/compiler/codegen/src/ir.rs index 662ba81dbe..ee29c65a16 100644 --- a/compiler/codegen/src/ir.rs +++ b/compiler/codegen/src/ir.rs @@ -244,7 +244,7 @@ impl CodeInfo { let instr_display = instr.display(display_arg, self); eprint!("{instr_display}: {depth} {effect:+} => "); } - if effect < 0 && depth < effect.abs() as u32 { + if effect < 0 && depth < effect.unsigned_abs() { panic!("The stack will underflow at {depth} with {effect} effect on {instr:?}"); } let new_depth = depth.checked_add_signed(effect).unwrap(); From 21272025c29f17b640016c78faf640212e26a4c4 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 11:54:46 -0700 Subject: [PATCH 09/10] improve error handling --- compiler/codegen/src/compile.rs | 14 ++++++-------- compiler/codegen/src/error.rs | 8 ++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index d32c3f2691..0a355c4a40 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -2158,10 +2158,7 @@ impl Compiler<'_> { for ident in attrs.iter().take(n_attrs).skip(i + 1) { let other = ident.as_str(); if attr == other { - todo!(); - // return Err(self.compiler_error( - // &format!("attribute name repeated in class pattern: {}", attr), - // )); + return Err(self.error(CodegenErrorType::RepeatedAttributePattern)); } } } @@ -2415,15 +2412,16 @@ impl Compiler<'_> { } else { let control_vec = control.as_ref().unwrap(); if nstores != control_vec.len() { - todo!(); - // return self.compiler_error("alternative patterns bind different names"); + return Err(self.error(CodegenErrorType::ConflictingNameBindPattern)); } else if nstores > 0 { // Check that the names occur in the same order. for icontrol in (0..nstores).rev() { let name = &control_vec[icontrol]; // Find the index of `name` in the current stores. - let istores = pc.stores.iter().position(|n| n == name).unwrap(); - // .ok_or_else(|| self.compiler_error("alternative patterns bind different names"))?; + let istores = + pc.stores.iter().position(|n| n == name).ok_or_else(|| { + self.error(CodegenErrorType::ConflictingNameBindPattern) + })?; if icontrol != istores { // The orders differ; we must reorder. assert!(istores < icontrol, "expected istores < icontrol"); diff --git a/compiler/codegen/src/error.rs b/compiler/codegen/src/error.rs index b1b4f9379f..4394b936d2 100644 --- a/compiler/codegen/src/error.rs +++ b/compiler/codegen/src/error.rs @@ -65,6 +65,8 @@ pub enum CodegenErrorType { ForbiddenName, DuplicateStore(String), UnreachablePattern(PatternUnreachableReason), + RepeatedAttributePattern, + ConflictingNameBindPattern, NotImplementedYet, // RustPython marker for unimplemented features } @@ -119,6 +121,12 @@ impl fmt::Display for CodegenErrorType { UnreachablePattern(reason) => { write!(f, "{reason} makes remaining patterns unreachable") } + RepeatedAttributePattern => { + write!(f, "attribute name repeated in class pattern") + } + ConflictingNameBindPattern => { + write!(f, "alternative patterns bind different names") + } NotImplementedYet => { write!(f, "RustPython does not implement this feature yet") } From 4d53f5925cea220701312a1851783abb992f8c45 Mon Sep 17 00:00:00 2001 From: Ashwin Naren Date: Tue, 15 Apr 2025 11:56:29 -0700 Subject: [PATCH 10/10] remove match test --- compiler/codegen/src/compile.rs | 33 ---- ...python_codegen__compile__tests__match.snap | 160 ------------------ 2 files changed, 193 deletions(-) delete mode 100644 compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index 0a355c4a40..762585783c 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -4596,37 +4596,4 @@ for stop_exc in (StopIteration('spam'), StopAsyncIteration('ham')): " )); } - - #[test] - fn test_match() { - assert_dis_snapshot!(compile_exec( - r#"\ -class Shape: - pass - -class Circle(Shape): - def __init__(self, radius): - self.radius = radius - -class Rectangle(Shape): - def __init__(self, width, height): - self.width = width - self.height = height - -def describe_shape(shape): - match shape: - case Circle(radius=r): - return f"A circle with radius {r}" - case Rectangle(width=w, height=h): - return f"A rectangle {w} by {h}" - case _: - return "Unknown shape" - -# Test it out -shapes = [Circle(5), Rectangle(4, 6), "not a shape"] -for s in shapes: - print(describe_shape(s)) -"# - )); - } } diff --git a/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap b/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap deleted file mode 100644 index 379d42f18b..0000000000 --- a/compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap +++ /dev/null @@ -1,160 +0,0 @@ ---- -source: compiler/codegen/src/compile.rs -expression: "compile_exec(r#\"\\\nclass Shape:\n pass\n\nclass Circle(Shape):\n def __init__(self, radius):\n self.radius = radius\n\nclass Rectangle(Shape):\n def __init__(self, width, height):\n self.width = width\n self.height = height\n\ndef describe_shape(shape):\n match shape:\n case Circle(radius=r):\n return f\"A circle with radius {r}\"\n case Rectangle(width=w, height=h):\n return f\"A rectangle {w} by {h}\"\n case _:\n return \"Unknown shape\"\n\n# Test it out\nshapes = [Circle(5), Rectangle(4, 6), \"not a shape\"]\nfor s in shapes:\n print(describe_shape(s))\n\"#)" ---- - 3 0 LoadBuildClass - 1 LoadConst (): 2 0 LoadGlobal (0, __name__) - 1 StoreLocal (1, __module__) - 2 LoadConst ("Shape") - 3 StoreLocal (2, __qualname__) - 4 LoadConst (None) - 5 StoreLocal (3, __doc__) - - 3 6 ReturnConst (None) - - 2 LoadConst ("Shape") - 3 MakeFunction (MakeFunctionFlags(0x0)) - 4 LoadConst ("Shape") - 5 CallFunctionPositional(2) - 6 StoreLocal (0, Shape) - - 7 7 LoadBuildClass - 8 LoadConst (): 5 0 LoadGlobal (0, __name__) - 1 StoreLocal (1, __module__) - 2 LoadConst ("Circle") - 3 StoreLocal (2, __qualname__) - 4 LoadConst (None) - 5 StoreLocal (3, __doc__) - - 7 6 LoadConst (): 7 0 LoadFast (1, radius) - 1 LoadFast (0, self) - 2 StoreAttr (0, radius) - 3 ReturnConst (None) - - 7 LoadConst ("Circle.__init__") - 8 MakeFunction (MakeFunctionFlags(0x0)) - 9 StoreLocal (4, __init__) - 10 ReturnConst (None) - - 9 LoadConst ("Circle") - 10 MakeFunction (MakeFunctionFlags(0x0)) - 11 LoadConst ("Circle") - - 5 12 LoadNameAny (0, Shape) - 13 CallFunctionPositional(3) - 14 StoreLocal (1, Circle) - - 12 15 LoadBuildClass - 16 LoadConst (): 9 0 LoadGlobal (0, __name__) - 1 StoreLocal (1, __module__) - 2 LoadConst ("Rectangle") - 3 StoreLocal (2, __qualname__) - 4 LoadConst (None) - 5 StoreLocal (3, __doc__) - - 12 6 LoadConst (): 11 0 LoadFast (1, width) - 1 LoadFast (0, self) - 2 StoreAttr (0, width) - - 12 3 LoadFast (2, height) - 4 LoadFast (0, self) - 5 StoreAttr (1, height) - 6 ReturnConst (None) - - 7 LoadConst ("Rectangle.__init__") - 8 MakeFunction (MakeFunctionFlags(0x0)) - 9 StoreLocal (4, __init__) - 10 ReturnConst (None) - - 17 LoadConst ("Rectangle") - 18 MakeFunction (MakeFunctionFlags(0x0)) - 19 LoadConst ("Rectangle") - - 9 20 LoadNameAny (0, Shape) - 21 CallFunctionPositional(3) - 22 StoreLocal (2, Rectangle) - - 21 23 LoadConst (): 15 0 LoadFast (0, shape) - 1 CopyItem (1) - - 16 2 LoadGlobal (0, Circle) - 3 LoadConst (("radius")) - 4 MatchClass (0) - 5 CopyItem (1) - 6 LoadConst (None) - 7 TestOperation (IsNot) - 8 JumpIfFalse (19) - 9 UnpackSequence (1) - 10 Pop - 11 Pop - - 17 12 LoadConst ("A circle with radius ") - 13 LoadConst ("") - 14 LoadFast (1, r) - 15 FormatValue (None) - 16 BuildString (2) - 17 ReturnValue - 18 Jump (47) - >> 19 Pop - 20 CopyItem (1) - - 18 21 LoadGlobal (1, Rectangle) - 22 LoadConst (("width", "height")) - 23 MatchClass (0) - 24 CopyItem (1) - 25 LoadConst (None) - 26 TestOperation (IsNot) - 27 JumpIfFalse (43) - 28 UnpackSequence (2) - 29 Pop - 30 Pop - 31 Pop - - 19 32 LoadConst ("A rectangle ") - 33 LoadConst ("") - 34 LoadFast (2, w) - 35 FormatValue (None) - 36 LoadConst (" by ") - 37 LoadConst ("") - 38 LoadFast (3, h) - 39 FormatValue (None) - 40 BuildString (4) - 41 ReturnValue - 42 Jump (47) - >> 43 Pop - 44 Pop - - 21 45 ReturnConst ("Unknown shape") - 46 Jump (47) - >> 47 ReturnConst (None) - - 24 LoadConst ("describe_shape") - 25 MakeFunction (MakeFunctionFlags(0x0)) - 26 StoreLocal (3, describe_shape) - - 24 27 LoadNameAny (1, Circle) - 28 LoadConst (5) - 29 CallFunctionPositional(1) - 30 LoadNameAny (2, Rectangle) - 31 LoadConst (4) - 32 LoadConst (6) - 33 CallFunctionPositional(2) - 34 LoadConst ("not a shape") - 35 BuildList (3) - 36 StoreLocal (4, shapes) - - 25 37 SetupLoop - 38 LoadNameAny (4, shapes) - 39 GetIter - >> 40 ForIter (49) - 41 StoreLocal (5, s) - - 26 42 LoadNameAny (6, print) - 43 LoadNameAny (3, describe_shape) - 44 LoadNameAny (5, s) - 45 CallFunctionPositional(1) - 46 CallFunctionPositional(1) - 47 Pop - 48 Jump (40) - >> 49 PopBlock - 50 ReturnConst (None)