diff --git a/compiler/codegen/src/compile.rs b/compiler/codegen/src/compile.rs index bd66c0c3d6..d386c9c030 100644 --- a/compiler/codegen/src/compile.rs +++ b/compiler/codegen/src/compile.rs @@ -26,9 +26,10 @@ use ruff_python_ast::{ ExprFString, ExprList, ExprName, ExprSlice, ExprStarred, ExprSubscript, ExprTuple, ExprUnaryOp, FString, FStringElement, FStringElements, FStringFlags, FStringPart, Identifier, Int, Keyword, MatchCase, ModExpression, ModModule, Operator, Parameters, Pattern, PatternMatchAs, - PatternMatchClass, PatternMatchOr, PatternMatchSequence, PatternMatchSingleton, - PatternMatchStar, PatternMatchValue, Singleton, Stmt, StmtExpr, TypeParam, TypeParamParamSpec, - TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, UnaryOp, WithItem, + PatternMatchClass, PatternMatchMapping, PatternMatchOr, PatternMatchSequence, + PatternMatchSingleton, PatternMatchStar, PatternMatchValue, Singleton, Stmt, StmtExpr, + TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, UnaryOp, + WithItem, }; use ruff_text_size::{Ranged, TextRange}; use rustpython_compiler_core::{ @@ -3149,6 +3150,11 @@ impl Compiler { /// Duplicate the effect of Python 3.10's ROT_* instructions using SWAPs. fn pattern_helper_rotate(&mut self, mut count: usize) -> CompileResult<()> { + // Rotate TOS (top of stack) to position `count` down + // This is done by a series of swaps + // For count=1, no rotation needed (already at top) + // For count=2, swap TOS with item 1 position down + // For count=3, swap TOS with item 2 positions down, then with item 1 position down while count > 1 { // Emit a SWAP instruction with the current count. emit!( @@ -3442,8 +3448,6 @@ impl Compiler { }); } - use bytecode::TestOperator::*; - // Emit instructions: // 1. Load the new tuple of attribute names. self.emit_load_const(ConstantData::Tuple { @@ -3456,7 +3460,12 @@ impl Compiler { // 4. Load None. self.emit_load_const(ConstantData::None); // 5. Compare with IS_OP 1. - emit!(self, Instruction::TestOperation { op: IsNot }); + emit!( + self, + Instruction::TestOperation { + op: bytecode::TestOperator::IsNot + } + ); // At this point the TOS is a tuple of (nargs + n_attrs) attributes (or None). pc.on_top += 1; @@ -3487,123 +3496,183 @@ impl Compiler { Ok(()) } - // fn compile_pattern_mapping(&mut self, p: &PatternMatchMapping, pc: &mut PatternContext) -> CompileResult<()> { - // // Ensure the pattern is a mapping pattern. - // let mapping = p; // Extract MatchMapping-specific data. - // let keys = &mapping.keys; - // let patterns = &mapping.patterns; - // let size = keys.len(); - // let n_patterns = patterns.len(); - - // if size != n_patterns { - // panic!("keys ({}) / patterns ({}) length mismatch in mapping pattern", size, n_patterns); - // // return self.compiler_error( - // // &format!("keys ({}) / patterns ({}) length mismatch in mapping pattern", size, n_patterns) - // // ); - // } - - // // A double-star target is present if `rest` is set. - // let star_target = mapping.rest; - - // // Keep the subject on top during the mapping and length checks. - // pc.on_top += 1; - // emit!(self, Instruction::MatchMapping); - // self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; - - // // If the pattern is just "{}" (empty mapping) and there's no star target, - // // we're done—pop the subject. - // if size == 0 && star_target.is_none() { - // pc.on_top -= 1; - // emit!(self, Instruction::Pop); - // return Ok(()); - // } - - // // If there are any keys, perform a length check. - // if size != 0 { - // emit!(self, Instruction::GetLen); - // self.emit_load_const(ConstantData::Integer { value: size.into() }); - // emit!(self, Instruction::CompareOperation { op: ComparisonOperator::GreaterOrEqual }); - // self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; - // } - - // // Check that the number of sub-patterns is not absurd. - // if size.saturating_sub(1) > (i32::MAX as usize) { - // panic!("too many sub-patterns in mapping pattern"); - // // return self.compiler_error("too many sub-patterns in mapping pattern"); - // } - - // // Collect all keys into a set for duplicate checking. - // let mut seen = HashSet::new(); - - // // For each key, validate it and check for duplicates. - // for (i, key) in keys.iter().enumerate() { - // if let Some(key_val) = key.as_literal_expr() { - // let in_seen = seen.contains(&key_val); - // if in_seen { - // panic!("mapping pattern checks duplicate key: {:?}", key_val); - // // return self.compiler_error(format!("mapping pattern checks duplicate key: {:?}", key_val)); - // } - // seen.insert(key_val); - // } else if !key.is_attribute_expr() { - // panic!("mapping pattern keys may only match literals and attribute lookups"); - // // return self.compiler_error("mapping pattern keys may only match literals and attribute lookups"); - // } - - // // Visit the key expression. - // self.compile_expression(key)?; - // } - // // Drop the set (its resources will be freed automatically). - - // // Build a tuple of keys and emit MATCH_KEYS. - // emit!(self, Instruction::BuildTuple { size: size as u32 }); - // emit!(self, Instruction::MatchKeys); - // // Now, on top of the subject there are two new tuples: one of keys and one of values. - // pc.on_top += 2; - - // // Prepare for matching the values. - // emit!(self, Instruction::CopyItem { index: 1_u32 }); - // self.emit_load_const(ConstantData::None); - // // TODO: should be is - // emit!(self, Instruction::TestOperation::IsNot); - // self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; - - // // Unpack the tuple of values. - // emit!(self, Instruction::UnpackSequence { size: size as u32 }); - // pc.on_top += size.saturating_sub(1); - - // // Compile each subpattern in "subpattern" mode. - // for pattern in patterns { - // pc.on_top = pc.on_top.saturating_sub(1); - // self.compile_pattern_subpattern(pattern, pc)?; - // } - - // // Consume the tuple of keys and the subject. - // pc.on_top = pc.on_top.saturating_sub(2); - // if let Some(star_target) = star_target { - // // If we have a starred name, bind a dict of remaining items to it. - // // This sequence of instructions performs: - // // rest = dict(subject) - // // for key in keys: del rest[key] - // emit!(self, Instruction::BuildMap { size: 0 }); // Build an empty dict. - // emit!(self, Instruction::Swap(3)); // Rearrange stack: [empty, keys, subject] - // emit!(self, Instruction::DictUpdate { size: 2 }); // Update dict with subject. - // emit!(self, Instruction::UnpackSequence { size: size as u32 }); // Unpack keys. - // let mut remaining = size; - // while remaining > 0 { - // emit!(self, Instruction::CopyItem { index: 1 + remaining as u32 }); // Duplicate subject copy. - // emit!(self, Instruction::Swap { index: 2_u32 }); // Bring key to top. - // emit!(self, Instruction::DeleteSubscript); // Delete key from dict. - // remaining -= 1; - // } - // // Bind the dict to the starred target. - // self.pattern_helper_store_name(Some(&star_target), pc)?; - // } else { - // // No starred target: just pop the tuple of keys and the subject. - // emit!(self, Instruction::Pop); - // emit!(self, Instruction::Pop); - // } - // Ok(()) - // } + fn compile_pattern_mapping( + &mut self, + p: &PatternMatchMapping, + pc: &mut PatternContext, + ) -> CompileResult<()> { + let mapping = p; + let keys = &mapping.keys; + let patterns = &mapping.patterns; + let size = keys.len(); + let n_patterns = patterns.len(); + + if size != n_patterns { + return Err(self.error(CodegenErrorType::SyntaxError(format!( + "keys ({size}) / patterns ({n_patterns}) length mismatch in mapping pattern" + )))); + } + + let star_target = &mapping.rest; + + // Step 1: Check if subject is a mapping + // Stack: [subject] + // We need to keep the subject on top during the mapping and length checks + pc.on_top += 1; + + emit!(self, Instruction::MatchMapping); + // Stack: [subject, bool] + + // If not a mapping, fail (jump and pop subject) + self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; + // Stack: [subject] (on success) + + // Step 2: If empty pattern with no star, consume subject + if size == 0 && star_target.is_none() { + // If the pattern is just "{}", we're done! Pop the subject: + pc.on_top -= 1; + emit!(self, Instruction::Pop); + return Ok(()); + } + + // Step 3: Check mapping has enough keys + if size != 0 { + // Stack: [subject] + emit!(self, Instruction::GetLen); + // Stack: [subject, len] + self.emit_load_const(ConstantData::Integer { value: size.into() }); + // Stack: [subject, len, size] + emit!( + self, + Instruction::CompareOperation { + op: ComparisonOperator::GreaterOrEqual + } + ); + // Stack: [subject, bool] + + // If not enough keys, fail + self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; + // Stack: [subject] + } + + // Step 4: Build keys tuple + if size.saturating_sub(1) > (i32::MAX as usize) { + return Err(self.error(CodegenErrorType::SyntaxError( + "too many sub-patterns in mapping pattern".to_string(), + ))); + } + + // Validate and compile keys + // NOTE: RustPython difference - using HashSet for duplicate checking + // CPython uses PySet with actual Python objects + let mut seen = std::collections::HashSet::new(); + + for key in keys.iter() { + // Validate key + let is_constant = matches!( + key, + Expr::NumberLiteral(_) + | Expr::StringLiteral(_) + | Expr::BytesLiteral(_) + | Expr::BooleanLiteral(_) + | Expr::NoneLiteral(_) + ); + let is_attribute = matches!(key, Expr::Attribute(_)); + + if is_constant { + let key_repr = format!("{key:?}"); + if seen.contains(&key_repr) { + return Err(self.error(CodegenErrorType::SyntaxError(format!( + "mapping pattern checks duplicate key: {key_repr:?}" + )))); + } + seen.insert(key_repr); + } else if !is_attribute { + return Err(self.error(CodegenErrorType::SyntaxError( + "mapping pattern keys may only match literals and attribute lookups" + .to_string(), + ))); + } + + // Compile key expression + self.compile_expression(key)?; + } + // Stack: [subject, key1, key2, ...] + + // Build tuple of keys + emit!( + self, + Instruction::BuildTuple { + size: u32::try_from(size).expect("too many keys in mapping pattern") + } + ); + // Stack: [subject, keys_tuple] + + // Step 5: Extract values using MATCH_KEYS + emit!(self, Instruction::MatchKeys); + // Stack: [subject, keys_or_none, values_or_none] + // There's now a tuple of keys and a tuple of values on top of the subject + pc.on_top += 2; + + emit!(self, Instruction::CopyItem { index: 1_u32 }); // Copy values_or_none (TOS) + // Stack: [subject, keys_or_none, values_or_none, values_or_none] + + self.emit_load_const(ConstantData::None); + // Stack: [subject, keys_or_none, values_or_none, values_or_none, None] + + emit!( + self, + Instruction::TestOperation { + op: bytecode::TestOperator::IsNot + } + ); + // Stack: [subject, keys_or_none, values_or_none, bool] + + // If values_or_none is None, fail (need to clean up 3 items: values_or_none, keys_or_none, subject) + self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?; + // Stack: [subject, keys_or_none, values_or_none] (on success) + + // Step 7: Process patterns + if size > 0 { + // Unpack values tuple + emit!( + self, + Instruction::UnpackSequence { + size: u32::try_from(size).expect("too many values in mapping pattern") + } + ); + // Stack: [subject, keys_or_none, value_n, ..., value_1] + // After UNPACK_SEQUENCE, we have size values on the stack + pc.on_top += size - 1; + + // Process each pattern with compile_pattern_subpattern + for pattern in patterns.iter() { + pc.on_top -= 1; + self.compile_pattern_subpattern(pattern, pc)?; + } + } + + // After all patterns processed, adjust on_top for subject and keys_or_none + pc.on_top -= 2; + + // Step 8: Clean up + // If we get this far, it's a match! Whatever happens next should consume + // the tuple of keys and the subject + + if let Some(_star_target) = star_target { + // TODO: Implement **rest pattern support + // This would involve BUILD_MAP, DICT_UPDATE, etc. + return Err(self.error(CodegenErrorType::SyntaxError( + "**rest pattern in mapping not yet implemented".to_string(), + ))); + } else { + // Pop the tuple of keys + emit!(self, Instruction::Pop); + // Pop the subject + emit!(self, Instruction::Pop); + } + Ok(()) + } fn compile_pattern_or( &mut self, @@ -3848,7 +3917,9 @@ impl Compiler { Pattern::MatchSequence(pattern_type) => { self.compile_pattern_sequence(pattern_type, pattern_context) } - // Pattern::MatchMapping(pattern_type) => self.compile_pattern_mapping(pattern_type, pattern_context), + Pattern::MatchMapping(pattern_type) => { + self.compile_pattern_mapping(pattern_type, pattern_context) + } Pattern::MatchClass(pattern_type) => { self.compile_pattern_class(pattern_type, pattern_context) } @@ -3861,11 +3932,6 @@ impl Compiler { Pattern::MatchOr(pattern_type) => { self.compile_pattern_or(pattern_type, pattern_context) } - _ => { - // The eprintln gives context as to which pattern type is not implemented. - eprintln!("not implemented pattern type: {pattern_type:?}"); - Err(self.error(CodegenErrorType::NotImplementedYet)) - } } } diff --git a/compiler/core/src/bytecode.rs b/compiler/core/src/bytecode.rs index 8e8104234f..206c10d888 100644 --- a/compiler/core/src/bytecode.rs +++ b/compiler/core/src/bytecode.rs @@ -1423,8 +1423,8 @@ impl Instruction { GetAIter => 0, GetANext => 1, EndAsyncFor => -2, - MatchMapping | MatchSequence => 0, - MatchKeys => -1, + MatchMapping | MatchSequence => 1, // Push bool result + MatchKeys => 1, // Pop 2 (subject, keys), push 3 (subject, keys_or_none, values_or_none) MatchClass(_) => -2, ExtendedArg => 0, } diff --git a/extra_tests/snippets/syntax_match.py b/extra_tests/snippets/syntax_match.py index 4e6cd0a962..be1a94eadb 100644 --- a/extra_tests/snippets/syntax_match.py +++ b/extra_tests/snippets/syntax_match.py @@ -12,11 +12,13 @@ # Test enum from enum import Enum + class Color(Enum): RED = 1 GREEN = 2 BLUE = 3 + def test_color(color): z = -1 match color: @@ -28,9 +30,11 @@ def test_color(color): z = 3 assert z == color.value + for color in Color: test_color(color) + # test or def test_or(i): z = -1 @@ -43,8 +47,78 @@ def test_or(i): z = 2 return z + assert test_or(0) == 0 assert test_or(1) == 0 assert test_or(2) == 1 assert test_or(3) == 1 assert test_or(4) == 2 + +# test mapping +data = {"a": 1, "b": 2} +match data: + case {"a": x}: + assert x == 1 + case _: + assert False + +match data: + case {"a": x, "b": y}: + assert x == 1 + assert y == 2 + case _: + assert False + +# test mapping with rest (TODO: implement **rest pattern) +# match data: +# case {"a": x, **rest}: +# assert x == 1 +# assert rest == {"b": 2} +# case _: +# assert False + +# test mapping pattern with wildcard fallback (reproduces wheelinfo.py issue) +test_dict = {"sha256": "abc123"} +result = None +match test_dict: + case {"sha256": checksum}: + result = checksum + case _: + result = "no checksum" +assert result == "abc123" + +# test with no match +test_dict2 = {"md5": "xyz789"} +match test_dict2: + case {"sha256": checksum}: + result = checksum + case _: + result = "no checksum" +assert result == "no checksum" + + +# test mapping patterns - comprehensive tests +def test_mapping_comprehensive(): + # Single key capture + data = {"a": 1} + match data: + case {"a": x}: + captured = x + case _: + captured = None + assert captured == 1, f"Expected 1, got {captured}" + + # Multiple keys + data = {"a": 1, "b": 2} + match data: + case {"a": x, "b": y}: + cap_x = x + cap_y = y + case _: + cap_x = cap_y = None + assert cap_x == 1, f"Expected x=1, got {cap_x}" + assert cap_y == 2, f"Expected y=2, got {cap_y}" + print("Comprehensive mapping tests passed!") + + +test_mapping_comprehensive() diff --git a/vm/src/frame.rs b/vm/src/frame.rs index de8cb84352..513b8fb646 100644 --- a/vm/src/frame.rs +++ b/vm/src/frame.rs @@ -680,11 +680,15 @@ impl ExecutingFrame<'_> { bytecode::Instruction::StoreSubscript => self.execute_store_subscript(vm), bytecode::Instruction::DeleteSubscript => self.execute_delete_subscript(vm), bytecode::Instruction::CopyItem { index } => { + // CopyItem { index: 1 } copies TOS + // CopyItem { index: 2 } copies second from top + // This is 1-indexed to match CPython + let idx = index.get(arg) as usize; let value = self .state .stack .len() - .checked_sub(index.get(arg) as usize) + .checked_sub(idx) .map(|i| &self.state.stack[i]) .unwrap(); self.push_value(value.clone()); @@ -699,8 +703,8 @@ impl ExecutingFrame<'_> { let len = self.state.stack.len(); let i = len - 1; // TOS index let index_val = index.get(arg) as usize; - // SWAP(i) swaps TOS with element i positions down from TOS - // So the target index is len - index_val + // CPython: SWAP(n) swaps TOS with PEEK(n) where PEEK(n) = stack_pointer[-n] + // This means swap TOS with the element at index (len - n) let j = len - index_val; self.state.stack.swap(i, j); Ok(None) @@ -1268,36 +1272,59 @@ impl ExecutingFrame<'_> { Ok(None) } bytecode::Instruction::MatchMapping => { - // Pop the subject from stack + // Pop and push back the subject to keep it on stack let subject = self.pop_value(); - - // Decide if it's a mapping, push True/False or handle error let is_mapping = PyMapping::check(&subject); + self.push_value(subject); self.push_value(vm.ctx.new_bool(is_mapping).into()); Ok(None) } bytecode::Instruction::MatchSequence => { - // Pop the subject from stack + // Pop and push back the subject to keep it on stack let subject = self.pop_value(); - - // Decide if it's a sequence (but not a mapping) let is_sequence = subject.to_sequence().check(); + self.push_value(subject); self.push_value(vm.ctx.new_bool(is_sequence).into()); Ok(None) } bytecode::Instruction::MatchKeys => { - // Typically we pop a sequence of keys first - let _keys = self.pop_value(); + // Pop keys tuple and subject + let keys_tuple = self.pop_value(); let subject = self.pop_value(); - // Check if subject is a dict (or mapping) and all keys match - if let Ok(_dict) = subject.downcast::() { - // Example: gather the values corresponding to keys - // If keys match, push the matched values & success - self.push_value(vm.ctx.new_bool(true).into()); + // Push the subject back first + self.push_value(subject.clone()); + + // Check if subject is a mapping and extract values for keys + if PyMapping::check(&subject) { + let keys = keys_tuple.downcast_ref::().unwrap(); + let mut values = Vec::new(); + let mut all_match = true; + + for key in keys { + match subject.get_item(key.as_object(), vm) { + Ok(value) => values.push(value), + Err(_) => { + all_match = false; + break; + } + } + } + + if all_match { + // Push keys_or_none (the original keys) and values_or_none + // Keys should remain as they were for potential **rest handling + self.push_value(keys_tuple); // keys_or_none + self.push_value(vm.ctx.new_tuple(values).into()); // values_or_none + } else { + // No match - push None twice + self.push_value(vm.ctx.none()); + self.push_value(vm.ctx.none()); + } } else { - // Push a placeholder to indicate no match - self.push_value(vm.ctx.new_bool(false).into()); + // Not a mapping - push None twice + self.push_value(vm.ctx.none()); + self.push_value(vm.ctx.none()); } Ok(None) }