Make SystemExit.code a writable attribute set at init by kangdora · Pull Request #8282 · RustPython/RustPython · GitHub
Skip to content

Make SystemExit.code a writable attribute set at init - #8282

Merged
youknowone merged 5 commits into
RustPython:mainfrom
kangdora:fix-systemexit-code
Jul 15, 2026
Merged

Make SystemExit.code a writable attribute set at init#8282
youknowone merged 5 commits into
RustPython:mainfrom
kangdora:fix-systemexit-code

Conversation

@kangdora

@kangdora kangdora commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

SystemExit.code was a read-only getset recomputed from args on
every access, so assigning or deleting exc.code raised
AttributeError, and mutating exc.args after init changed code.
It is now stored in a dedicated struct field (#[repr(C)] with a
PyBaseException base, the same approach as OSError), set at
__init__ time — writable, deletable, and independent from args
after init, as in CPython. Keeping it in a field rather than the
instance dict leaves SystemExit().__dict__ empty, matching CPython.

A second commit updates handle_exit_exception to read the exit
status from the code attribute like CPython's handle_system_exit;
without it, reassigning exc.code would not affect the process exit
status:

import sys
try:
    sys.exit(1)
except SystemExit as e:
    e.code = 0   # before: AttributeError: ... not writable
    raise
# process exit status — before: 1, after: 0 (matches CPython)

Test plan

  • Verified against CPython 3.14.6: attribute write/delete including the subclass repro from SystemExit.code is read-only; CPython allows assignment #8230, exit statuses for sys.exit() /
    sys.exit(3) / sys.exit('msg') / raise SystemExit(1, 2), and the re-raise case above.
  • Lib/test/test_exceptions.py, Lib/test/test_sys.py: no regressions (the single test_badisinstance failure is pre-existing on main).
  • test_threading / test_io thread-finalization cases pass (SystemExit is
    now created via the full constructor path, not new_exception).
  • cargo fmt / cargo clippy: clean (no new warnings).

Assisted-by: Claude Code:claude-fable-5

Summary by CodeRabbit

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of SystemExit during termination, including finalization and non-main threads.
    • Exit codes are now derived from SystemExit.code (with None mapping to 0; integer values supported).
    • SystemExit output is generated more reliably from the code value when present.
    • SystemExit created via exit (and internal thread exit) now follows the same raising path, ensuring consistent behavior with provided exit values.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/vm/src/exceptions.rs (1)

1658-1676: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move the BaseException_init note to the PyBaseException::slot_init call below. The current placement describes the match above, not the call it refers to.

🤖 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/exceptions.rs` around lines 1658 - 1676, Move the “Call
BaseException_init first (handles args)” comment from above the code match in
PySystemExit::slot_init to immediately above the PyBaseException::slot_init
call, so it documents the call it refers to.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@crates/vm/src/exceptions.rs`:
- Around line 1658-1676: Move the “Call BaseException_init first (handles args)”
comment from above the code match in PySystemExit::slot_init to immediately
above the PyBaseException::slot_init call, so it documents the call it refers
to.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 6a9aa971-2421-42b9-9cad-54153f7ebee2

📥 Commits

Reviewing files that changed from the base of the PR and between 4d6ac3f and fb8f7b1.

📒 Files selected for processing (1)
  • crates/vm/src/exceptions.rs

@youknowone youknowone added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 15, 2026

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks great, thanks! welcome to RustPython project!

note about the change: once we couldn't set getset to exception subtypes. that restriction was resolved early this year, but this one was not updated for a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SystemExit.code is read-only; CPython allows assignment

2 participants