Add tp_str by youknowone · Pull Request #6495 · RustPython/RustPython

youknowone

youknowone commented
Dec 24, 2025
•

edited by coderabbitai Bot

Loading

Summary by CodeRabbit

Refactor
- Streamlined the internal architecture of string representation handling within the virtual machine, including improved slot management for consistency and maintainability across core functionality.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

📝 Walkthrough

Walkthrough

This PR adds __str__ slot support to the RustPython VM by introducing a new Str variant to the SlotFunc enum, adding a corresponding str field to PyTypeSlots, converting the base object's __str__ method to use the slot-based mechanism, and wiring the slot through the class implementation machinery with refactored hash handling.

Changes

Cohort / File(s)	Summary
Slot Function Variant `crates/vm/src/builtins/descriptor.rs`	Added `Str(StringifyFunc)` variant to `SlotFunc` enum; updated `Debug` impl; consolidated `Repr` and `Str` call logic with shared argument validation and dispatch.
Type Slots Registry `crates/vm/src/types/slot.rs`	Added `str: AtomicCell<Option<StringifyFunc>>` field to `PyTypeSlots` struct to explicitly track the `__str__` slot.
Base Object Implementation `crates/vm/src/builtins/object.rs`	Converted `__str__` from `#[pymethod]` with `PyObjectRef` receiver to `#[pyslot]` slot-based `slot_str` with `&PyObject` receiver; logic remains unchanged.
Class Slot Wiring `crates/vm/src/class.rs`	Added `__str__` slot wrapper via `add_slot_wrapper!` macro; refactored `__hash__` handling from nested pattern matching to `.load().map_or(...)` for consistency.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

__hash__ to slot_wrapper #6480 — Foundational PR that introduced the SlotFunc enum and slot-wrapper machinery; this PR extends it with __str__ support.
__repr__ with slot wrapper #6486 — Adds Repr(StringifyFunc) variant to SlotFunc; directly related as this PR adds the parallel Str variant and unifies their call handling.
iter with slot-wrapper #6488 — Also extends slot-wrapper machinery by adding new SlotFunc variants for different slot types; shares the same architectural pattern.

Suggested reviewers

ShaharNaveh

🐰 A slot for strings now shines so bright,
The __str__ path slots into place just right,
With Stringify threads through the machine,
The rabbit approves of this design so clean! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add tp_str' directly and accurately describes the main change: introducing a new tp_str (string representation) slot to the RustPython VM, which is implemented across multiple files to support str slot-based method binding.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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

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

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/vm/src/types/slot.rs (1)
488-873: Critical: Missing update_slot case for __str__.

The update_slot function handles dynamic slot updates when attributes are added/removed from types, but there's no case for __str__. This means when a heap type dynamically defines or removes __str__, the str slot won't be updated, potentially breaking the slot mechanism for dynamically defined __str__ methods.

Add a case similar to __repr__ (lines 570-572):
🔎 Proposed fix
            _ if name == identifier!(ctx, __repr__) => {
                update_slot!(repr, repr_wrapper);
            }
+           _ if name == identifier!(ctx, __str__) => {
+               toggle_slot!(str, str_wrapper);
+           }
            _ if name == identifier!(ctx, __hash__) => {
Note: You'll also need to define str_wrapper function similar to repr_wrapper (line 351-359) if it doesn't already exist:
fn str_wrapper(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyRef<PyStr>> {
    let ret = vm.call_special_method(zelf, identifier!(vm, __str__), ())?;
    ret.downcast::<PyStr>().map_err(|obj| {
        vm.new_type_error(format!(
            "__str__ returned non-string (type {})",
            obj.class()
        ))
    })
}

📜 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 ebdc033 and ed736da.

⛔ Files ignored due to path filters (1)

Lib/test/test_inspect/test_inspect.py is excluded by !Lib/**

📒 Files selected for processing (4)

crates/vm/src/builtins/descriptor.rs
crates/vm/src/builtins/object.rs
crates/vm/src/class.rs
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/builtins/descriptor.rs
crates/vm/src/types/slot.rs
crates/vm/src/builtins/object.rs
crates/vm/src/class.rs

🧬 Code graph analysis (1)

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

crates/vm/src/types/slot.rs (4)

iter (1440-1440)

iter (1476-1478)

hash_not_implemented (374-376)

hash (1103-1103)

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

hash (438-461)

hash (1155-1173)

⏰ 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). (2)

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

🔇 Additional comments (5)

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

143-143: LGTM: str slot field added correctly.

The new str field follows the same pattern as the existing repr slot on line 144.

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

402-402: LGTM: Str variant added correctly.

The new Str(StringifyFunc) variant properly extends the SlotFunc enum to support the __str__ slot.

438-449: LGTM: Unified handling of Repr and Str improves maintainability.

The refactored approach consolidates the argument validation and dispatch logic for both __repr__ and __str__, reducing code duplication while correctly differentiating error messages.

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

168-168: LGTM: __str__ slot wrapper added correctly.

The slot wrapper follows the same pattern as __repr__ on line 167 and properly integrates the new Str slot with the class implementation machinery.

173-177: LGTM: Cleaner __hash__ handling.

The refactored approach using map_or and pointer comparison is more concise and readable than the previous implementation, while maintaining the same behavior.

Comment on lines +428 to 432

		#[pyslot]
		fn slot_str(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyStrRef> {
		// FIXME: try tp_repr first and fallback to object.__repr__
		zelf.repr(vm)
		}

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Minor: Clarify the FIXME comment.

The conversion to a slot-based implementation is correct, but the FIXME comment is confusing. It mentions "try tp_repr first" but this is the __str__ slot implementation. While the fallback to repr is correct per CPython behavior (where object.__str__ calls object.__repr__), the comment should clarify this intent.

🔎 Suggested comment improvement

-    // FIXME: try tp_repr first and fallback to object.__repr__
+    // Default __str__ implementation falls back to __repr__
+    // CPython behavior: object.__str__() calls object.__repr__()
     fn slot_str(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyStrRef> {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	#[pyslot]
	fn slot_str(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyStrRef> {
	// FIXME: try tp_repr first and fallback to object.__repr__
	zelf.repr(vm)
	}
	#[pyslot]
	fn slot_str(zelf: &PyObject, vm: &VirtualMachine) -> PyResult<PyStrRef> {
	// Default __str__ implementation falls back to __repr__
	// CPython behavior: object.__str__() calls object.__repr__()
	zelf.repr(vm)
	}

🤖 Prompt for AI Agents

In crates/vm/src/builtins/object.rs around lines 428 to 432, the FIXME comment
is misleading because this is the __str__ slot implementation and should clarify
that we call the object's str (tp_str) and, per CPython semantics for
object.__str__, fall back to object.__repr__ via zelf.repr(vm); update the
comment to explicitly state "This is the __str__ slot: use tp_str, and fall back
to object.__repr__ (implemented here by calling zelf.repr(vm))" so future
readers understand the intent.

This was referenced

Mar 19, 2026

Add tp_str by youknowone · Pull Request #6495 · RustPython/RustPython

youknowone commented Dec 24, 2025 • edited by coderabbitai Bot Loading

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowone commented
Dec 24, 2025
•

edited by coderabbitai Bot

Loading