◐ Shell
reader mode source ↗
Skip to content

init debug helper#6315

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:init-debug
Nov 30, 2025
Merged

init debug helper#6315
youknowone merged 1 commit into
RustPython:mainfrom
youknowone:init-debug

Conversation

@youknowone

@youknowone youknowone commented Nov 30, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

#4884

Summary by CodeRabbit

  • Chores
    • Improved debug-only error messages to give clearer guidance when object initialization fails and when subclassing support may be limited.
    • Added a non-functional inline note highlighting a potential safety concern related to the instance type.

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

@coderabbitai

coderabbitai Bot commented Nov 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

Adds debug-only diagnostics to downcast/init error paths in two VM modules: captures class names, inspects error messages for duplicate class occurrences, and augments/panics with a suggestion about types that may not support init or subclassing.

Changes

Cohort / File(s) Summary
Debug error diagnostics
crates/vm/src/types/slot.rs		 In Initializer::slot_init, capture the class name when Zelf downcast fails; in debug builds, reprint the error, check for the class name appearing twice, and panic with a message suggesting the type may not support init (prompting to override slot_init) when detected. Adds a non-functional safety comment in __init__.
Enhanced downcast error messages
crates/vm/src/vm/vm_new.rs		 In debug assertion path for downcast errors where actual and expected class match, prefix the existing reminder about #[pyclass(with(Constructor))]? with an additional note suggesting the type may not support subclassing well. Only the diagnostic text changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Inspect debug-only conditional and panic condition in crates/vm/src/types/slot.rs.
  • Verify the updated diagnostic wording in crates/vm/src/vm/vm_new.rs for clarity and correctness.

Poem

🐰 I nibble logs by lantern light,
Two names echo — what a sight!
A helpful panic, a gentle nudge,
So types behave and errors budge.
Hoppity hop, the fix is bright.

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 'init debug helper' is concise and clearly describes the main changes: adding debug-only helper logic to the init path for improved error diagnostics.
Docstring Coverage ✅ Passed Docstring coverage is 100.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
📜 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 080a2f6 and d5af480.

📒 Files selected for processing (2)
  • crates/vm/src/types/slot.rs (1 hunks)
  • crates/vm/src/vm/vm_new.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/vm/src/vm/vm_new.rs
  • crates/vm/src/types/slot.rs
⏰ 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 snippets and cpython tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Check Rust code with clippy
  • GitHub Check: Run tests under miri
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Ensure compilation on various targets

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.

@youknowone youknowone marked this pull request as ready for review November 30, 2025 02:09
coderabbitai[bot]
coderabbitai Bot reviewed Nov 30, 2025
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Actionable comments posted: 1

🧹 Nitpick comments (1)
crates/vm/src/types/slot.rs (1)

866-866: TODO comment lacks specificity.

The comment mentions a potential safety concern about zelf but doesn't explain what the concern is or under what conditions it might be unsafe. This makes it difficult to evaluate or address the issue.

Please clarify the safety concern with more details. For example:

// TODO: check if this is safe. When __init__ is called on an uninitialized or 
// incorrectly typed object, zelf might not have the expected payload type. 
// Consider whether zelf should be `PyObjectRef` with explicit type checking.
📜 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 b84f7c1 and 080a2f6.

📒 Files selected for processing (2)
  • crates/vm/src/types/slot.rs (1 hunks)
  • crates/vm/src/vm/vm_new.rs (1 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/vm/src/types/slot.rs
  • crates/vm/src/vm/vm_new.rs
🧬 Code graph analysis (1)
crates/vm/src/types/slot.rs (2)
crates/vm/src/stdlib/ctypes/field.rs (2)
  • zelf (96-97)
  • zelf (196-197)
crates/derive-impl/src/error.rs (1)
  • panic (106-112)
⏰ 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 (1)
crates/vm/src/vm/vm_new.rs (1)

512-512: LGTM! Helpful debug diagnostic message.

The extended error message provides useful guidance for developers encountering downcast failures when class IDs match, pointing them toward the #[pyclass(with(Constructor))] attribute as a potential solution.

Hide details View details @youknowone youknowone merged commit 8e0a86d into RustPython:main Nov 30, 2025
13 checks passed
@youknowone youknowone deleted the init-debug branch November 30, 2025 07:58
@coderabbitai coderabbitai Bot mentioned this pull request Dec 16, 2025
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Dec 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone