Allow with() in pyexception by youknowone · Pull Request #6436 · RustPython/RustPython

youknowone

Walkthrough

The PR modifies the #[pyexception] derive macro to detect and honor with(Constructor) clauses in attributes. It introduces conditional slot generation flags, accumulates extra attributes for forwarding, and wires them into the generated code while removing previous restrictions on attributes.

Changes

Cohort / File(s)	Change Summary
#[pyexception] derive macro enhancement `crates/derive-impl/src/pyclass.rs`	Reworks attribute processing to detect `with(Constructor)` clause, introduce `has_slot_new` flag, and accumulate `extra_attrs`. Removes unconditional attribute errors and wires extra attributes into generated `#[pyclass(...)]` via `extra_attrs_tokens`. Marks slot generation contingent on flags and adds TODO for Constructor/Initializer handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Attribute parsing and accumulation logic for with(...) clauses requires careful verification
Conditional slot generation based on has_slot_new and has_slot_init flags—verify logic paths and backward compatibility
extra_attrs_tokens wiring into generated code—ensure attributes propagate correctly without breaking existing implementations
Macro code generation can be error-prone; pay special attention to token stream handling

Possibly related PRs

RustPython/RustPython#6218: Also modifies #[pyexception] derive in derive-impl/src/pyclass.rs, changing generated slot visibility and PyPayload path—may interact with these attribute/slot changes.

Suggested reviewers

coolreader18
ShaharNaveh

Poem

🐰 A macro so clever, with slots to arrange,
Constructor and attributes in harmonious change,
With flags now conditional, the logic runs true,
Extra attrs dancing—old errors bid adieu! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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 'Allow with() in pyexception' directly and clearly describes the main change: enabling the with() clause in pyexception declarations, which aligns with the changeset's core objective.

✨ 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 db71554 and da8dfad.

📒 Files selected for processing (1)

crates/derive-impl/src/pyclass.rs (3 hunks)

🧰 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/derive-impl/src/pyclass.rs

🧬 Code graph analysis (1)

crates/derive-impl/src/pyclass.rs (2)

crates/derive-impl/src/from_args.rs (1)

meta (95-95)

crates/derive-impl/src/lib.rs (1)

pyclass (41-47)

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

GitHub Check: Run rust tests (macos-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Run tests under miri
GitHub Check: Check Rust code with clippy
GitHub Check: Run snippets and cpython tests on wasm-wasi
GitHub Check: Ensure compilation on various targets
GitHub Check: Check the WASM package and demo

🔇 Additional comments (4)

crates/derive-impl/src/pyclass.rs (4)

630-630: TODO comment indicates incomplete implementation.

The comment notes that slot_new and slot_init should be properly constrained by Constructor or Initializer traits in the future. This suggests the current implementation is a work-in-progress that will require additional validation or constraints in subsequent changes.

668-672: LGTM!

The conditional generation of extra_attrs_tokens is correct. The leading comma on line 671 properly integrates the extra attributes into the existing #[pyclass(...)] attribute list.

675-675: LGTM!

The modification correctly forwards the accumulated with(...) attributes to the generated #[pyclass(...)] macro, enabling proper attribute propagation from #[pyexception] impl blocks.

582-604: Remove the reference to Unconstructible — it is not a supported pattern in the codebase and does not appear in the implementation.

The review comment correctly identifies that the code only checks for Constructor when determining whether to skip generating default slot_new. However, the claim that extract_impl_attrs treats both Constructor and Unconstructible similarly is incorrect—Unconstructible is not a supported attribute pattern in this codebase. The concern about Unconstructible should be removed from the review comment.

The actual concern worth noting is that the code only handles Constructor from the with(...) attribute, which appears to be the only supported constructor trait variant for pyexception at this time. This is not an oversight but rather the intended behavior for the current implementation.

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