Skip to content

Commit 1241a6e

Browse files
committed
Simplify implementation
1 parent ffc36df commit 1241a6e

File tree

2 files changed

+32
-142
lines changed

2 files changed

+32
-142
lines changed

compiler/codegen/src/compile.rs

Lines changed: 29 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -3517,24 +3517,21 @@ impl Compiler {
35173517

35183518
// Step 1: Check if subject is a mapping
35193519
// Stack: [subject]
3520-
// pc.on_top = 0 initially
3520+
// We need to keep the subject on top during the mapping and length checks
3521+
pc.on_top += 1;
35213522

35223523
emit!(self, Instruction::MatchMapping);
35233524
// Stack: [subject, bool]
35243525

3525-
// The bool will be consumed by PopJumpIfFalse
3526-
// Don't change pc.on_top because the bool is temporary
3527-
35283526
// If not a mapping, fail (jump and pop subject)
35293527
self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
35303528
// Stack: [subject] (on success)
3531-
// pc.on_top remains 0
35323529

35333530
// Step 2: If empty pattern with no star, consume subject
35343531
if size == 0 && star_target.is_none() {
3535-
// Pattern matches, consume subject
3532+
// If the pattern is just "{}", we're done! Pop the subject:
3533+
pc.on_top -= 1;
35363534
emit!(self, Instruction::Pop);
3537-
pc.on_top = 0;
35383535
return Ok(());
35393536
}
35403537

@@ -3566,8 +3563,9 @@ impl Compiler {
35663563
}
35673564

35683565
// Validate and compile keys
3569-
use std::collections::HashSet;
3570-
let mut seen = HashSet::new();
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();
35713569

35723570
for key in keys.iter() {
35733571
// Validate key
@@ -3613,13 +3611,8 @@ impl Compiler {
36133611
// Step 5: Extract values using MATCH_KEYS
36143612
emit!(self, Instruction::MatchKeys);
36153613
// Stack: [subject, keys_or_none, values_or_none]
3616-
// Note: MatchKeys pops subject and keys_tuple, then pushes back subject, keys_or_none, values_or_none
3617-
3618-
// Step 6: Check if match succeeded (values_or_none is not None)
3619-
// We need to keep track of what's on the stack for cleanup
3620-
// Stack: [subject, keys_or_none, values_or_none]
3621-
// On failure, we need to clean up keys_or_none and values_or_none but keep subject
3622-
pc.on_top = 2; // keys_or_none and values_or_none are temporary
3614+
// There's now a tuple of keys and a tuple of values on top of the subject
3615+
pc.on_top += 2;
36233616

36243617
emit!(self, Instruction::CopyItem { index: 1_u32 }); // Copy values_or_none (TOS)
36253618
// Stack: [subject, keys_or_none, values_or_none, values_or_none]
@@ -3649,138 +3642,35 @@ impl Compiler {
36493642
}
36503643
);
36513644
// Stack: [subject, keys_or_none, value_n, ..., value_1]
3652-
// UnpackSequence pushes in reverse order, so value_1 is on top
3653-
3654-
// For single capture pattern, use simplified approach
3655-
if size == 1 {
3656-
if let Pattern::MatchAs(p) = &patterns[0] {
3657-
if p.pattern.is_none() && p.name.is_some() {
3658-
// Simple capture: rearrange stack
3659-
// Current: [subject, keys_or_none, value1]
3660-
// Target: [value1] (after cleanup)
3661-
3662-
// SWAP 3: swap value1 with subject
3663-
emit!(self, Instruction::Swap { index: 3 });
3664-
// Stack: [value1, keys_or_none, subject]
3665-
3666-
// Pop keys_or_none
3667-
emit!(self, Instruction::Pop);
3668-
// Stack: [value1, subject]
3669-
3670-
// Pop subject
3671-
emit!(self, Instruction::Pop);
3672-
// Stack: [value1]
3673-
3674-
// Record the capture for compile_match_inner
3675-
pc.stores.push(p.name.as_ref().unwrap().to_string());
3676-
pc.on_top = 0;
3677-
return Ok(());
3678-
}
3679-
}
3680-
}
3681-
3682-
// For multiple captures, handle them directly like CPython
3683-
// UnpackSequence pushed values in reverse: [subject, keys_or_none, v_n, ..., v_1]
3684-
// We need to rearrange to get [v_1, v_2, ..., v_n] for storage
3685-
3686-
// First, check if all patterns are simple captures
3687-
let all_simple_captures = patterns.iter().all(
3688-
|p| matches!(p, Pattern::MatchAs(m) if m.pattern.is_none() && m.name.is_some()),
3689-
);
3690-
3691-
if all_simple_captures {
3692-
// Use direct SWAP sequences to rearrange stack
3693-
// Goal: move values to correct positions and clean up subject/keys
3694-
3695-
// Current: [subject, keys_or_none, v_n, ..., v_1]
3696-
// We need: [v_1, v_2, ..., v_n]
3697-
3698-
// Complex SWAP sequence for 2 values (most common case)
3699-
if size == 2 {
3700-
// [subject, keys_or_none, v2, v1]
3701-
// We need [v1, v2] at the end
3702-
3703-
// After UnpackSequence: [subject, keys_or_none, v2, v1]
3704-
// v1 = values[0] = 1, v2 = values[1] = 2
3705-
// Goal: [v1, v2] for storing to x, y
3706-
3707-
// Apply exact SWAP sequence
3708-
// Current after UNPACK_SEQUENCE 2: [subject, keys_or_none, v2, v1]
3709-
// Target: [v2, v1] so that x=v1, y=v2 when stored
3710-
3711-
// Apply SWAP sequence
3712-
emit!(self, Instruction::Swap { index: 2 }); // v1 <-> v2
3713-
// [subject, keys_or_none, v1, v2]
3714-
3715-
emit!(self, Instruction::Swap { index: 4 }); // v2 <-> subject
3716-
// [v2, keys_or_none, v1, subject]
3717-
3718-
emit!(self, Instruction::Swap { index: 2 }); // subject <-> v1
3719-
// [v2, keys_or_none, subject, v1]
3720-
3721-
emit!(self, Instruction::Swap { index: 3 }); // v1 <-> keys_or_none
3722-
// [v2, v1, subject, keys_or_none]
3723-
3724-
// Step 3: Clean up
3725-
emit!(self, Instruction::Pop); // remove keys_or_none
3726-
emit!(self, Instruction::Pop); // remove subject
3727-
// [v2, v1]
3645+
// After UNPACK_SEQUENCE, we have size values on the stack
3646+
pc.on_top += size - 1;
37283647

3729-
// Record captures in order
3730-
for pattern in patterns.iter() {
3731-
if let Pattern::MatchAs(p) = pattern {
3732-
if let Some(name) = &p.name {
3733-
pc.stores.push(name.to_string());
3734-
}
3735-
}
3736-
}
3737-
pc.on_top = 0;
3738-
return Ok(());
3739-
}
3740-
3741-
// For other sizes, fall back to general approach
3742-
}
3743-
3744-
// General approach using pattern_helper_store_name
3648+
// Process each pattern with compile_pattern_subpattern
37453649
for pattern in patterns.iter() {
3746-
// Calculate position correctly
3747-
// After UnpackSequence: values are in reverse order
3748-
// pattern[0] should match v1 (which is at top after unpack)
3749-
// pattern[1] should match v2 (which is below v1)
3750-
3751-
// Current value is at top, others below
3752-
pc.on_top = 0; // Value to process is at top
3753-
3754-
// Process this pattern
3650+
pc.on_top -= 1;
37553651
self.compile_pattern_subpattern(pattern, pc)?;
37563652
}
3757-
3758-
// After all patterns processed
3759-
// Stack: [subject, keys_or_none] plus any captured values rotated to bottom
37603653
}
37613654

3762-
// Step 8: Clean up
3763-
// After pattern processing:
3764-
// - If patterns captured values, they've been rotated to bottom of stack
3765-
// - Stack: [captured..., subject, keys_or_none]
3766-
// We need to clean up keys_or_none and subject
3767-
3768-
// Pop keys_or_none
3769-
emit!(self, Instruction::Pop);
3770-
// Stack: [captured..., subject]
3655+
// After all patterns processed, adjust on_top for subject and keys_or_none
3656+
pc.on_top -= 2;
37713657

3772-
// Pop subject (it will be consumed by this pattern match)
3773-
emit!(self, Instruction::Pop);
3774-
// Stack: [captured...]
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
37753661

3776-
if let Some(star_target) = star_target {
3777-
// subject is on top of stack
3778-
pc.on_top = 0;
3779-
self.pattern_helper_store_name(Some(star_target), pc)?;
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);
37803673
}
3781-
3782-
// Final state: only captured values on stack
3783-
pc.on_top = 0;
37843674
Ok(())
37853675
}
37863676

vm/src/frame.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -680,8 +680,8 @@ impl ExecutingFrame<'_> {
680680
bytecode::Instruction::StoreSubscript => self.execute_store_subscript(vm),
681681
bytecode::Instruction::DeleteSubscript => self.execute_delete_subscript(vm),
682682
bytecode::Instruction::CopyItem { index } => {
683-
// CopyItem { index: 1 } copies TOS (like Python's COPY 1)
684-
// CopyItem { index: 2 } copies second from top (like Python's COPY 2)
683+
// CopyItem { index: 1 } copies TOS
684+
// CopyItem { index: 2 } copies second from top
685685
// This is 1-indexed to match CPython
686686
let idx = index.get(arg) as usize;
687687
let value = self
@@ -705,7 +705,7 @@ impl ExecutingFrame<'_> {
705705
let index_val = index.get(arg) as usize;
706706
// CPython: SWAP(n) swaps TOS with PEEK(n) where PEEK(n) = stack_pointer[-n]
707707
// This means swap TOS with the element at index (len - n)
708-
let j = len.saturating_sub(index_val);
708+
let j = len - index_val;
709709
self.state.stack.swap(i, j);
710710
Ok(None)
711711
}

0 commit comments

Comments
 (0)