repr with slot wrapper by youknowone · Pull Request #6486 · RustPython/RustPython

youknowone

📝 Walkthrough

Walkthrough

The PR systematically migrates __str__ and __repr__ method signatures across exception types and builtin objects from PyRef-based and PyBaseExceptionRef receivers to Py<Self> and &Py<PyBaseException> patterns. Method return types shift to PyResult<PyStrRef>. Additionally, descriptor slot functions are extended with a new Repr variant, and representation slot wrappers are auto-generated in class definitions.

Changes

Cohort / File(s)	Summary
Exception type str signature migrations `crates/vm/src/exceptions.rs`, `crates/vm/src/exception_group.rs`, `crates/stdlib/src/ssl/error.rs`	Updated multiple exception classes' `__str__` methods to accept `&Py<PyBaseException>` receiver (zelf) instead of `PyBaseExceptionRef` and return `PyResult<PyStrRef>` instead of `PyStrRef`/`String`. PyBaseException now includes `Py` in its `with(...)` declaration. Internal logic adapted to use `zelf.as_object()` and wrap results in `Ok(...)`.
Builtin type str updates `crates/vm/src/builtins/str.rs`, `crates/vm/src/builtins/weakproxy.rs`, `crates/vm/src/stdlib/winreg.rs`	PyStr.str changed from PyRef-based method to Py-based with logic distinguishing exact str vs. subclass instances. PyWeakProxy and PyHkey.str now use `&Py<Self>` receiver pattern and return `PyResult<PyStrRef>`/`PyResult<PyRef<PyStr>>` respectively.
Slot and descriptor infrastructure `crates/vm/src/builtins/descriptor.rs`, `crates/vm/src/types/slot.rs`	Added `Repr(StringifyFunc)` variant to `SlotFunc` enum with corresponding `call` and Debug handling. Removed public `__repr__` method from Representable trait. Modified PySlotWrapper descriptor access by removing early-return guard for `Some(obj)` when `vm.is_none(&obj)`.
Class definition and error handling `crates/vm/src/class.rs`, `crates/vm/src/stdlib/io.rs`	Added auto-generation of `__repr__` slot wrapper in `PyClassImpl::extend_class` when a repr slot is defined. Enhanced error message construction in BufferedMixin to safely handle exception string conversion with fallback.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Fix openssl build and shared ssl/error.rs #6456 — Modifies the same ssl_error module's PySSLError.str implementation with receiver type changes.
Complete tp_repr #5852 — Changes how object representation slots and repr are defined and invoked across the codebase.
New subclass payload layout #6319 — Broadly refactors exception payloads to use Py<...> wrappers and updates exception str/error construction patterns.

Suggested reviewers

ShaharNaveh
coolreader18
arihant2math

Poem

🐰 Hop, hop! The strings now dance with Py,
No more PyRef, we say goodbye,
Descriptors bloom with Repr so bright,
Class slots awaken—what a sight!
Exceptions speak in types so true,
RustPython's str breaks through! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: implementing repr as a slot wrapper, which is the primary focus across multiple files in this changeset.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c3bc5e and ebbbb4f.

📒 Files selected for processing (10)

crates/stdlib/src/ssl/error.rs
crates/vm/src/builtins/descriptor.rs
crates/vm/src/builtins/str.rs
crates/vm/src/builtins/weakproxy.rs
crates/vm/src/class.rs
crates/vm/src/exception_group.rs
crates/vm/src/exceptions.rs
crates/vm/src/stdlib/io.rs
crates/vm/src/stdlib/winreg.rs
crates/vm/src/types/slot.rs

💤 Files with no reviewable changes (1)

crates/vm/src/types/slot.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 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/class.rs
crates/vm/src/stdlib/io.rs
crates/vm/src/builtins/weakproxy.rs
crates/stdlib/src/ssl/error.rs
crates/vm/src/exception_group.rs
crates/vm/src/builtins/str.rs
crates/vm/src/builtins/descriptor.rs
crates/vm/src/stdlib/winreg.rs
crates/vm/src/exceptions.rs

🧠 Learnings (3)

📚 Learning: 2025-11-10T06:27:41.954Z

Learnt from: youknowone
Repo: RustPython/RustPython PR: 6243
File: vm/src/function/buffer.rs:123-129
Timestamp: 2025-11-10T06:27:41.954Z
Learning: In vm/src/function/buffer.rs, line 36 in the try_rw_bytes_like method intentionally uses TypeError (not BufferError) for the "buffer is not a read-write bytes-like object" error case, even though a similar error message in ArgMemoryBuffer::try_from_borrowed_object uses BufferError. The TypeError is the intended exception type for this specific code path.

Applied to files:

crates/vm/src/stdlib/io.rs

📚 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/stdlib/io.rs
crates/vm/src/exception_group.rs
crates/vm/src/exceptions.rs

📚 Learning: 2025-11-29T12:17:28.606Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

crates/vm/src/builtins/descriptor.rs

🧬 Code graph analysis (8)

crates/vm/src/class.rs (2)

crates/vm/src/builtins/object.rs (1)

__repr__ (466-468)

crates/vm/src/types/structseq.rs (1)

__repr__ (229-231)

crates/vm/src/stdlib/io.rs (2)

crates/vm/src/stdlib/builtins.rs (1)

format (393-399)

crates/stdlib/src/pystruct.rs (1)

format (254-256)

crates/vm/src/builtins/weakproxy.rs (2)

crates/vm/src/stdlib/winreg.rs (1)

__str__ (236-238)

crates/vm/src/builtins/object.rs (1)

__str__ (429-432)

crates/stdlib/src/ssl/error.rs (3)

crates/vm/src/builtins/weakproxy.rs (1)

__str__ (82-84)

crates/vm/src/exception_group.rs (1)

__str__ (185-203)

crates/vm/src/exceptions.rs (7)

__str__ (641-648)

__str__ (1533-1543)

__str__ (1737-1826)

__str__ (2032-2076)

__str__ (2184-2210)

__str__ (2241-2268)

__str__ (2298-2323)

crates/vm/src/exception_group.rs (1)

crates/vm/src/exceptions.rs (8)

__str__ (641-648)

__str__ (1533-1543)

__str__ (1737-1826)

__str__ (2032-2076)

__str__ (2184-2210)

__str__ (2241-2268)

__str__ (2298-2323)

zelf (1684-1684)

crates/vm/src/builtins/str.rs (5)

crates/vm/src/builtins/weakproxy.rs (1)

__str__ (82-84)

crates/vm/src/stdlib/ctypes/array.rs (1)

zelf (306-307)

crates/vm/src/stdlib/ctypes/structure.rs (1)

zelf (136-137)

crates/vm/src/stdlib/ctypes/simple.rs (1)

zelf (250-251)

crates/vm/src/stdlib/ctypes/union.rs (1)

zelf (304-305)

crates/vm/src/stdlib/winreg.rs (2)

crates/vm/src/builtins/str.rs (3)

__str__ (1452-1460)

s (1783-1788)

s (1814-1818)

crates/vm/src/builtins/weakproxy.rs (1)

__str__ (82-84)

crates/vm/src/exceptions.rs (2)

crates/vm/src/exception_group.rs (4)

__str__ (185-203)

zelf (212-213)

obj (195-195)

obj (386-387)

crates/vm/src/builtins/object.rs (1)

__str__ (429-432)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Run snippets and cpython tests (windows-latest)

🔇 Additional comments (16)

crates/vm/src/class.rs (1)

177-189: repr slot wrapper wiring is consistent with existing slot wrappers

The new __repr__ wrapper follows the same pattern as __init__ / __hash__: it only installs a PySlotWrapper when a repr slot exists and __repr__ is not already present in the type dict, and correctly uses SlotFunc::Repr. This is a clean, minimal integration with the new stringification slot.

crates/vm/src/stdlib/io.rs (1)

1468-1475: More robust formatting of argument-binding errors in BufferedMixin::slot_init

Using e.__str__(vm) with a safe fallback to "<error getting exception str>" keeps the existing error shape ("CLASS_NAME() <msg>") while avoiding failures during stringification. The mapped exception class via e.class() is preserved, so behavior is consistent and safer.

crates/stdlib/src/ssl/error.rs (1)

8-9: PySSLError::str correctly migrated to &Py / PyStrRef

The new signature matches the project-wide pattern (&Py<PyBaseException> -> PyResult<PyStrRef>), and the body still:

Prefers a non-None strerror attribute (OSError-compatible),

Falls back to str(args[0]) or str(args).

No behavior regressions are apparent; this aligns with the core exception stringification helpers.

Also applies to: 45-61

crates/vm/src/builtins/descriptor.rs (1)

10-11: Repr slot integration via SlotFunc::Repr is well-structured

Importing StringifyFunc and adding SlotFunc::Repr(StringifyFunc) cleanly extends the existing slot abstraction.

SlotFunc::call validates that __repr__ receives no extra args (bound vs unbound) and forwards to the underlying stringify function, returning the resulting PyStrRef as a Python object, mirroring the Hash case.

The Debug impl is kept up to date.

This should make the new __repr__ slot wrappers behave exactly like native Python methods.

Also applies to: 398-402, 405-410, 414-441

crates/vm/src/builtins/weakproxy.rs (1)

81-84: PyWeakProxy::str updated to new receiver style without changing behavior

Switching to fn __str__(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyStrRef> and delegating with zelf.try_upgrade(vm)?.str(vm) aligns with the new Py<Self>-based convention while preserving existing semantics for live and dead proxies.

crates/vm/src/exception_group.rs (1)

185-203: BaseExceptionGroup::str correctly migrated to PyStrRef with preserved semantics

The new __str__:

Safely stringifies arg[0] via .str(vm) with a default empty message,

Counts sub-exceptions from arg[1] when it’s a PyTuple,

Produces "message (N sub-exception[s])" as a PyStrRef via vm.ctx.new_str.

This matches the intended ExceptionGroup string representation while conforming to the new &Py<PyBaseException> / PyStrRef pattern.

crates/vm/src/stdlib/winreg.rs (1)

12-12: PyHkey stringification and PyStr usage look good; double‑check HKEY/{:p} compatibility

The new __str__ signature (fn __str__(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyRef<PyStr>>) and body (vm.ctx.new_str(format!("<PyHkey:{:p}>", zelf.hkey.load()))) are consistent with the rest of the VM’s __str__ changes and produce the expected textual handle representation.

The py2reg updates to use PyStr in downcasts (downcast::<PyStr>(), downcast_ref::<PyStr>()) are straightforward and match the new import, with no behavior change.

One thing to verify: depending on the windows-sys version in use, Registry::HKEY may be an isize rather than a raw pointer type. In that case, format!("<PyHkey:{:p}>", zelf.hkey.load()) would fail to compile because {:p} requires a pointer. If you do run into a Pointer-trait compile error, consider either:

Casting to a raw pointer (e.g., zelf.hkey.load() as *mut core::ffi::c_void), or

Formatting as a hex integer instead (e.g., "0x{:x}" with as usize).

Also applies to: 236-238, 1032-1035, 1050-1052

crates/vm/src/builtins/str.rs (1)

1452-1460: LGTM!

The signature change from self to zelf: &Py<Self> and the PyResult<PyStrRef> return type aligns well with the broader refactor pattern across this PR. The logic correctly distinguishes between exact str types (returning self) and subclasses (constructing a new exact PyStr), which is the expected Python behavior.

crates/vm/src/exceptions.rs (8)

562-636: LGTM!

Adding Py to the with(...) clause correctly enables the separate impl Py<PyBaseException> block pattern, which is required for the new __str__ signature using the &Py<Self> receiver.

638-649: LGTM!

The new impl Py<PyBaseException> block correctly implements __str__ with the &self receiver pattern. The logic properly handles the three cases: empty args (empty string), single arg (direct string), and multiple args (tuple-like format). This provides a clean base implementation that subclasses can delegate to.

1533-1543: LGTM!

The PyKeyError::__str__ correctly maintains its special behavior (using repr for single-key errors) while properly delegating to the base __str__ implementation for other cases. The ? operator correctly propagates any errors from the base method.

1737-1826: LGTM!

The PyOSError::__str__ implementation correctly handles all the complex formatting cases for OS errors (errno, strerror, filenames, Windows winerror) while properly delegating to the base __str__ as a fallback. The attribute access via zelf.as_object() is consistent with the new receiver pattern.

2032-2076: LGTM!

The PySyntaxError::__str__ correctly formats syntax error messages with optional filename and line number information, falling back to the base __str__ implementation when the args format doesn't match expectations.

2184-2210: LGTM!

The PyUnicodeDecodeError::__str__ correctly formats decode error messages with encoding, byte position, and reason. The early return for missing object attribute is a sensible defensive measure.

2241-2268: LGTM!

The PyUnicodeEncodeError::__str__ correctly formats encode error messages, using UnicodeEscapeCodepoint for proper character escape display. The implementation mirrors the decode error pattern appropriately.

2298-2323: LGTM!

The PyUnicodeTranslateError::__str__ correctly formats translate error messages. Unlike encode/decode errors, translate errors don't have an encoding field, which is properly reflected in the message format.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

__repr__ with slot wrapper by youknowone · Pull Request #6486 · RustPython/RustPython

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

repr with slot wrapper by youknowone · Pull Request #6486 · RustPython/RustPython