GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Allow with() in pyexception by youknowone · Pull Request #6436 · RustPython/RustPython

Conversation

@youknowone

@youknowone youknowone commented Dec 14, 2025 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • PyException declarations now support with(Constructor) clause for enhanced customization
    • Extra attributes in with(...) arguments are now properly preserved and propagated

  • Bug Fixes

    • Removed overly restrictive validation that previously rejected attributes on PyException implementations

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@youknowone
Allow with() in pyexception
da8dfad
@youknowone
youknowone marked this pull request as ready for review December 14, 2025 02:52
@coderabbitai

coderabbitai Bot commented Dec 14, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands and usage tips.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@youknowone
youknowone merged commit 21300f6 into RustPython:main Dec 14, 2025
13 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@youknowone
youknowone deleted the pyexception-with branch December 14, 2025 03:16
@coderabbitai coderabbitai Bot mentioned this pull request Dec 22, 2025
disallow __new__, __init__ #6446
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@youknowone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.