Skip to content

Commit ddc0849

Browse files
youknowoneyouknowone
authored
Fix match mapping pattern (#6081)
1 parent a9a9e3b commit ddc0849

File tree

4 files changed

+316
-149
lines changed

4 files changed

+316
-149
lines changed

‎compiler/codegen/src/compile.rs

Lines changed: 195 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ use ruff_python_ast::{
2626
ExprFString, ExprList, ExprName, ExprSlice, ExprStarred, ExprSubscript, ExprTuple, ExprUnaryOp,
2727
FString, FStringElement, FStringElements, FStringFlags, FStringPart, Identifier, Int, Keyword,
2828
MatchCase, ModExpression, ModModule, Operator, Parameters, Pattern, PatternMatchAs,
29-
PatternMatchClass, PatternMatchOr, PatternMatchSequence, PatternMatchSingleton,
30-
PatternMatchStar, PatternMatchValue, Singleton, Stmt, StmtExpr, TypeParam, TypeParamParamSpec,
31-
TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, UnaryOp, WithItem,
29+
PatternMatchClass, PatternMatchMapping, PatternMatchOr, PatternMatchSequence,
30+
PatternMatchSingleton, PatternMatchStar, PatternMatchValue, Singleton, Stmt, StmtExpr,
31+
TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, TypeParams, UnaryOp,
32+
WithItem,
3233
};
3334
use ruff_text_size::{Ranged, TextRange};
3435
use rustpython_compiler_core::{
@@ -3149,6 +3150,11 @@ impl Compiler {
31493150

31503151
/// Duplicate the effect of Python 3.10's ROT_* instructions using SWAPs.
31513152
fn pattern_helper_rotate(&mut self, mut count: usize) -> CompileResult<()> {
3153+
// Rotate TOS (top of stack) to position `count` down
3154+
// This is done by a series of swaps
3155+
// For count=1, no rotation needed (already at top)
3156+
// For count=2, swap TOS with item 1 position down
3157+
// For count=3, swap TOS with item 2 positions down, then with item 1 position down
31523158
while count > 1 {
31533159
// Emit a SWAP instruction with the current count.
31543160
emit!(
@@ -3442,8 +3448,6 @@ impl Compiler {
34423448
});
34433449
}
34443450

3445-
use bytecode::TestOperator::*;
3446-
34473451
// Emit instructions:
34483452
// 1. Load the new tuple of attribute names.
34493453
self.emit_load_const(ConstantData::Tuple {
@@ -3456,7 +3460,12 @@ impl Compiler {
34563460
// 4. Load None.
34573461
self.emit_load_const(ConstantData::None);
34583462
// 5. Compare with IS_OP 1.
3459-
emit!(self, Instruction::TestOperation { op: IsNot });
3463+
emit!(
3464+
self,
3465+
Instruction::TestOperation {
3466+
op: bytecode::TestOperator::IsNot
3467+
}
3468+
);
34603469

34613470
// At this point the TOS is a tuple of (nargs + n_attrs) attributes (or None).
34623471
pc.on_top += 1;
@@ -3487,123 +3496,183 @@ impl Compiler {
34873496
Ok(())
34883497
}
34893498

3490-
// fn compile_pattern_mapping(&mut self, p: &PatternMatchMapping, pc: &mut PatternContext) -> CompileResult<()> {
3491-
// // Ensure the pattern is a mapping pattern.
3492-
// let mapping = p; // Extract MatchMapping-specific data.
3493-
// let keys = &mapping.keys;
3494-
// let patterns = &mapping.patterns;
3495-
// let size = keys.len();
3496-
// let n_patterns = patterns.len();
3497-
3498-
// if size != n_patterns {
3499-
// panic!("keys ({}) / patterns ({}) length mismatch in mapping pattern", size, n_patterns);
3500-
// // return self.compiler_error(
3501-
// // &format!("keys ({}) / patterns ({}) length mismatch in mapping pattern", size, n_patterns)
3502-
// // );
3503-
// }
3504-
3505-
// // A double-star target is present if `rest` is set.
3506-
// let star_target = mapping.rest;
3507-
3508-
// // Keep the subject on top during the mapping and length checks.
3509-
// pc.on_top += 1;
3510-
// emit!(self, Instruction::MatchMapping);
3511-
// self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
3512-
3513-
// // If the pattern is just "{}" (empty mapping) and there's no star target,
3514-
// // we're done—pop the subject.
3515-
// if size == 0 && star_target.is_none() {
3516-
// pc.on_top -= 1;
3517-
// emit!(self, Instruction::Pop);
3518-
// return Ok(());
3519-
// }
3520-
3521-
// // If there are any keys, perform a length check.
3522-
// if size != 0 {
3523-
// emit!(self, Instruction::GetLen);
3524-
// self.emit_load_const(ConstantData::Integer { value: size.into() });
3525-
// emit!(self, Instruction::CompareOperation { op: ComparisonOperator::GreaterOrEqual });
3526-
// self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
3527-
// }
3528-
3529-
// // Check that the number of sub-patterns is not absurd.
3530-
// if size.saturating_sub(1) > (i32::MAX as usize) {
3531-
// panic!("too many sub-patterns in mapping pattern");
3532-
// // return self.compiler_error("too many sub-patterns in mapping pattern");
3533-
// }
3534-
3535-
// // Collect all keys into a set for duplicate checking.
3536-
// let mut seen = HashSet::new();
3537-
3538-
// // For each key, validate it and check for duplicates.
3539-
// for (i, key) in keys.iter().enumerate() {
3540-
// if let Some(key_val) = key.as_literal_expr() {
3541-
// let in_seen = seen.contains(&key_val);
3542-
// if in_seen {
3543-
// panic!("mapping pattern checks duplicate key: {:?}", key_val);
3544-
// // return self.compiler_error(format!("mapping pattern checks duplicate key: {:?}", key_val));
3545-
// }
3546-
// seen.insert(key_val);
3547-
// } else if !key.is_attribute_expr() {
3548-
// panic!("mapping pattern keys may only match literals and attribute lookups");
3549-
// // return self.compiler_error("mapping pattern keys may only match literals and attribute lookups");
3550-
// }
3551-
3552-
// // Visit the key expression.
3553-
// self.compile_expression(key)?;
3554-
// }
3555-
// // Drop the set (its resources will be freed automatically).
3556-
3557-
// // Build a tuple of keys and emit MATCH_KEYS.
3558-
// emit!(self, Instruction::BuildTuple { size: size as u32 });
3559-
// emit!(self, Instruction::MatchKeys);
3560-
// // Now, on top of the subject there are two new tuples: one of keys and one of values.
3561-
// pc.on_top += 2;
3562-
3563-
// // Prepare for matching the values.
3564-
// emit!(self, Instruction::CopyItem { index: 1_u32 });
3565-
// self.emit_load_const(ConstantData::None);
3566-
// // TODO: should be is
3567-
// emit!(self, Instruction::TestOperation::IsNot);
3568-
// self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
3569-
3570-
// // Unpack the tuple of values.
3571-
// emit!(self, Instruction::UnpackSequence { size: size as u32 });
3572-
// pc.on_top += size.saturating_sub(1);
3573-
3574-
// // Compile each subpattern in "subpattern" mode.
3575-
// for pattern in patterns {
3576-
// pc.on_top = pc.on_top.saturating_sub(1);
3577-
// self.compile_pattern_subpattern(pattern, pc)?;
3578-
// }
3579-
3580-
// // Consume the tuple of keys and the subject.
3581-
// pc.on_top = pc.on_top.saturating_sub(2);
3582-
// if let Some(star_target) = star_target {
3583-
// // If we have a starred name, bind a dict of remaining items to it.
3584-
// // This sequence of instructions performs:
3585-
// // rest = dict(subject)
3586-
// // for key in keys: del rest[key]
3587-
// emit!(self, Instruction::BuildMap { size: 0 }); // Build an empty dict.
3588-
// emit!(self, Instruction::Swap(3)); // Rearrange stack: [empty, keys, subject]
3589-
// emit!(self, Instruction::DictUpdate { size: 2 }); // Update dict with subject.
3590-
// emit!(self, Instruction::UnpackSequence { size: size as u32 }); // Unpack keys.
3591-
// let mut remaining = size;
3592-
// while remaining > 0 {
3593-
// emit!(self, Instruction::CopyItem { index: 1 + remaining as u32 }); // Duplicate subject copy.
3594-
// emit!(self, Instruction::Swap { index: 2_u32 }); // Bring key to top.
3595-
// emit!(self, Instruction::DeleteSubscript); // Delete key from dict.
3596-
// remaining -= 1;
3597-
// }
3598-
// // Bind the dict to the starred target.
3599-
// self.pattern_helper_store_name(Some(&star_target), pc)?;
3600-
// } else {
3601-
// // No starred target: just pop the tuple of keys and the subject.
3602-
// emit!(self, Instruction::Pop);
3603-
// emit!(self, Instruction::Pop);
3604-
// }
3605-
// Ok(())
3606-
// }
3499+
fn compile_pattern_mapping(
3500+
&mut self,
3501+
p: &PatternMatchMapping,
3502+
pc: &mut PatternContext,
3503+
) -> CompileResult<()> {
3504+
let mapping = p;
3505+
let keys = &mapping.keys;
3506+
let patterns = &mapping.patterns;
3507+
let size = keys.len();
3508+
let n_patterns = patterns.len();
3509+
3510+
if size != n_patterns {
3511+
return Err(self.error(CodegenErrorType::SyntaxError(format!(
3512+
"keys ({size}) / patterns ({n_patterns}) length mismatch in mapping pattern"
3513+
))));
3514+
}
3515+
3516+
let star_target = &mapping.rest;
3517+
3518+
// Step 1: Check if subject is a mapping
3519+
// Stack: [subject]
3520+
// We need to keep the subject on top during the mapping and length checks
3521+
pc.on_top += 1;
3522+
3523+
emit!(self, Instruction::MatchMapping);
3524+
// Stack: [subject, bool]
3525+
3526+
// If not a mapping, fail (jump and pop subject)
3527+
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
3528+
// Stack: [subject] (on success)
3529+
3530+
// Step 2: If empty pattern with no star, consume subject
3531+
if size == 0 && star_target.is_none() {
3532+
// If the pattern is just "{}", we're done! Pop the subject:
3533+
pc.on_top -= 1;
3534+
emit!(self, Instruction::Pop);
3535+
return Ok(());
3536+
}
3537+
3538+
// Step 3: Check mapping has enough keys
3539+
if size != 0 {
3540+
// Stack: [subject]
3541+
emit!(self, Instruction::GetLen);
3542+
// Stack: [subject, len]
3543+
self.emit_load_const(ConstantData::Integer { value: size.into() });
3544+
// Stack: [subject, len, size]
3545+
emit!(
3546+
self,
3547+
Instruction::CompareOperation {
3548+
op: ComparisonOperator::GreaterOrEqual
3549+
}
3550+
);
3551+
// Stack: [subject, bool]
3552+
3553+
// If not enough keys, fail
3554+
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
3555+
// Stack: [subject]
3556+
}
3557+
3558+
// Step 4: Build keys tuple
3559+
if size.saturating_sub(1) > (i32::MAX as usize) {
3560+
return Err(self.error(CodegenErrorType::SyntaxError(
3561+
"too many sub-patterns in mapping pattern".to_string(),
3562+
)));
3563+
}
3564+
3565+
// Validate and compile keys
3566+
// NOTE: RustPython difference - using HashSet<String> for duplicate checking
3567+
// CPython uses PySet with actual Python objects
3568+
let mut seen = std::collections::HashSet::new();
3569+
3570+
for key in keys.iter() {
3571+
// Validate key
3572+
let is_constant = matches!(
3573+
key,
3574+
Expr::NumberLiteral(_)
3575+
| Expr::StringLiteral(_)
3576+
| Expr::BytesLiteral(_)
3577+
| Expr::BooleanLiteral(_)
3578+
| Expr::NoneLiteral(_)
3579+
);
3580+
let is_attribute = matches!(key, Expr::Attribute(_));
3581+
3582+
if is_constant {
3583+
let key_repr = format!("{key:?}");
3584+
if seen.contains(&key_repr) {
3585+
return Err(self.error(CodegenErrorType::SyntaxError(format!(
3586+
"mapping pattern checks duplicate key: {key_repr:?}"
3587+
))));
3588+
}
3589+
seen.insert(key_repr);
3590+
} else if !is_attribute {
3591+
return Err(self.error(CodegenErrorType::SyntaxError(
3592+
"mapping pattern keys may only match literals and attribute lookups"
3593+
.to_string(),
3594+
)));
3595+
}
3596+
3597+
// Compile key expression
3598+
self.compile_expression(key)?;
3599+
}
3600+
// Stack: [subject, key1, key2, ...]
3601+
3602+
// Build tuple of keys
3603+
emit!(
3604+
self,
3605+
Instruction::BuildTuple {
3606+
size: u32::try_from(size).expect("too many keys in mapping pattern")
3607+
}
3608+
);
3609+
// Stack: [subject, keys_tuple]
3610+
3611+
// Step 5: Extract values using MATCH_KEYS
3612+
emit!(self, Instruction::MatchKeys);
3613+
// Stack: [subject, keys_or_none, values_or_none]
3614+
// There's now a tuple of keys and a tuple of values on top of the subject
3615+
pc.on_top += 2;
3616+
3617+
emit!(self, Instruction::CopyItem { index: 1_u32 }); // Copy values_or_none (TOS)
3618+
// Stack: [subject, keys_or_none, values_or_none, values_or_none]
3619+
3620+
self.emit_load_const(ConstantData::None);
3621+
// Stack: [subject, keys_or_none, values_or_none, values_or_none, None]
3622+
3623+
emit!(
3624+
self,
3625+
Instruction::TestOperation {
3626+
op: bytecode::TestOperator::IsNot
3627+
}
3628+
);
3629+
// Stack: [subject, keys_or_none, values_or_none, bool]
3630+
3631+
// If values_or_none is None, fail (need to clean up 3 items: values_or_none, keys_or_none, subject)
3632+
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
3633+
// Stack: [subject, keys_or_none, values_or_none] (on success)
3634+
3635+
// Step 7: Process patterns
3636+
if size > 0 {
3637+
// Unpack values tuple
3638+
emit!(
3639+
self,
3640+
Instruction::UnpackSequence {
3641+
size: u32::try_from(size).expect("too many values in mapping pattern")
3642+
}
3643+
);
3644+
// Stack: [subject, keys_or_none, value_n, ..., value_1]
3645+
// After UNPACK_SEQUENCE, we have size values on the stack
3646+
pc.on_top += size - 1;
3647+
3648+
// Process each pattern with compile_pattern_subpattern
3649+
for pattern in patterns.iter() {
3650+
pc.on_top -= 1;
3651+
self.compile_pattern_subpattern(pattern, pc)?;
3652+
}
3653+
}
3654+
3655+
// After all patterns processed, adjust on_top for subject and keys_or_none
3656+
pc.on_top -= 2;
3657+
3658+
// Step 8: Clean up
3659+
// If we get this far, it's a match! Whatever happens next should consume
3660+
// the tuple of keys and the subject
3661+
3662+
if let Some(_star_target) = star_target {
3663+
// TODO: Implement **rest pattern support
3664+
// This would involve BUILD_MAP, DICT_UPDATE, etc.
3665+
return Err(self.error(CodegenErrorType::SyntaxError(
3666+
"**rest pattern in mapping not yet implemented".to_string(),
3667+
)));
3668+
} else {
3669+
// Pop the tuple of keys
3670+
emit!(self, Instruction::Pop);
3671+
// Pop the subject
3672+
emit!(self, Instruction::Pop);
3673+
}
3674+
Ok(())
3675+
}
36073676

36083677
fn compile_pattern_or(
36093678
&mut self,
@@ -3848,7 +3917,9 @@ impl Compiler {
38483917
Pattern::MatchSequence(pattern_type) => {
38493918
self.compile_pattern_sequence(pattern_type, pattern_context)
38503919
}
3851-
// Pattern::MatchMapping(pattern_type) => self.compile_pattern_mapping(pattern_type, pattern_context),
3920+
Pattern::MatchMapping(pattern_type) => {
3921+
self.compile_pattern_mapping(pattern_type, pattern_context)
3922+
}
38523923
Pattern::MatchClass(pattern_type) => {
38533924
self.compile_pattern_class(pattern_type, pattern_context)
38543925
}
@@ -3861,11 +3932,6 @@ impl Compiler {
38613932
Pattern::MatchOr(pattern_type) => {
38623933
self.compile_pattern_or(pattern_type, pattern_context)
38633934
}
3864-
_ => {
3865-
// The eprintln gives context as to which pattern type is not implemented.
3866-
eprintln!("not implemented pattern type: {pattern_type:?}");
3867-
Err(self.error(CodegenErrorType::NotImplementedYet))
3868-
}
38693935
}
38703936
}
38713937

‎compiler/core/src/bytecode.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,8 +1425,8 @@ impl Instruction {
14251425
GetAIter => 0,
14261426
GetANext => 1,
14271427
EndAsyncFor => -2,
1428-
MatchMapping | MatchSequence => 0,
1429-
MatchKeys => -1,
1428+
MatchMapping | MatchSequence => 1, // Push bool result
1429+
MatchKeys => 1, // Pop 2 (subject, keys), push 3 (subject, keys_or_none, values_or_none)
14301430
MatchClass(_) => -2,
14311431
ExtendedArg => 0,
14321432
}

0 commit comments

Comments
 (0)