Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0d4faa0

Browse filesBrowse files
coolreader18coolreader18
authored
Merge pull request #5700 from arihant2math/match-cleanup
Cleanup match statement codegen
2 parents fd665f6 + 4d53f59 commit 0d4faa0
Copy full SHA for 0d4faa0

File tree

6 files changed

+56
-143
lines changed
Filter options

6 files changed

+56
-143
lines changed

‎compiler/codegen/src/compile.rs

Copy file name to clipboardExpand all lines: compiler/codegen/src/compile.rs
+44-76Lines changed: 44 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,32 +1944,35 @@ impl Compiler<'_> {
19441944
n: Option<&Identifier>,
19451945
pc: &mut PatternContext,
19461946
) -> CompileResult<()> {
1947-
// If no name is provided, simply pop the top of the stack.
1948-
if n.is_none() {
1949-
emit!(self, Instruction::Pop);
1950-
return Ok(());
1951-
}
1952-
let name = n.unwrap();
1953-
1954-
// Check if the name is forbidden for storing.
1955-
if self.forbidden_name(name.as_str(), NameUsage::Store)? {
1956-
return Err(self.compile_error_forbidden_name(name.as_str()));
1957-
}
1947+
match n {
1948+
// If no name is provided, simply pop the top of the stack.
1949+
None => {
1950+
emit!(self, Instruction::Pop);
1951+
Ok(())
1952+
}
1953+
Some(name) => {
1954+
// Check if the name is forbidden for storing.
1955+
if self.forbidden_name(name.as_str(), NameUsage::Store)? {
1956+
return Err(self.compile_error_forbidden_name(name.as_str()));
1957+
}
19581958

1959-
// Ensure we don't store the same name twice.
1960-
if pc.stores.contains(&name.to_string()) {
1961-
return Err(self.error(CodegenErrorType::DuplicateStore(name.as_str().to_string())));
1962-
}
1959+
// Ensure we don't store the same name twice.
1960+
// TODO: maybe pc.stores should be a set?
1961+
if pc.stores.contains(&name.to_string()) {
1962+
return Err(
1963+
self.error(CodegenErrorType::DuplicateStore(name.as_str().to_string()))
1964+
);
1965+
}
19631966

1964-
// Calculate how many items to rotate:
1965-
// the count is the number of items to preserve on top plus the current stored names,
1966-
// plus one for the new value.
1967-
let rotations = pc.on_top + pc.stores.len() + 1;
1968-
self.pattern_helper_rotate(rotations)?;
1967+
// Calculate how many items to rotate:
1968+
let rotations = pc.on_top + pc.stores.len() + 1;
1969+
self.pattern_helper_rotate(rotations)?;
19691970

1970-
// Append the name to the captured stores.
1971-
pc.stores.push(name.to_string());
1972-
Ok(())
1971+
// Append the name to the captured stores.
1972+
pc.stores.push(name.to_string());
1973+
Ok(())
1974+
}
1975+
}
19731976
}
19741977

19751978
fn pattern_unpack_helper(&mut self, elts: &[Pattern]) -> CompileResult<()> {
@@ -2155,10 +2158,7 @@ impl Compiler<'_> {
21552158
for ident in attrs.iter().take(n_attrs).skip(i + 1) {
21562159
let other = ident.as_str();
21572160
if attr == other {
2158-
todo!();
2159-
// return Err(self.compiler_error(
2160-
// &format!("attribute name repeated in class pattern: {}", attr),
2161-
// ));
2161+
return Err(self.error(CodegenErrorType::RepeatedAttributePattern));
21622162
}
21632163
}
21642164
}
@@ -2185,16 +2185,6 @@ impl Compiler<'_> {
21852185

21862186
let nargs = patterns.len();
21872187
let n_attrs = kwd_attrs.len();
2188-
let nkwd_patterns = kwd_patterns.len();
2189-
2190-
// Validate that keyword attribute names and patterns match in length.
2191-
if n_attrs != nkwd_patterns {
2192-
let msg = format!(
2193-
"kwd_attrs ({}) / kwd_patterns ({}) length mismatch in class pattern",
2194-
n_attrs, nkwd_patterns
2195-
);
2196-
unreachable!("{}", msg);
2197-
}
21982188

21992189
// Check for too many sub-patterns.
22002190
if nargs > u32::MAX as usize || (nargs + n_attrs).saturating_sub(1) > i32::MAX as usize {
@@ -2223,6 +2213,8 @@ impl Compiler<'_> {
22232213
});
22242214
}
22252215

2216+
use bytecode::TestOperator::*;
2217+
22262218
// Emit instructions:
22272219
// 1. Load the new tuple of attribute names.
22282220
self.emit_load_const(ConstantData::Tuple {
@@ -2235,7 +2227,7 @@ impl Compiler<'_> {
22352227
// 4. Load None.
22362228
self.emit_load_const(ConstantData::None);
22372229
// 5. Compare with IS_OP 1.
2238-
emit!(self, Instruction::IsOperation(true));
2230+
emit!(self, Instruction::TestOperation { op: IsNot });
22392231

22402232
// At this point the TOS is a tuple of (nargs + n_attrs) attributes (or None).
22412233
pc.on_top += 1;
@@ -2253,20 +2245,12 @@ impl Compiler<'_> {
22532245
pc.on_top -= 1;
22542246

22552247
// Process each sub-pattern.
2256-
for i in 0..total {
2257-
// Decrement the on_top counter as each sub-pattern is processed.
2248+
for subpattern in patterns.iter().chain(kwd_patterns.iter()) {
2249+
// Decrement the on_top counter as each sub-pattern is processed
2250+
// (on_top should be zero at the end of the algorithm as a sanity check).
22582251
pc.on_top -= 1;
2259-
let subpattern = if i < nargs {
2260-
// Positional sub-pattern.
2261-
&patterns[i]
2262-
} else {
2263-
// Keyword sub-pattern.
2264-
&kwd_patterns[i - nargs]
2265-
};
22662252
if subpattern.is_wildcard() {
2267-
// For wildcard patterns, simply pop the top of the stack.
22682253
emit!(self, Instruction::Pop);
2269-
continue;
22702254
}
22712255
// Compile the subpattern without irrefutability checks.
22722256
self.compile_pattern_subpattern(subpattern, pc)?;
@@ -2351,7 +2335,7 @@ impl Compiler<'_> {
23512335
// emit!(self, Instruction::CopyItem { index: 1_u32 });
23522336
// self.emit_load_const(ConstantData::None);
23532337
// // TODO: should be is
2354-
// emit!(self, Instruction::IsOperation(true));
2338+
// emit!(self, Instruction::TestOperation::IsNot);
23552339
// self.jump_to_fail_pop(pc, JumpOp::PopJumpIfFalse)?;
23562340

23572341
// // Unpack the tuple of values.
@@ -2428,15 +2412,16 @@ impl Compiler<'_> {
24282412
} else {
24292413
let control_vec = control.as_ref().unwrap();
24302414
if nstores != control_vec.len() {
2431-
todo!();
2432-
// return self.compiler_error("alternative patterns bind different names");
2415+
return Err(self.error(CodegenErrorType::ConflictingNameBindPattern));
24332416
} else if nstores > 0 {
24342417
// Check that the names occur in the same order.
24352418
for icontrol in (0..nstores).rev() {
24362419
let name = &control_vec[icontrol];
24372420
// Find the index of `name` in the current stores.
2438-
let istores = pc.stores.iter().position(|n| n == name).unwrap();
2439-
// .ok_or_else(|| self.compiler_error("alternative patterns bind different names"))?;
2421+
let istores =
2422+
pc.stores.iter().position(|n| n == name).ok_or_else(|| {
2423+
self.error(CodegenErrorType::ConflictingNameBindPattern)
2424+
})?;
24402425
if icontrol != istores {
24412426
// The orders differ; we must reorder.
24422427
assert!(istores < icontrol, "expected istores < icontrol");
@@ -2480,14 +2465,14 @@ impl Compiler<'_> {
24802465
self.switch_to_block(end);
24812466

24822467
// Adjust the final captures.
2483-
let nstores = control.as_ref().unwrap().len();
2484-
let nrots = nstores + 1 + pc.on_top + pc.stores.len();
2485-
for i in 0..nstores {
2468+
let n_stores = control.as_ref().unwrap().len();
2469+
let n_rots = n_stores + 1 + pc.on_top + pc.stores.len();
2470+
for i in 0..n_stores {
24862471
// Rotate the capture to its proper place.
2487-
self.pattern_helper_rotate(nrots)?;
2472+
self.pattern_helper_rotate(n_rots)?;
24882473
let name = &control.as_ref().unwrap()[i];
24892474
// Check for duplicate binding.
2490-
if pc.stores.iter().any(|n| n == name) {
2475+
if pc.stores.contains(name) {
24912476
return Err(self.error(CodegenErrorType::DuplicateStore(name.to_string())));
24922477
}
24932478
pc.stores.push(name.clone());
@@ -4608,23 +4593,6 @@ for stop_exc in (StopIteration('spam'), StopAsyncIteration('ham')):
46084593
self.assertIs(ex, stop_exc)
46094594
else:
46104595
self.fail(f'{stop_exc} was suppressed')
4611-
"
4612-
));
4613-
}
4614-
4615-
#[test]
4616-
fn test_match() {
4617-
assert_dis_snapshot!(compile_exec(
4618-
"\
4619-
class Test:
4620-
pass
4621-
4622-
t = Test()
4623-
match t:
4624-
case Test():
4625-
assert True
4626-
case _:
4627-
assert False
46284596
"
46294597
));
46304598
}

‎compiler/codegen/src/error.rs

Copy file name to clipboardExpand all lines: compiler/codegen/src/error.rs
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ pub enum CodegenErrorType {
6565
ForbiddenName,
6666
DuplicateStore(String),
6767
UnreachablePattern(PatternUnreachableReason),
68+
RepeatedAttributePattern,
69+
ConflictingNameBindPattern,
6870
NotImplementedYet, // RustPython marker for unimplemented features
6971
}
7072

@@ -119,6 +121,12 @@ impl fmt::Display for CodegenErrorType {
119121
UnreachablePattern(reason) => {
120122
write!(f, "{reason} makes remaining patterns unreachable")
121123
}
124+
RepeatedAttributePattern => {
125+
write!(f, "attribute name repeated in class pattern")
126+
}
127+
ConflictingNameBindPattern => {
128+
write!(f, "alternative patterns bind different names")
129+
}
122130
NotImplementedYet => {
123131
write!(f, "RustPython does not implement this feature yet")
124132
}

‎compiler/codegen/src/ir.rs

Copy file name to clipboardExpand all lines: compiler/codegen/src/ir.rs
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,9 @@ impl CodeInfo {
244244
let instr_display = instr.display(display_arg, self);
245245
eprint!("{instr_display}: {depth} {effect:+} => ");
246246
}
247+
if effect < 0 && depth < effect.unsigned_abs() {
248+
panic!("The stack will underflow at {depth} with {effect} effect on {instr:?}");
249+
}
247250
let new_depth = depth.checked_add_signed(effect).unwrap();
248251
if DEBUG {
249252
eprintln!("{new_depth}");

‎compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap

Copy file name to clipboardExpand all lines: compiler/codegen/src/snapshots/rustpython_codegen__compile__tests__match.snap
-53Lines changed: 0 additions & 53 deletions
This file was deleted.

‎compiler/core/src/bytecode.rs

Copy file name to clipboardExpand all lines: compiler/core/src/bytecode.rs
+1-6Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,6 @@ pub enum Instruction {
437437
TestOperation {
438438
op: Arg<TestOperator>,
439439
},
440-
/// If the argument is true, perform IS NOT. Otherwise perform the IS operation.
441-
// TODO: duplication of TestOperator::{Is,IsNot}. Fix later.
442-
IsOperation(Arg<bool>),
443440
CompareOperation {
444441
op: Arg<ComparisonOperator>,
445442
},
@@ -1227,8 +1224,7 @@ impl Instruction {
12271224
BinaryOperation { .. }
12281225
| BinaryOperationInplace { .. }
12291226
| TestOperation { .. }
1230-
| CompareOperation { .. }
1231-
| IsOperation(..) => -1,
1227+
| CompareOperation { .. } => -1,
12321228
BinarySubscript => -1,
12331229
CopyItem { .. } => 1,
12341230
Pop => -1,
@@ -1436,7 +1432,6 @@ impl Instruction {
14361432
BinarySubscript => w!(BinarySubscript),
14371433
LoadAttr { idx } => w!(LoadAttr, name = idx),
14381434
TestOperation { op } => w!(TestOperation, ?op),
1439-
IsOperation(neg) => w!(IsOperation, neg),
14401435
CompareOperation { op } => w!(CompareOperation, ?op),
14411436
CopyItem { index } => w!(CopyItem, index),
14421437
Pop => w!(Pop),

‎vm/src/frame.rs

Copy file name to clipboardExpand all lines: vm/src/frame.rs
-8Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -851,14 +851,6 @@ impl ExecutingFrame<'_> {
851851
bytecode::Instruction::UnaryOperation { op } => self.execute_unary_op(vm, op.get(arg)),
852852
bytecode::Instruction::TestOperation { op } => self.execute_test(vm, op.get(arg)),
853853
bytecode::Instruction::CompareOperation { op } => self.execute_compare(vm, op.get(arg)),
854-
bytecode::Instruction::IsOperation(neg) => {
855-
let a = self.pop_value();
856-
let b = self.pop_value();
857-
// xor with neg to invert the result if needed
858-
let result = vm.ctx.new_bool(a.is(b.as_ref()) ^ neg.get(arg));
859-
self.push_value(result.into());
860-
Ok(None)
861-
}
862854
bytecode::Instruction::ReturnValue => {
863855
let value = self.pop_value();
864856
self.unwind_blocks(vm, UnwindReason::Returning { value })

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.