Add except* support #6530
Add except* support #6530
Conversation
📝 WalkthroughWalkthroughThis PR implements runtime support for Python 3.11's 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)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
f6ded50 to
fd03d97
Compare
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>
fd03d97 to
1e2fa6b
Compare
1e2fa6b to
8c2cddc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/codegen/src/compile.rs (1)
2220-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.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Lib/test/test_compile.pyis excluded by!Lib/**
📒 Files selected for processing (6)
.cspell.dict/python-more.txtcrates/codegen/src/compile.rscrates/compiler-core/src/bytecode.rscrates/vm/src/exceptions.rscrates/vm/src/frame.rsextra_tests/snippets/builtin_exceptions.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto 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.rscrates/compiler-core/src/bytecode.rscrates/vm/src/frame.rscrates/codegen/src/compile.rs
**/*.py
📄 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
🧠 Learnings (1)
📚 Learning: 2025-12-09T08:46:58.660Z
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.rscrates/vm/src/frame.rs
🧬 Code graph analysis (1)
crates/vm/src/frame.rs (1)
crates/vm/src/exceptions.rs (3)
exc_value(2462-2462)exception_group_match(2438-2491)prep_reraise_star(2494-2538)
🔇 Additional comments (9)
.cspell.dict/python-more.txt (1)
202-202: Dictionary entryreraisedlooks correctSpelling and alphabetical placement are consistent with the rest of the cspell word list; matches new exception reraising terminology.
extra_tests/snippets/builtin_exceptions.py (1)
371-380: except normalization test matches CPython semantics*The assertions about
ExceptionGroupwrapping a nakedValueError("x")and preserving the inner exception and its arguments align with Python 3.11’s documentedexcept*behavior and PEP 654 examples. No issues spotted here.If you also run
extra_testsunder CPython, please confirm they only execute on a 3.11+ interpreter so theexcept*syntax is always accepted.crates/vm/src/frame.rs (2)
694-702:CheckEgMatchVM handler is stack‑correct and aligns with CPythonThe handler pops
match_typethenexc_value, callsexception_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’sCHECK_EG_MATCHcontract 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_NONEpattern on both naked exceptions and nestedExceptionGroups, comparing behavior against CPython 3.11+.
2506-2511:PrepReraiseStarintrinsic wiring is correct
CallIntrinsic2already popsarg2thenarg1and forwards(arg1, arg2)intocall_intrinsic_2; here those are correctly treated as(orig, excs)and delegated toexceptions::prep_reraise_star. This matches CPython’s binary intrinsicprep_reraise_star(orig, excs)signature.Please confirm via tests mirroring CPython’s
_testinternalcapi.PrepReraiseStarcases (nested groups, combinations of raised vs reraised exceptions) that the intrinsic is only ever invoked withexcsas a list andorigas the original exception/group.crates/compiler-core/src/bytecode.rs (2)
655-657:CheckEgMatchinstruction is consistently integrated
- The new
Instruction::CheckEgMatchvariant, its stack effect (0, pop 2/push 2), and disassembly labelCHECK_EG_MATCHare all wired consistently.- This matches CPython’s
CHECK_EG_MATCHbehavior and the new VM handler inframe.rs.Once the compiler emits
CheckEgMatchforexcept*bodies, please run disassembly on a smallexcept*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 reorderedIntrinsicFunction2 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
Instructionenum (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/vm/src/exceptions.rs (1)
2436-2491:exception_group_matchcorrectly implementsCHECK_EG_MATCHsemanticsThe logic closely mirrors CPython’s
_PyEval_ExceptionGroupMatch:
Noneinput returns(None, None).- Naked exceptions that match
match_typeare wrapped in anExceptionGroup("", (exc,))and have their traceback copied, matching documented “naked exception” behavior forexcept*.- Exception groups delegate to
.split(match_type)with robust error checking, and you return(rest, matched)in the order expected by the VM’sCHECK_EG_MATCHhandler.- Non‑matching values fall through to
(exc_value.clone(), None).This should give correct behavior for both bare exceptions and nested
ExceptionGroups in simpleexcept*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) theNonecase to mirror CPython’s_testinternalcapicoverage.crates/codegen/src/compile.rs (2)
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:vsexcept* (A, B):- except* must specify an exception type - correctly enforces this PEP 654 requirement
2184-2184: CheckEgMatch instruction is properly implemented and tested.The
CheckEgMatchinstruction (defined incrates/compiler-core/src/bytecode.rs:657) has a complete VM implementation incrates/vm/src/frame.rs:694-702that callsexception_group_match()fromcrates/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-380verify except* handling with both naked and grouped exceptions.
| /// 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.
🧩 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 bareValueErrorwithexcept*). 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
origgaps: CPython's_PyExc_PrepReraiseStarhas special-case handling whenorigis not anExceptionGroupitself. 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.
fix #4479
close #6522
@ShaharNaveh Does this looking better?
Summary by CodeRabbit
Release Notes
New Features
except*exception handling syntax, enabling proper matching and filtering of exceptions across multiple handlers. Non-group exceptions are automatically wrapped when caught withexcept*.Tests
✏️ Tip: You can customize this high-level summary in your review settings.