Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .cspell.dict/python-more.txt
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ readbuffer
reconstructor
refcnt
releaselevel
reraised
reverseitemiterator
reverseiterator
reversekeyiterator
Expand Down
6 changes: 0 additions & 6 deletions Lib/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1536,8 +1536,6 @@ def test_try_except_as(self):
"""
self.check_stack_size(snippet)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_try_except_star_qualified(self):
snippet = """
try:
Expand All @@ -1549,8 +1547,6 @@ def test_try_except_star_qualified(self):
"""
self.check_stack_size(snippet)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_try_except_star_as(self):
snippet = """
try:
Expand All @@ -1562,8 +1558,6 @@ def test_try_except_star_as(self):
"""
self.check_stack_size(snippet)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_try_except_star_finally(self):
snippet = """
try:
Expand Down
157 changes: 150 additions & 7 deletions crates/codegen/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,7 @@ impl Compiler {
..
}) => {
if *is_star {
self.compile_try_star_statement(body, handlers, orelse, finalbody)?
self.compile_try_star_except(body, handlers, orelse, finalbody)?
} else {
self.compile_try_statement(body, handlers, orelse, finalbody)?
}
Expand Down Expand Up @@ -2119,14 +2119,157 @@ impl Compiler {
Ok(())
}

fn compile_try_star_statement(
fn compile_try_star_except(
&mut self,
_body: &[Stmt],
_handlers: &[ExceptHandler],
_orelse: &[Stmt],
_finalbody: &[Stmt],
body: &[Stmt],
handlers: &[ExceptHandler],
orelse: &[Stmt],
finalbody: &[Stmt],
) -> CompileResult<()> {
Err(self.error(CodegenErrorType::NotImplementedYet))
// Simplified except* implementation using CheckEgMatch
let handler_block = self.new_block();
let finally_block = self.new_block();

if !finalbody.is_empty() {
emit!(
self,
Instruction::SetupFinally {
handler: finally_block,
}
);
}

let else_block = self.new_block();

emit!(
self,
Instruction::SetupExcept {
handler: handler_block,
}
);
self.compile_statements(body)?;
emit!(self, Instruction::PopBlock);
emit!(self, Instruction::Jump { target: else_block });

self.switch_to_block(handler_block);
// Stack: [exc]

for handler in handlers {
let ExceptHandler::ExceptHandler(ExceptHandlerExceptHandler {
type_, name, body, ..
}) = handler;

let skip_block = self.new_block();
let next_block = self.new_block();

// Compile exception type
if let Some(exc_type) = type_ {
// Check for unparenthesized tuple (e.g., `except* A, B:` instead of `except* (A, B):`)
if let Expr::Tuple(ExprTuple { elts, range, .. }) = exc_type.as_ref()
&& let Some(first) = elts.first()
&& range.start().to_u32() == first.range().start().to_u32()
{
return Err(self.error(CodegenErrorType::SyntaxError(
"multiple exception types must be parenthesized".to_owned(),
)));
}
self.compile_expression(exc_type)?;
} else {
return Err(self.error(CodegenErrorType::SyntaxError(
"except* must specify an exception type".to_owned(),
)));
}
// Stack: [exc, type]

emit!(self, Instruction::CheckEgMatch);
// Stack: [rest, match]

// Check if match is None (truthy check)
emit!(self, Instruction::CopyItem { index: 1 });
emit!(self, Instruction::ToBool);
emit!(self, Instruction::PopJumpIfFalse { target: skip_block });

// Handler matched - store match to name if provided
// Stack: [rest, match]
if let Some(alias) = name {
self.store_name(alias.as_str())?;
} else {
emit!(self, Instruction::PopTop);
}
// Stack: [rest]

self.compile_statements(body)?;

if let Some(alias) = name {
self.emit_load_const(ConstantData::None);
self.store_name(alias.as_str())?;
self.compile_name(alias.as_str(), NameUsage::Delete)?;
}

emit!(self, Instruction::Jump { target: next_block });

// No match - pop match (None) and continue with rest
self.switch_to_block(skip_block);
emit!(self, Instruction::PopTop); // drop match (None)
// Stack: [rest]

self.switch_to_block(next_block);
// Stack: [rest] - continue with rest for next handler
}

let handled_block = self.new_block();

// Check if remainder is truthy (has unhandled exceptions)
// Stack: [rest]
emit!(self, Instruction::CopyItem { index: 1 });
emit!(self, Instruction::ToBool);
emit!(
self,
Instruction::PopJumpIfFalse {
target: handled_block
}
);
// Reraise unhandled exceptions
emit!(
self,
Instruction::Raise {
kind: bytecode::RaiseKind::Raise
}
);

// All exceptions handled
self.switch_to_block(handled_block);
emit!(self, Instruction::PopTop); // drop remainder (None)
emit!(self, Instruction::PopException);

if !finalbody.is_empty() {
emit!(self, Instruction::PopBlock);
emit!(self, Instruction::EnterFinally);
}

emit!(
self,
Instruction::Jump {
target: finally_block,
}
);

// try-else path
self.switch_to_block(else_block);
self.compile_statements(orelse)?;

if !finalbody.is_empty() {
emit!(self, Instruction::PopBlock);
emit!(self, Instruction::EnterFinally);
}

self.switch_to_block(finally_block);
if !finalbody.is_empty() {
self.compile_statements(finalbody)?;
emit!(self, Instruction::EndFinally);
}

Ok(())
}

fn is_forbidden_arg_name(name: &str) -> bool {
Expand Down
7 changes: 6 additions & 1 deletion crates/compiler-core/src/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ op_arg_enum!(
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[repr(u8)]
pub enum IntrinsicFunction2 {
// PrepReraiseS tar = 1,
PrepReraiseStar = 1,
TypeVarWithBound = 2,
TypeVarWithConstraint = 3,
SetFunctionTypeParams = 4,
Expand Down Expand Up @@ -652,6 +652,9 @@ pub enum Instruction {
CallMethodPositional {
nargs: Arg<u32>,
},
/// Check if exception matches except* handler type.
/// Pops exc_value and match_type, pushes (rest, match).
CheckEgMatch,
CompareOperation {
op: Arg<ComparisonOperator>,
},
Expand Down Expand Up @@ -1721,6 +1724,7 @@ impl Instruction {
CallMethodKeyword { nargs } => -1 - (nargs.get(arg) as i32) - 3 + 1,
CallFunctionEx { has_kwargs } => -1 - (has_kwargs.get(arg) as i32) - 1 + 1,
CallMethodEx { has_kwargs } => -1 - (has_kwargs.get(arg) as i32) - 3 + 1,
CheckEgMatch => 0, // pops 2 (exc, type), pushes 2 (rest, match)
ConvertValue { .. } => 0,
FormatSimple => 0,
FormatWithSpec => -1,
Expand Down Expand Up @@ -1887,6 +1891,7 @@ impl Instruction {
CallMethodEx { has_kwargs } => w!(CALL_METHOD_EX, has_kwargs),
CallMethodKeyword { nargs } => w!(CALL_METHOD_KEYWORD, nargs),
CallMethodPositional { nargs } => w!(CALL_METHOD_POSITIONAL, nargs),
CheckEgMatch => w!(CHECK_EG_MATCH),
CompareOperation { op } => w!(COMPARE_OPERATION, ?op),
ContainsOp(inv) => w!(CONTAINS_OP, ?inv),
Continue { target } => w!(CONTINUE, target),
Expand Down
124 changes: 124 additions & 0 deletions crates/vm/src/exceptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2432,3 +2432,127 @@ pub(super) mod types {
#[repr(transparent)]
pub struct PyEncodingWarning(PyWarning);
}

/// Match exception against except* handler type.
/// Returns (rest, match) tuple.
pub fn exception_group_match(
exc_value: &PyObjectRef,
match_type: &PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<(PyObjectRef, PyObjectRef)> {
// Implements _PyEval_ExceptionGroupMatch

// If exc_value is None, return (None, None)
if vm.is_none(exc_value) {
return Ok((vm.ctx.none(), vm.ctx.none()));
}

// Check if exc_value matches match_type
if exc_value.is_instance(match_type, vm)? {
// Full match of exc itself
let is_eg = exc_value.fast_isinstance(vm.ctx.exceptions.base_exception_group);
let matched = if is_eg {
exc_value.clone()
} else {
// Naked exception - wrap it in ExceptionGroup
let excs = vm.ctx.new_tuple(vec![exc_value.clone()]);
let eg_type: PyObjectRef = crate::exception_group::exception_group().to_owned().into();
let wrapped = eg_type.call((vm.ctx.new_str(""), excs), vm)?;
// Copy traceback from original exception
if let Ok(exc) = exc_value.clone().downcast::<types::PyBaseException>()
&& let Some(tb) = exc.__traceback__()
&& let Ok(wrapped_exc) = wrapped.clone().downcast::<types::PyBaseException>()
{
let _ = wrapped_exc.set___traceback__(tb.into(), vm);
}
wrapped
};
return Ok((vm.ctx.none(), matched));
}

// Check for partial match if it's an exception group
if exc_value.fast_isinstance(vm.ctx.exceptions.base_exception_group) {
let pair = vm.call_method(exc_value, "split", (match_type.clone(),))?;
let pair_tuple: PyTupleRef = pair.try_into_value(vm)?;
if pair_tuple.len() < 2 {
return Err(vm.new_type_error(format!(
"{}.split must return a 2-tuple, got tuple of size {}",
exc_value.class().name(),
pair_tuple.len()
)));
}
let matched = pair_tuple[0].clone();
let rest = pair_tuple[1].clone();
return Ok((rest, matched));
}

// No match
Ok((exc_value.clone(), vm.ctx.none()))
}

/// Prepare exception for reraise in except* block.
pub fn prep_reraise_star(orig: PyObjectRef, excs: PyObjectRef, vm: &VirtualMachine) -> PyResult {
// Implements _PyExc_PrepReraiseStar
use crate::builtins::PyList;

let excs_list = excs
.downcast::<PyList>()
.map_err(|_| vm.new_type_error("expected list for prep_reraise_star"))?;

let excs_vec: Vec<PyObjectRef> = excs_list.borrow_vec().to_vec();

// Filter out None values
let mut raised: Vec<PyObjectRef> = Vec::new();
let mut reraised: Vec<PyObjectRef> = Vec::new();

for exc in excs_vec {
if vm.is_none(&exc) {
continue;
}
// Check if this exception was in the original exception group
if !vm.is_none(&orig) && is_same_exception_metadata(&exc, &orig, vm) {
reraised.push(exc);
} else {
raised.push(exc);
}
}

// If no exceptions to reraise, return None
if raised.is_empty() && reraised.is_empty() {
return Ok(vm.ctx.none());
}

// Combine raised and reraised exceptions
let mut all_excs = raised;
all_excs.extend(reraised);

if all_excs.len() == 1 {
// If only one exception, just return it
return Ok(all_excs.into_iter().next().unwrap());
}

// Create new ExceptionGroup
let excs_tuple = vm.ctx.new_tuple(all_excs);
let eg_type: PyObjectRef = crate::exception_group::exception_group().to_owned().into();
eg_type.call((vm.ctx.new_str(""), excs_tuple), vm)
}
Comment on lines +2493 to +2538
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, examine the prep_reraise_star and is_same_exception_metadata implementations
wc -l crates/vm/src/exceptions.rs

Repository: RustPython/RustPython

Length of output: 97


🏁 Script executed:

# Check the is_same_exception_metadata function to understand the "fragile membership check" claim
rg -A 20 "fn is_same_exception_metadata" crates/vm/src/exceptions.rs

Repository: RustPython/RustPython

Length of output: 789


🏁 Script executed:

# Look for tests related to prep_reraise_star and exception groups
fd -e rs -e py . crates/ | xargs grep -l "prep_reraise_star\|PrepReraiseStar" 2>/dev/null

Repository: RustPython/RustPython

Length of output: 154


🏁 Script executed:

# Search for tests in the test directory or within exceptions.rs
rg -n "test.*except\*\|test.*reraise" crates/vm/src/exceptions.rs | head -20

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check exception_group.rs to understand the design decisions mentioned in the learnings
cat -n crates/vm/src/exception_group.rs | sed -n '170,190p'

Repository: RustPython/RustPython

Length of output: 881


🏁 Script executed:

# Read the full prep_reraise_star implementation to see the context
sed -n '2493,2538p' crates/vm/src/exceptions.rs

Repository: RustPython/RustPython

Length of output: 1587


🏁 Script executed:

# Look at the complete function after line 2538 (mentioned as "Also applies to: 2540-2558")
sed -n '2530,2560p' crates/vm/src/exceptions.rs

Repository: RustPython/RustPython

Length of output: 1209


🏁 Script executed:

# Search for subgroup method to understand how it works
rg -A 15 "fn subgroup\|pub fn subgroup" crates/

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Look for tests related to except* and exception groups
find crates tests -name "*.rs" -o -name "*.py" | xargs grep -l "except\*\|ExceptionGroup" 2>/dev/null | head -10

Repository: RustPython/RustPython

Length of output: 333


🏁 Script executed:

# Search for test files that might test exception groups
fd -e py . tests/ | xargs grep -l "except\*" 2>/dev/null | head -5

Repository: RustPython/RustPython

Length of output: 158


🏁 Script executed:

# Find test directories in the repository
find . -type d -name test* 2>/dev/null | head -10

Repository: RustPython/RustPython

Length of output: 122


🏁 Script executed:

# Look at repo structure
git ls-files | grep -E "test|spec" | head -20

Repository: RustPython/RustPython

Length of output: 750


🏁 Script executed:

# Search for Python test files related to exception groups
find . -name "*.py" -type f 2>/dev/null | xargs grep -l "except\*\|ExceptionGroup" 2>/dev/null | head -10

Repository: RustPython/RustPython

Length of output: 109


🏁 Script executed:

# Look for subgroup implementation in exception_group.rs
rg -B 5 -A 20 "fn subgroup" crates/vm/src/exception_group.rs

Repository: RustPython/RustPython

Length of output: 999


🏁 Script executed:

# Check CPython's _PyExc_PrepReraiseStar to understand the expected behavior
web_search
CPython _PyExc_PrepReraiseStar implementation nested exception groups

Repository: RustPython/RustPython

Length of output: 161


🌐 Web query:

CPython _PyExc_PrepReraiseStar implementation exception groups

💡 Result:

Short answer — what _PyExc_PrepReraiseStar does and why

  • _PyExc_PrepReraiseStar is an internal CPython helper used when re‑raising an exception group with the "raise " (reraise‑star) semantics introduced by PEP 654. Its job is to prepare a value suitable for re‑raising from an original exception object and a sequence (or group) of nested exceptions: normalizing/constructing the appropriate ExceptionGroup/BaseExceptionGroup object, and making sure context/cause/traceback behavior matches the semantics PEP 654 requires for except / reraise-star. [1][2]

  • The function is declared in CPython’s internal pycore pyerrors header as:
    PyObject* _PyExc_PrepReraiseStar(PyObject *orig, PyObject *excs);
    (where orig is the original exception object and excs is the sequence / group of exceptions to be reraised). [2]

  • Conceptually it implements the re‑raise semantics described in PEP 654: when an except* handler yields a subset of a group (or re‑raises parts), CPython must produce a (possibly new) exception group that preserves appropriate metadata (message, cause/context, and composed tracebacks for leaf exceptions) and obeys ephemeral/handler semantics documented in PEP 654. _PyExc_PrepReraiseStar centralizes those preparation steps before the interpreter actually raises the resulting exception object. [1]

References

  • PEP 654 — Exception Groups and except*. [1]
  • CPython internal declaration (pycore pyerrors.h) showing _PyExc_PrepReraiseStar. [2]

🏁 Script executed:

# Check the full subgroup implementation
rg -B 5 -A 40 "fn subgroup" crates/vm/src/exception_group.rs

Repository: RustPython/RustPython

Length of output: 1625


🏁 Script executed:

# Check for exception group tests in the test directory
find Lib/test -name "*.py" | xargs grep -l "except\*\|ExceptionGroup" 2>/dev/null | head -10

Repository: RustPython/RustPython

Length of output: 108


🏁 Script executed:

# Look at the builtin_exceptions.py test file
cat extra_tests/snippets/builtin_exceptions.py | head -100

Repository: RustPython/RustPython

Length of output: 2105


🏁 Script executed:

# Search for prep_reraise_star usage and tests
rg -B 5 -A 10 "prep_reraise_star" crates/

Repository: RustPython/RustPython

Length of output: 2371


🏁 Script executed:

# Search for tests related to except* more thoroughly
find . -type f \( -name "*.py" -o -name "*.rs" \) | xargs grep -l "except\s*\*" 2>/dev/null | head -20

Repository: RustPython/RustPython

Length of output: 235


🏁 Script executed:

# Look at the compiler to see how except* is handled and what excs contains
rg -B 10 -A 10 "PrepReraiseStar\|prep_reraise_star" crates/compiler-core/ 2>/dev/null

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Check if there are any tests files in extra_tests
find extra_tests -name "*.py" | xargs grep -l "except\s*\*\|ExceptionGroup" 2>/dev/null

Repository: RustPython/RustPython

Length of output: 107


🏁 Script executed:

# Look at the derive method in exception_group.rs to understand the design decision mentioned in learnings
rg -B 5 -A 30 "#\[pymethod\].*fn derive" crates/vm/src/exception_group.rs

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Search for derive method in exception_group.rs with different pattern
rg -B 3 -A 25 "fn derive" crates/vm/src/exception_group.rs

Repository: RustPython/RustPython

Length of output: 1931


🏁 Script executed:

# Check builtin_exceptions.py for except* tests
grep -n "except\s*\*\|ExceptionGroup" extra_tests/snippets/builtin_exceptions.py | head -20

Repository: RustPython/RustPython

Length of output: 255


🏁 Script executed:

# Read the full builtin_exceptions.py to see if there are exception group tests
wc -l extra_tests/snippets/builtin_exceptions.py

Repository: RustPython/RustPython

Length of output: 111


🏁 Script executed:

# Check what's in the codegen compile.rs regarding except* handling
rg -B 5 -A 15 "except\s*\*" crates/codegen/src/compile.rs | head -60

Repository: RustPython/RustPython

Length of output: 2105


🏁 Script executed:

# Look for how exceptions are handled in except* blocks to understand what excs contains
rg -B 10 -A 10 "except_star\|except\*" crates/compiler-core/src/bytecode.rs | head -100

Repository: RustPython/RustPython

Length of output: 47


🏁 Script executed:

# Read more of the builtin_exceptions.py test to see what except* coverage exists
sed -n '371,390p' extra_tests/snippets/builtin_exceptions.py

Repository: RustPython/RustPython

Length of output: 409


🏁 Script executed:

# Search for more except* test cases
rg -B 3 -A 10 "except\s*\*" extra_tests/snippets/builtin_exceptions.py

Repository: RustPython/RustPython

Length of output: 451


🏁 Script executed:

# Look at what CheckEgMatch does and how it populates excs for prep_reraise_star
rg -B 5 -A 15 "CheckEgMatch" crates/vm/src/

Repository: RustPython/RustPython

Length of output: 1538


🏁 Script executed:

# Understand the flow: check bytecode for how excs list is built before calling prep_reraise_star
rg -B 20 "PrepReraiseStar" crates/compiler-core/src/bytecode.rs | head -80

Repository: RustPython/RustPython

Length of output: 619


🏁 Script executed:

# Look at how the compiler generates the excs list that gets passed to prep_reraise_star
rg -B 15 -A 5 "PrepReraiseStar\|prep_reraise" crates/codegen/src/compile.rs | head -100

Repository: RustPython/RustPython

Length of output: 47


prep_reraise_star and exception grouping behavior need verification against complex PEP 654 scenarios

The current implementation handles basic cases but has notable gaps:

  • Flattened exception structure: All matched and reraised exceptions are collected into a single flat tuple and wrapped as ExceptionGroup("", all_excs). This differs from CPython's approach of preserving nested group structure when selectively reraising subsets, which may affect tracebacks and exception grouping semantics in non-trivial cases.

  • Limited test coverage: Only one trivial test exists (extra_tests/snippets/builtin_exceptions.py:371–380—a bare ValueError with except*). No tests cover nested exception groups, multiple exceptions, mixed raise/reraise patterns, or the richer PEP 654 scenarios. This makes it difficult to assess correctness beyond the simplest paths.

  • Potential non-group orig gaps: CPython's _PyExc_PrepReraiseStar has special-case handling when orig is not an ExceptionGroup itself. The current implementation does not appear to have this optimization.

To improve confidence, add tests for nested groups and complex except* patterns (matching the PEP 654 examples), then compare behavior against CPython. The design decision to always create BaseExceptionGroup (rather than preserving the original exception class) is intentional per prior decisions, but the exception grouping logic itself should be validated against CPython's behavior for correctness.


/// Check if two exceptions have the same metadata (for reraise detection)
fn is_same_exception_metadata(exc1: &PyObjectRef, exc2: &PyObjectRef, vm: &VirtualMachine) -> bool {
// Check if exc1 is part of exc2's exception group
if exc2.fast_isinstance(vm.ctx.exceptions.base_exception_group) {
let exc_class: PyObjectRef = exc1.class().to_owned().into();
if let Ok(result) = vm.call_method(exc2, "subgroup", (exc_class,))
&& !vm.is_none(&result)
&& let Ok(subgroup_excs) = result.get_attr("exceptions", vm)
&& let Ok(tuple) = subgroup_excs.downcast::<PyTuple>()
{
for e in tuple.iter() {
if e.is(exc1) {
return true;
}
}
}
}
exc1.is(exc2)
}
15 changes: 15 additions & 0 deletions crates/vm/src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,15 @@ impl ExecutingFrame<'_> {
let args = self.collect_positional_args(nargs.get(arg));
self.execute_method_call(args, vm)
}
bytecode::Instruction::CheckEgMatch => {
let match_type = self.pop_value();
let exc_value = self.pop_value();
let (rest, matched) =
crate::exceptions::exception_group_match(&exc_value, &match_type, vm)?;
self.push_value(rest);
self.push_value(matched);
Ok(None)
}
bytecode::Instruction::CompareOperation { op } => self.execute_compare(vm, op.get(arg)),
bytecode::Instruction::ContainsOp(invert) => {
let b = self.pop_value();
Expand Down Expand Up @@ -2494,6 +2503,12 @@ impl ExecutingFrame<'_> {
.into();
Ok(type_var)
}
bytecode::IntrinsicFunction2::PrepReraiseStar => {
// arg1 = orig (original exception)
// arg2 = excs (list of exceptions raised/reraised in except* blocks)
// Returns: exception to reraise, or None if nothing to reraise
crate::exceptions::prep_reraise_star(arg1, arg2, vm)
}
}
}

Expand Down
Loading
Loading