Conversation
…o __devirt_define! Rewrites expand_trait and expand_impl to generate the dispatch code directly, eliminating the macro_rules pattern-matching bottleneck. This immediately enables support for unsafe trait/fn, method lifetime params, method attributes (#[must_use], doc comments), supertraits, extern "ABI" fn, and method where clauses — all parsed by syn and emitted faithfully via quote!. The declarative macro path (__devirt_define!) is unchanged and keeps its current limited feature set as the default-features=false fallback. New trybuild tests: attr_unsafe_fn, attr_method_lifetimes, attr_supertraits, attr_must_use (pass); attr_unsafe_missing_on_impl (compile-fail). Extended equivalence test with supertraits, method lifetimes, and #[must_use] coverage. https://claude.ai/code/session_01KeSis2aTgupBqgw9RtWXXZ
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughProc-macro Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer source
participant Proc as Proc-macro (#[devirt])
participant Decl as Declarative macro (devirt!)
participant Gen as Generated dispatch code
participant Runtime as Runtime (vtable/data)
Dev->>Proc: annotate trait/impl with #[devirt]
Proc->>Gen: emit full dispatch code via quote! (inner trait, marker, dyn helpers)
Dev->>Decl: use devirt! macro (older path)
Decl->>Gen: expand to __devirt_define! (limited plain-fn signatures)
Gen->>Runtime: runtime uses vtable/data extraction and hot-type matching
Runtime->>Gen: call forwarding to concrete impl (__spec_* methods)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/lib.rs`:
- Around line 33-38: The doc comment claims the proc-macro attribute `#[devirt]`
emits dispatch code directly, which violates the repo rule that all dispatch
expansion must be centralized in the hidden, exported declarative macro
`__devirt_define!`; fix by restoring delegation so `#[devirt]` and the `devirt!`
declarative macro both call `__devirt_define!` (which must have `#[doc(hidden)]`
and `#[macro_export]` and support the `@trait` and `@impl` entry points), or
alternatively update this comment to explicitly state the new architecture and
add/update `__devirt_define!` to match the documented contract — ensure
references to `#[devirt]`, `devirt!`, and `__devirt_define!` in the code and
docs are consistent.
In `@crates/core/tests/ui_attr/attr_must_use.rs`:
- Around line 11-13: The test currently masks must_use warnings by consuming
return values; add a new compile-fail case that ensures #[must_use] on the trait
method compute is preserved through macro expansion by creating a test variant
with #![deny(unused_must_use)] and an expression that calls compute() without
assigning or using its result (i.e., omit the let _ = ...) so the compiler must
emit an unused_must_use error; reference the trait/method name compute and the
existing attr_must_use.rs test when adding this negative test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e4373048-3888-4ae2-89b9-e08073d10667
📒 Files selected for processing (10)
crates/core/src/lib.rscrates/core/tests/equivalence.rscrates/core/tests/ui_attr.rscrates/core/tests/ui_attr/attr_method_lifetimes.rscrates/core/tests/ui_attr/attr_must_use.rscrates/core/tests/ui_attr/attr_supertraits.rscrates/core/tests/ui_attr/attr_unsafe_fn.rscrates/core/tests/ui_attr/attr_unsafe_missing_on_impl.rscrates/core/tests/ui_attr/attr_unsafe_missing_on_impl.stderrcrates/macros/src/lib.rs
Comment on lines
+33
to
+38
| //! The proc-macro attribute (`#[devirt]`) emits the dispatch code | ||
| //! directly via `quote!`, supporting the full range of method | ||
| //! signatures that `syn` can parse (lifetimes, `unsafe fn`, | ||
| //! supertraits, attributes, `extern "ABI" fn`, etc.). | ||
| //! The declarative macro (`devirt!`) delegates to `__devirt_define!`, | ||
| //! which has more limited syntax support. |
There was a problem hiding this comment.
Architecture contract drift from the repository macro-expansion rule.
Line 33 through Line 38 document direct proc-macro dispatch emission instead of routing through __devirt_define!. That introduces two expansion engines and diverges from the repo’s stated single-source expansion contract; please either restore delegation or update the project rule/docs in the same change set.
As per coding guidelines "crates/core/src/**/*.rs: Place all dispatch expansion logic in __devirt_define! macro with #[doc(hidden)], #[macro_export] attributes, and implement two entry points: @trait ... and @impl ...".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/core/src/lib.rs` around lines 33 - 38, The doc comment claims the
proc-macro attribute `#[devirt]` emits dispatch code directly, which violates
the repo rule that all dispatch expansion must be centralized in the hidden,
exported declarative macro `__devirt_define!`; fix by restoring delegation so
`#[devirt]` and the `devirt!` declarative macro both call `__devirt_define!`
(which must have `#[doc(hidden)]` and `#[macro_export]` and support the `@trait`
and `@impl` entry points), or alternatively update this comment to explicitly
state the new architecture and add/update `__devirt_define!` to match the
documented contract — ensure references to `#[devirt]`, `devirt!`, and
`__devirt_define!` in the code and docs are consistent.
…ompile-fail test - CLAUDE.md: Document that the proc-macro attribute emits directly via quote! while only the declarative macro delegates to __devirt_define!. Update Macro Structure section to describe two independent expansion paths and clarify Dispatch Arms apply to the declarative-macro path only. - Add attr_must_use_unused.rs compile-fail test: verifies #[must_use] propagates through macro expansion by calling compute() without using its result under #![deny(unused_must_use)], proving the attribute reaches the inherent dispatch method on dyn Trait. https://claude.ai/code/session_01KeSis2aTgupBqgw9RtWXXZ
This was referenced Apr 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Rewrites expand_trait and expand_impl to generate the dispatch code
directly, eliminating the macro_rules pattern-matching bottleneck.
This immediately enables support for unsafe trait/fn, method lifetime
params, method attributes (#[must_use], doc comments), supertraits,
extern "ABI" fn, and method where clauses — all parsed by syn and
emitted faithfully via quote!.
The declarative macro path (__devirt_define!) is unchanged and keeps
its current limited feature set as the default-features=false fallback.
New trybuild tests: attr_unsafe_fn, attr_method_lifetimes,
attr_supertraits, attr_must_use (pass); attr_unsafe_missing_on_impl
(compile-fail). Extended equivalence test with supertraits, method
lifetimes, and #[must_use] coverage.