◐ Shell
clean mode source ↗

Complete tp_repr by youknowone · Pull Request #5852 · RustPython/RustPython

coderabbitai

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5ccd4f and d0ae853.

📒 Files selected for processing (2)
  • vm/src/protocol/object.rs (1 hunks)
  • vm/src/types/structseq.rs (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
vm/src/protocol/object.rs (2) 
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: Follow Rust best practices for error handling and memory management in all RustPython Rust code.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: Use the macro system (such as 'pyclass', 'pymodule', 'pyfunction') when implementing Python functionality in Rust for RustPython.
vm/src/types/structseq.rs (5) 
Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: Use the macro system (such as 'pyclass', 'pymodule', 'pyfunction') when implementing Python functionality in Rust for RustPython.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: When implementing a Python module in Rust for RustPython, use the #[pymodule] attribute and expose functions and classes using #[pyfunction], #[pyclass], and #[pymethod] as appropriate.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: Follow Rust best practices for error handling and memory management in all RustPython Rust code.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: When comparing behavior with CPython, use the 'python' command explicitly for CPython and 'cargo run --' for RustPython to avoid confusion.

Learnt from: CR
PR: RustPython/RustPython#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-06-26T12:52:11.109Z
Learning: To add a Python module to the RustPython interpreter, use 'vm.add_native_module' with the module name and module factory.
⏰ Context from checks skipped due to timeout of 90000ms (6)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run tests under miri
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Check the WASM package and demo
🔇 Additional comments (5)
vm/src/protocol/object.rs (1)

335-338: Verify this breaking change is intentional and won't cause runtime failures.

The removal of the fallback mechanism for missing __repr__ slots is a significant behavioral change. This now enforces that every object must have a properly defined repr slot, which could cause runtime errors for existing objects that previously relied on the dynamic fallback.

Please verify that all built-in types and user-defined classes have proper repr slots defined:

#!/bin/bash
# Description: Search for pyclass definitions that might be missing repr slots
# Expected: All classes should have either explicit repr slots or inherit them

echo "Searching for pyclass definitions..."
ast-grep --pattern $'#[pyclass]
$$$
trait $NAME {
  $$$
}'

echo -e "\nSearching for explicit slot_repr implementations..."
rg -A 5 "slot_repr|#\[pyslot\].*repr"

echo -e "\nSearching for __repr__ method implementations..."
rg -A 3 "fn __repr__"
vm/src/types/structseq.rs (4)

2-3: Import changes look appropriate.

The addition of PyObject and PyStrRef imports aligns with the new slot-based approach that returns PyStrRef and accepts generic &PyObject parameters.

60-76: LGTM! Recursion guard and formatting logic preserved correctly.

The refactored code maintains the original logic for:

  • Recursion detection using ReprGuard
  • Special handling for single required field with trailing comma
  • Proper field formatting with name-value pairs

The change from zelf.as_ref() is appropriate for the new parameter type.

77-79: Return type change aligns with slot requirements.

The change from returning a String to PyStrRef is correct for the slot-based approach and maintains consistency with other slot implementations.

81-84: Simplified wrapper method is clean and consistent.

The __repr__ method is now appropriately simplified to just delegate to the slot implementation, which follows the established pattern for slot-based methods.