Add except* support - #6530
Conversation
Sorry, something went wrong.
📝 Walkthrough
WalkthroughThis PR implements runtime support for Python 3.11's except* exception handling syntax. Changes span bytecode instruction definitions, compiler except* statement handling, VM exception matching and reraising logic, frame instruction execution, and supporting tests for exception group normalization. Changes
Sequence DiagramsequenceDiagram
participant Try as Try Block
participant Match as Exception Matcher
participant Handler as Handler Executor
participant Reraise as Reraise Logic
participant Finally as Finally Block
Try->>Match: Exception occurs
Match->>Match: exception_group_match()
alt Exception matches handler type
Match->>Handler: Wrap in ExceptionGroup if needed
Handler->>Handler: Bind exception name
Handler->>Handler: Execute handler body
Handler->>Reraise: Prepare unhandled exceptions
else No match
Match->>Reraise: Exception doesn't match
end
Reraise->>Reraise: prep_reraise_star()
Reraise->>Finally: Construct/combine exceptions
Finally->>Finally: Execute finally block
Finally->>Finally: Raise combined exception if needed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>
Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>
Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>
Co-authored-by: youknowone <69878+youknowone@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)crates/codegen/src/compile.rs (1)📜 Review details2220-2243: Consider adding inline comments documenting stack state transitions.
The logic is correct, but the stack state transitions through the unhandled exception check are complex:
- Line 2224: [rest] → [rest, rest]
- Line 2225: [rest, rest] → [rest, bool]
- Line 2226: [rest, bool] → [rest] (if truthy) or jump to handled_block (if falsy)
- Line 2236: Raise with [rest] on stack
- Lines 2242-2243: Pop rest and exception state in handled_block
Adding stack state comments (similar to line 2155) would improve maintainability for future developers working on exception handling.
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between c9bf8df and 8c2cddc.
⛔ Files ignored due to path filters (1)- Lib/test/test_compile.py is excluded by !Lib/**
- .cspell.dict/python-more.txt
- crates/codegen/src/compile.rs
- crates/compiler-core/src/bytecode.rs
- crates/vm/src/exceptions.rs
- crates/vm/src/frame.rs
- extra_tests/snippets/builtin_exceptions.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust
Files:
- crates/vm/src/exceptions.rs
- crates/compiler-core/src/bytecode.rs
- crates/vm/src/frame.rs
- crates/codegen/src/compile.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: In most cases, Python code should not be edited; bug fixes should be made through Rust code modifications only
Follow PEP 8 style for custom Python code
Use ruff for linting Python code
Files:
- extra_tests/snippets/builtin_exceptions.py
Learnt from: youknowone Repo: RustPython/RustPython PR: 6358 File: crates/vm/src/exception_group.rs:173-185 Timestamp: 2025-12-09T08:46:58.660Z Learning: In crates/vm/src/exception_group.rs, the derive() method intentionally always creates a BaseExceptionGroup instance rather than preserving the original exception class type. This is a deliberate design decision that differs from CPython's behavior.
Applied to files:
- crates/vm/src/exceptions.rs
- crates/vm/src/frame.rs
crates/vm/src/exceptions.rs (3)🔇 Additional comments (9)
- exc_value (2462-2462)
- exception_group_match (2438-2491)
- prep_reraise_star (2494-2538)
.cspell.dict/python-more.txt (1)extra_tests/snippets/builtin_exceptions.py (1)202-202: Dictionary entry reraised looks correct
Spelling and alphabetical placement are consistent with the rest of the cspell word list; matches new exception reraising terminology.
crates/vm/src/frame.rs (2)371-380: except normalization test matches CPython semantics*
The assertions about ExceptionGroup wrapping a naked ValueError("x") and preserving the inner exception and its arguments align with Python 3.11’s documented except* behavior and PEP 654 examples. No issues spotted here.
If you also run extra_tests under CPython, please confirm they only execute on a 3.11+ interpreter so the except* syntax is always accepted.
crates/compiler-core/src/bytecode.rs (2)694-702: CheckEgMatch VM handler is stack‑correct and aligns with CPython
The handler pops match_type then exc_value, calls exception_group_match, and pushes (rest, matched) in that order, giving a net stack effect of 0 as declared in the bytecode metadata. This matches CPython’s CHECK_EG_MATCH contract where the stack shape becomes [..., rest_or_exc, match?] with the match value on top. No issues found.
It would still be good to add/extend a VM test that exercises the full CHECK_EG_MATCH → POP_JUMP_IF_NONE pattern on both naked exceptions and nested ExceptionGroups, comparing behavior against CPython 3.11+.
2506-2511: PrepReraiseStar intrinsic wiring is correct
CallIntrinsic2 already pops arg2 then arg1 and forwards (arg1, arg2) into call_intrinsic_2; here those are correctly treated as (orig, excs) and delegated to exceptions::prep_reraise_star. This matches CPython’s binary intrinsic prep_reraise_star(orig, excs) signature.
Please confirm via tests mirroring CPython’s _testinternalcapi.PrepReraiseStar cases (nested groups, combinations of raised vs reraised exceptions) that the intrinsic is only ever invoked with excs as a list and orig as the original exception/group.
crates/vm/src/exceptions.rs (1)655-657: CheckEgMatch instruction is consistently integrated
- The new Instruction::CheckEgMatch variant, its stack effect (0, pop 2/push 2), and disassembly label CHECK_EG_MATCH are all wired consistently.
- This matches CPython’s CHECK_EG_MATCH behavior and the new VM handler in frame.rs.
Once the compiler emits CheckEgMatch for except* bodies, please run disassembly on a small except* snippet under both CPython 3.11+ and RustPython to confirm opcode sequences and stack effects agree.
Also applies to: 1727-1728, 1894-1895
577-583: IntrinsicFunction2 was newly added, not reordered
IntrinsicFunction2 with discriminants PrepReraiseStar=1 through SetTypeparamDefault=5 is a new enum, not a reordering of existing variants. The actual bytecode compatibility concern was the alphabetical sorting of the Instruction enum (commit d287d1e), which changed discriminant values for all instructions. FORMAT_VERSION remains at 4 despite this structural change.
Frozen bytecode (embedded stdlib) is regenerated on each build and deserialized at runtime, so persisted frozen modules are not affected. RustPython's standard VM does not actively implement .pyc caching, making this a non-issue for typical deployments.
crates/codegen/src/compile.rs (2)2436-2491: exception_group_match correctly implements CHECK_EG_MATCH semantics
The logic closely mirrors CPython’s _PyEval_ExceptionGroupMatch:
- None input returns (None, None).
- Naked exceptions that match match_type are wrapped in an ExceptionGroup("", (exc,)) and have their traceback copied, matching documented “naked exception” behavior for except*.
- Exception groups delegate to .split(match_type) with robust error checking, and you return (rest, matched) in the order expected by the VM’s CHECK_EG_MATCH handler.
- Non‑matching values fall through to (exc_value.clone(), None).
This should give correct behavior for both bare exceptions and nested ExceptionGroups in simple except* cases.
Consider adding tests that hit: (a) partial matches on nested ExceptionGroups, (b) invalid .split() implementations on custom group subclasses (ensuring your TypeError path triggers), and (c) the None case to mirror CPython’s _testinternalcapi coverage.
2166-2181: LGTM! Syntax validation correctly handles except requirements.*
The implementation properly validates:
- Multiple exception types must be parenthesized - the source range comparison correctly detects except* A, B: vs except* (A, B):
- except* must specify an exception type - correctly enforces this PEP 654 requirement
2184-2184: CheckEgMatch instruction is properly implemented and tested.
The CheckEgMatch instruction (defined in crates/compiler-core/src/bytecode.rs:657) has a complete VM implementation in crates/vm/src/frame.rs:694-702 that calls exception_group_match() from crates/vm/src/exceptions.rs:2438-2499. This function:
- Splits exception groups using the split() method to separate matching and non-matching exceptions
- Returns (rest, match) tuples as documented
- Handles nested exception groups correctly through the split mechanism
- Wraps naked exceptions in ExceptionGroup when they match the type
Tests in extra_tests/snippets/builtin_exceptions.py:371-380 verify except* handling with both naked and grouped exceptions.
Sorry, something went wrong.
| /// 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) | ||
| } |
There was a problem hiding this comment.
⚠️ 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.rsRepository: 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.rsRepository: 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/nullRepository: 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 -20Repository: 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.rsRepository: 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.rsRepository: 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 -10Repository: 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 -5Repository: RustPython/RustPython
Length of output: 158
🏁 Script executed:
# Find test directories in the repository
find . -type d -name test* 2>/dev/null | head -10Repository: RustPython/RustPython
Length of output: 122
🏁 Script executed:
# Look at repo structure
git ls-files | grep -E "test|spec" | head -20Repository: 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 -10Repository: 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.rsRepository: 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 groupsRepository: 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.rsRepository: 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 -10Repository: RustPython/RustPython
Length of output: 108
🏁 Script executed:
# Look at the builtin_exceptions.py test file
cat extra_tests/snippets/builtin_exceptions.py | head -100Repository: 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 -20Repository: 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/nullRepository: 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/nullRepository: 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.rsRepository: 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.rsRepository: 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 -20Repository: 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.pyRepository: 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 -60Repository: 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 -100Repository: 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.pyRepository: 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.pyRepository: 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 -80Repository: 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 -100Repository: 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.
Sorry, something went wrong.
ShaharNaveh
left a comment
There was a problem hiding this comment.
Looks great!
Sorry, something went wrong.
fix #4479
close #6522
@ShaharNaveh Does this looking better?
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.