Add builtin.PythonFinalizationError#7966
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds ChangesException type exports
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
These changes are unrelated, but failed on my machine. I guess that they are skipped on thr CI
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_builtin.py (TODO: 18) dependencies: dependent tests: (no tests depend on builtin) [x] lib: cpython/Lib/pickle.py dependencies:
dependent tests: (100 tests)
Legend:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vm/src/stdlib/builtins.rs`:
- Line 1477: The builtins mapping adds "PythonFinalizationError" pointing to
ctx.exceptions.python_finalization_error but the exception_hierarchy.txt doesn’t
include that symbol so the “ordered by exception_hierarchy.txt” guarantee is
invalid; update exception_hierarchy.txt to include PythonFinalizationError in
the proper place in the hierarchy (or adjust the ordering comment in
crates/vm/src/stdlib/builtins.rs) and ensure the symbol name matches the
constructor defined in crates/vm/src/exceptions.rs (PythonFinalizationError /
ctx.exceptions.python_finalization_error) so the wiring and ordering are
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 646474cc-d196-4510-bfa6-8d6286e519c1
⛔ Files ignored due to path filters (2)
Lib/test/test_builtin.pyis excluded by!Lib/**Lib/test/test_pickle.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/stdlib/builtins.rs
| "TimeoutError" => ctx.exceptions.timeout_error.to_owned(), | ||
| "ReferenceError" => ctx.exceptions.reference_error.to_owned(), | ||
| "RuntimeError" => ctx.exceptions.runtime_error.to_owned(), | ||
| "PythonFinalizationError" => ctx.exceptions.python_finalization_error.to_owned(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if exception_hierarchy.txt exists and verify PythonFinalizationError placement
echo "=== Checking exception_hierarchy.txt ==="
fd -t f 'exception_hierarchy' --exec cat {}
# Verify python_finalization_error field exists in the exceptions context
echo -e "\n=== Verifying python_finalization_error field exists ==="
rg -n 'python_finalization_error' --type rust
# Check for any related tests or documentation
echo -e "\n=== Checking for related tests ==="
rg -n 'PythonFinalizationError' --type rust -g '!builtins.rs'Repository: RustPython/RustPython
Length of output: 14832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== exception_hierarchy* files ==="
fd -t f 'exception_hierarchy' .
echo
echo "=== exception_hierarchy* occurrences of PythonFinalizationError ==="
for f in $(fd -t f 'exception_hierarchy' .); do
if rg -n 'PythonFinalizationError' "$f" >/dev/null; then
echo "-- $f"
rg -n 'PythonFinalizationError' "$f"
fi
done
echo
echo "=== builtins.rs context around line 1477 ==="
nl -ba crates/vm/src/stdlib/builtins.rs | sed -n '1455,1505p'
echo
echo "=== builtins.rs comment about exception_hierarchy.txt ==="
nl -ba crates/vm/src/stdlib/builtins.rs | sed -n '1405,1445p'
echo
echo "=== exceptions.rs definition of PyPythonFinalizationError base ==="
nl -ba crates/vm/src/exceptions.rs | sed -n '2425,2475p'Repository: RustPython/RustPython
Length of output: 1964
Confirm PythonFinalizationError wiring; hierarchy ordering can’t be checked from exception_hierarchy.txt
ctx.exceptions.python_finalization_errorexists (crates/vm/src/exceptions.rs) and the builtins mapping follows the same pattern as other exceptions.- The
exception_hierarchy/exception_hierarchy.txtcontent available here doesn’t includePythonFinalizationError, so the “ordered by exception_hierarchy.txt” requirement can’t be validated—add it to the hierarchy file (or adjust the ordering comment) to keep this guarantee accurate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/vm/src/stdlib/builtins.rs` at line 1477, The builtins mapping adds
"PythonFinalizationError" pointing to ctx.exceptions.python_finalization_error
but the exception_hierarchy.txt doesn’t include that symbol so the “ordered by
exception_hierarchy.txt” guarantee is invalid; update exception_hierarchy.txt to
include PythonFinalizationError in the proper place in the hierarchy (or adjust
the ordering comment in crates/vm/src/stdlib/builtins.rs) and ensure the symbol
name matches the constructor defined in crates/vm/src/exceptions.rs
(PythonFinalizationError / ctx.exceptions.python_finalization_error) so the
wiring and ordering are consistent.
Summary by CodeRabbit
PythonFinalizationErrorexception is now available in the builtins module for proper error handling during finalization processes.