Skip to content

Cleanup match statement codegen #5700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
coolreader18 merged 10 commits into from
Apr 18, 2025
Merged

Cleanup match statement codegen #5700

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
49b348c
Remove Instruction::IsOperation
arihant2math Apr 15, 2025
c7042fd
remove unneeded validation
arihant2math Apr 15, 2025
456e555
better error
arihant2math Apr 15, 2025
d47944b
error handling
arihant2math Apr 15, 2025
09c199a
match cleanup
arihant2math Apr 15, 2025
e949c9a
rename
arihant2math Apr 15, 2025
628287c
update snapshot
arihant2math Apr 15, 2025
d44324d
clippy
arihant2math Apr 15, 2025
2127202
improve error handling
arihant2math Apr 15, 2025
4d53f59
remove match test
arihant2math Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
match cleanup
  • Loading branch information
@arihant2math
arihant2math committed Apr 15, 2025
commit 09c199a1bab2a292fe4370fcde255d89dbefef51
99 changes: 55 additions & 44 deletions compiler/codegen/src/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(());
}
Comment on lines +1947 to +1952
Copy link
Copy Markdown
Member

@coolreader18 coolreader18 Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also use let-else, to prevent rightward drift.

        let Some(name) = n else {
            // If no name is provided, simply pop the top of the stack.
            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<()> {
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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))
"#
));
}
}
Loading