Unify devirt APIs: devirt! macro + #[devirt] attribute (0.2.0)#14
Merged
Unify devirt APIs: devirt! macro + #[devirt] attribute (0.2.0)#14
Conversation
Replace `devirt::r#trait!` / `devirt::r#impl!` with a single `devirt` name that works as either a proc-macro attribute (default) or a declarative macro (with `default-features = false`). Both APIs delegate to `__devirt_define!`, a #[doc(hidden)] internal macro that holds all dispatch expansion logic. The `[hot]` marker on impls is removed — hot-path specialization is driven entirely by the trait's hot-type list. Breaking changes: - `devirt::r#trait!` and `devirt::r#impl!` removed - `[hot]` marker on impl blocks no longer accepted - Version bumped to 0.2.0 New crate structure: - crates/macros (devirt-macros): proc-macro crate with #[devirt] attr - crates/core: gains optional `macros` feature (on by default) Also updates LTO guidance: LTO is no longer required since all dispatch logic expands via macros into the user's crate. https://claude.ai/code/session_01XRfaF7hqTVzJwR8tBrnkom
The __devirt_define! macro used bare `T` as the generic parameter in __devirt_vtable_for and the blanket impl. When the user's trait was also named `T`, the generic parameter shadowed the trait name, causing a compile error. Rename both to `__DevirtT` to avoid collisions. Also updates the equivalence test to use trait name `T` to exercise this edge case. https://claude.ai/code/session_01XRfaF7hqTVzJwR8tBrnkom
📝 WalkthroughSummary by CodeRabbit
WalkthroughUnifies devirtualization behind an internal Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Code
participant Compiler as Rust Compiler
participant Attr as devirt::devirt (proc-macro)
participant Def as devirt::__devirt_define!
User->>Compiler: #[devirt(TypeA,TypeB)] trait T { ... }
Compiler->>Attr: trait TokenStream
Attr->>Attr: parse & validate hot types
Attr->>Def: emit ::devirt::__devirt_define! { `@trait` [...] ... }
Def->>Def: expand trait -> vtable, helpers, markers
Def-->>Compiler: expanded tokens
Compiler-->>User: compiled trait + dispatch
User->>Compiler: #[devirt] impl T for TypeA { ... }
Compiler->>Attr: impl TokenStream
Attr->>Attr: parse & validate (empty attr)
Attr->>Def: emit ::devirt::__devirt_define! { `@impl` [] T for TypeA { ... } }
Def->>Def: expand impl -> marker impls / method wiring
Def-->>Compiler: expanded tokens
Compiler-->>User: compiled impl
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/examples/shapes.rs (1)
16-33:⚠️ Potential issue | 🟠 MajorUse the public
devirtAPI in the example.This example is the user-facing
cargo run --example shapespath, but it now teachesdevirt::__devirt_define!, which is explicitly an internal#[doc(hidden)]hook. That means the example no longer exercises the supported surface for the default feature set, and it invites downstream users to depend on an implementation detail. Please switch this example to#[devirt::devirt](or cfg-split it if you also want a no-default-features variant).Also applies to: 37-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/core/examples/shapes.rs` around lines 16 - 33, The example currently invokes the internal macro devirt::__devirt_define! to define the Shape trait and its variants; replace that with the public API by annotating the trait with #[devirt::devirt] (or provide a cfg-split variant for no-default-features) so the example uses the supported surface: remove the devirt::__devirt_define! wrapper and declare the pub trait Shape { fn area(&self)->f64; fn perimeter(&self)->f64; fn scale(&mut self,f64); fn try_scale(&mut self,f64)->bool; fn describe(&self); fn name(&self)->&str; } with #[devirt::devirt] above it (keeping Circle and Rect implementations as-is) so the example targets the public devirt API.
🤖 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 51-56: The documentation examples for the declarative macro
devirt::devirt! are missing the required trait keyword; update both snippets
that declare MyTrait (the blocks using devirt::devirt! with MyTrait [HotType1,
HotType2] and the second example) to include "trait" (i.e., use "pub trait
MyTrait [...]") so the macro pattern matches and the examples compile when
copy-pasted.
In `@crates/core/tests/equivalence.rs`:
- Around line 54-70: Extend both decl_dispatch and attr_dispatch tests to
exercise all four unsafe dispatch arms instead of only the &self -> value path:
for the Hot and Cold instances used in decl::T and attr::T, add runtime
assertions that invoke the &self->value method (existing get()), the &self->void
method, the &mut->value method, and the &mut->void method so `@dispatch_void`,
`@dispatch_mut`, and `@dispatch_mut_void` code paths run through the proc-macro
surface; locate these additions inside the decl_dispatch (for cfg(not(feature =
"macros"))) and attr_dispatch (for cfg(feature = "macros")) functions
referencing Hot, Cold, and the dyn decl::T / dyn attr::T types.
In `@crates/macros/src/lib.rs`:
- Around line 98-141: expand_impl currently drops trait-path qualifiers, impl
generics/where-clauses, and method attributes when building the output; update
it to forward the full trait path (use the whole trait_path instead of
trait_path.segments.last().ident), include impl_item.generics and its
where_clause when emitting the impl header, and preserve each method's
attributes (include m.attrs when constructing method_bodies, similar to
expand_trait) so qualifiers like super::Trait, generics (impl<T>), where
clauses, and #[inline]/#[cfg] survive expansion; alternatively, if you cannot
support qualified paths or generics, make expand_impl emit a clear syn::Error
for non-plain cases (qualified trait paths, non-empty impl_item.generics, or
methods with attrs) instead of silently dropping them.
- Around line 50-95: expand_trait currently drops generics, where clauses,
supertraits, associated items, and default method bodies (and expand_impl drops
impl generics/where/attrs); add explicit validation in expand_trait (inspect
trait_item.generics, trait_item.supertraits, trait_item.items for
syn::TraitItem::Type/Const and for TraitItem::Fn with Some(block) default
bodies, and trait_item.generics.where_clause) and in expand_impl (inspect
syn::ItemImpl generics, self_ty, attrs, and where_clause) and when any
unsupported form is present return a compile error using syn::Error::new(span,
"...") .to_compile_error().into() with clear messages referencing the
unsupported feature (e.g. "devirt: generic traits are not supported", "devirt:
associated types/constants are not supported", "devirt: default method bodies
are not supported", "devirt: impl generics/where/attrs are not supported") so
the macro fails early rather than silently dropping elements; use the existing
trait_item/impl_item identifiers and proc_macro2::Span from the items for
accurate error spans.
---
Outside diff comments:
In `@crates/core/examples/shapes.rs`:
- Around line 16-33: The example currently invokes the internal macro
devirt::__devirt_define! to define the Shape trait and its variants; replace
that with the public API by annotating the trait with #[devirt::devirt] (or
provide a cfg-split variant for no-default-features) so the example uses the
supported surface: remove the devirt::__devirt_define! wrapper and declare the
pub trait Shape { fn area(&self)->f64; fn perimeter(&self)->f64; fn scale(&mut
self,f64); fn try_scale(&mut self,f64)->bool; fn describe(&self); fn
name(&self)->&str; } with #[devirt::devirt] above it (keeping Circle and Rect
implementations as-is) so the example targets the public devirt API.
🪄 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: b353d6a1-3391-47ed-a3b2-ce092807b2b0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
CLAUDE.mdCargo.tomlREADME.mdcrates/core/Cargo.tomlcrates/core/benches/dispatch.rscrates/core/examples/shapes.rscrates/core/src/lib.rscrates/core/tests/equivalence.rscrates/core/tests/kani.rscrates/core/tests/ui/all_arms.rscrates/core/tests/ui/missing_method.rscrates/core/tests/ui/missing_method.stderrcrates/core/tests/ui/multi_arg.rscrates/core/tests/ui/multi_hot.rscrates/core/tests/ui/pub_trait.rscrates/core/tests/ui/single_hot.rscrates/core/tests/ui/wrong_signature.rscrates/core/tests/ui/wrong_signature.stderrcrates/core/tests/ui_attr.rscrates/core/tests/ui_attr/attr_all_arms.rscrates/core/tests/ui_attr/attr_args_on_impl.rscrates/core/tests/ui_attr/attr_args_on_impl.stderrcrates/core/tests/ui_attr/attr_missing_args.rscrates/core/tests/ui_attr/attr_missing_args.stderrcrates/core/tests/ui_attr/attr_multi_hot.rscrates/core/tests/ui_attr/attr_on_struct.rscrates/core/tests/ui_attr/attr_on_struct.stderrcrates/core/tests/ui_attr/attr_single_hot.rscrates/macros/Cargo.tomlcrates/macros/src/lib.rsfuzz/fuzz_targets/dispatch.rs
- Examples, benchmarks, and fuzz target now use the public #[devirt::devirt] attribute API instead of __devirt_define! - Fix missing `trait` keyword in lib.rs doc example for devirt! - Extend equivalence test to exercise all four dispatch arms (&self->val, &self->void, &mut->val, &mut->void) - Add proc-macro validation: reject generic traits/impls, where clauses, supertraits, associated types/constants, and default method bodies with clear error messages - Forward method attributes (#[inline], #[cfg], etc.) through expand_impl instead of silently dropping them https://claude.ai/code/session_01XRfaF7hqTVzJwR8tBrnkom
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/lib.rs (1)
238-258:⚠️ Potential issue | 🟠 MajorFix
@impl,@spec_decl, and@inherent_declto handle method attributes forwarded by the proc-macro.The proc-macro forwards trait and impl method attributes to
__devirt_define!, but the macro patterns cannot handle them:
@implexpectsfn $method(...)but receives#[attr] fn $method(...), causing the pattern to fail.@spec_declsilently discards trait method attributes ($(#[$_attr:meta])*is never emitted).@inherent_decloverwrites method attributes with hardcoded#[inline]and#[doc(hidden)], discarding user-provided attributes.Either thread attributes through to generated methods (preserving user intent) or filter them out in the proc-macro before expanding.
🤖 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 238 - 258, The macro patterns must accept and forward method attributes instead of dropping or overwriting them: update the `@impl`, `@spec_decl`, and `@inherent_decl` arms in __devirt_define! to include an optional $(#[$_attr:meta])* matcher on method productions (e.g. match $(#[$_attr:meta])* fn $method(...)) and emit those attributes when generating the helper methods (e.g. attach $(#[$_attr])* to the generated [<__spec_ $method>] or impl-method definitions) rather than hardcoding #[inline] and #[doc(hidden)]; alternatively, if attributes must be filtered earlier, have the proc-macro strip them before calling __devirt_define! so these macro arms only see the clean fn form. Ensure the unique symbols `@impl`, `@spec_decl`, `@inherent_decl`, and the generated names like [<__spec_ $method>] are updated to preserve and re-emit user-provided attributes.
🤖 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/macros/src/lib.rs`:
- Around line 60-115: Add explicit checks rejecting unsafe traits and unsafe
impls in expand_trait and expand_impl: inspect the ItemTrait::unsafety field in
expand_trait and the ItemImpl::unsafety field in expand_impl at the top of each
function (before the existing generic/supertrait/associated-item validations)
and return a compile_error
(syn::Error::new_spanned(...).to_compile_error().into()) with a message like
"#[devirt] does not support unsafe traits" or "#[devirt] does not support unsafe
impls" when unsafety.is_some(); this prevents silently dropping unsafety when
generating the safe macro output.
---
Outside diff comments:
In `@crates/core/src/lib.rs`:
- Around line 238-258: The macro patterns must accept and forward method
attributes instead of dropping or overwriting them: update the `@impl`,
`@spec_decl`, and `@inherent_decl` arms in __devirt_define! to include an optional
$(#[$_attr:meta])* matcher on method productions (e.g. match $(#[$_attr:meta])*
fn $method(...)) and emit those attributes when generating the helper methods
(e.g. attach $(#[$_attr])* to the generated [<__spec_ $method>] or impl-method
definitions) rather than hardcoding #[inline] and #[doc(hidden)]; alternatively,
if attributes must be filtered earlier, have the proc-macro strip them before
calling __devirt_define! so these macro arms only see the clean fn form. Ensure
the unique symbols `@impl`, `@spec_decl`, `@inherent_decl`, and the generated names
like [<__spec_ $method>] are updated to preserve and re-emit user-provided
attributes.
🪄 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: 8edb48de-f5c7-4140-96d5-dbf87fd9a940
📒 Files selected for processing (6)
crates/core/benches/dispatch.rscrates/core/examples/shapes.rscrates/core/src/lib.rscrates/core/tests/equivalence.rscrates/macros/src/lib.rsfuzz/fuzz_targets/dispatch.rs
Comment on lines
+60
to
+115
| // Reject unsupported trait features. | ||
| if !trait_item.generics.params.is_empty() { | ||
| return syn::Error::new_spanned( | ||
| &trait_item.generics, | ||
| "#[devirt] does not support generic traits", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| if let Some(where_clause) = &trait_item.generics.where_clause { | ||
| return syn::Error::new_spanned( | ||
| where_clause, | ||
| "#[devirt] does not support where clauses on traits", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| if !trait_item.supertraits.is_empty() { | ||
| return syn::Error::new_spanned( | ||
| &trait_item.supertraits, | ||
| "#[devirt] does not support supertraits", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| for item in &trait_item.items { | ||
| match item { | ||
| syn::TraitItem::Type(t) => { | ||
| return syn::Error::new_spanned( | ||
| t, | ||
| "#[devirt] does not support associated types", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| syn::TraitItem::Const(c) => { | ||
| return syn::Error::new_spanned( | ||
| c, | ||
| "#[devirt] does not support associated constants", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| syn::TraitItem::Fn(f) => { | ||
| if f.default.is_some() { | ||
| return syn::Error::new_spanned( | ||
| f, | ||
| "#[devirt] does not support default method bodies", | ||
| ) | ||
| .to_compile_error() | ||
| .into(); | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '60,115p' crates/macros/src/lib.rs
sed -n '165,191p' crates/macros/src/lib.rs
sed -n '124,132p' crates/core/src/lib.rs
sed -n '513,524p' crates/core/src/lib.rsRepository: Kab1r/devirt
Length of output: 3263
Reject unsafe trait and unsafe impl with explicit validation.
The expand_trait and expand_impl functions do not validate the unsafety field on ItemTrait and ItemImpl. Since the generated macros only emit safe forms, #[devirt] unsafe trait T { ... } would silently expand into a safe trait, dropping the safety contract instead of failing. Add checks to reject these unsupported cases before other validations:
Guard checks for unsafety
fn expand_trait(attr: TokenStream, trait_item: &syn::ItemTrait) -> TokenStream {
if attr.is_empty() {
return syn::Error::new(
proc_macro2::Span::call_site(),
"expected hot types: #[devirt(Type1, Type2)]",
)
.to_compile_error()
.into();
}
// Reject unsupported trait features.
+ if let Some(unsafety) = &trait_item.unsafety {
+ return syn::Error::new_spanned(
+ unsafety,
+ "#[devirt] does not support unsafe traits",
+ )
+ .to_compile_error()
+ .into();
+ }
if !trait_item.generics.params.is_empty() { // Reject unsupported impl features.
+ if let Some(unsafety) = &impl_item.unsafety {
+ return syn::Error::new_spanned(
+ unsafety,
+ "#[devirt] does not support unsafe impl blocks",
+ )
+ .to_compile_error()
+ .into();
+ }
if !impl_item.generics.params.is_empty() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/macros/src/lib.rs` around lines 60 - 115, Add explicit checks
rejecting unsafe traits and unsafe impls in expand_trait and expand_impl:
inspect the ItemTrait::unsafety field in expand_trait and the ItemImpl::unsafety
field in expand_impl at the top of each function (before the existing
generic/supertrait/associated-item validations) and return a compile_error
(syn::Error::new_spanned(...).to_compile_error().into()) with a message like
"#[devirt] does not support unsafe traits" or "#[devirt] does not support unsafe
impls" when unsafety.is_some(); this prevents silently dropping unsafety when
generating the safe macro output.
Bug 1: `#[devirt(Foo)] unsafe trait Bar` silently dropped `unsafe`, generating a safe trait. Fix: parameterize `__devirt_define!` with `[$($unsafety:tt)*]` and emit it on the inner trait, public trait, blanket impl, and user impl. Bug 2: Method attributes (`#[must_use]`, doc comments, `#[deprecated]`) were parsed but not emitted in `@inherent_decl` and `@impl` arms. Fix: forward `$(#[$attr])*` in all four `@inherent_decl` arms and add `$(#[$method_attr:meta])*` to the `@impl` arm. Also: - Add `unsafe trait`/`unsafe impl` arms to `devirt!` declarative macro - Forward unsafety through proc-macro `expand_trait`/`expand_impl` - Gate example/bench on `required-features = ["macros"]` so `--no-default-features` doesn't try to compile attribute-using code - Fix doc example missing `trait` keyword in `devirt!` macro - Add trybuild tests for both features (ui + ui_attr) - Update all ~38 existing `__devirt_define!` call sites with `[]` https://claude.ai/code/session_01XRfaF7hqTVzJwR8tBrnkom
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/macros/src/lib.rs (1)
193-195:⚠️ Potential issue | 🟡 MinorQualified trait paths are silently truncated, causing incorrect scope resolution.
Line 194 extracts only the final identifier from the trait path (
trait_path.segments.last()...ident), soimpl super::MyTrait for Fooexpands to@impl [] MyTrait for Foo { ... }. This refers toMyTraitin the local scope rather thansuper::MyTrait, potentially causing confusing errors or silent miscompilation if both exist.Since
__devirt_define!'s@implpattern only accepts$trait_name:ident(not a path), the proc-macro should reject qualified paths explicitly rather than silently truncating them.🛡️ Proposed fix: reject qualified trait paths
let Some((_, trait_path, _)) = &impl_item.trait_ else { return syn::Error::new( proc_macro2::Span::call_site(), "#[devirt] requires `impl Trait for Type`, not a bare impl block", ) .to_compile_error() .into(); }; + // Reject qualified trait paths (e.g., super::Trait, crate::Trait) + if trait_path.segments.len() > 1 { + return syn::Error::new_spanned( + trait_path, + "#[devirt] does not support qualified trait paths; use just the trait name", + ) + .to_compile_error() + .into(); + } + // Reject unsupported impl features.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/macros/src/lib.rs` around lines 193 - 195, The code currently truncates qualified trait paths by taking only trait_path.segments.last() into trait_name, causing `impl super::MyTrait for Foo` to be treated as `MyTrait`; change this to explicitly reject qualified paths: in the code that handles the trait in the impl (referencing impl_item, trait_path and trait_name and the macro pattern __devirt_define!/@impl with $trait_name:ident), check trait_path for a leading_colon or more than one segment and, if found, produce a clear compile-time error (use syn::Error::new_spanned on trait_path or similar) telling the user that only plain identifiers are allowed rather than silently truncating to the last segment.
🤖 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/Cargo.toml`:
- Line 14: The dependency entry for devirt-macros in Cargo.toml uses only a path
which prevents crates.io publishing; update the devirt-macros dependency entry
(the devirt-macros = { ... } line) to inherit the workspace version by adding
version.workspace = true alongside path and optional fields so it matches the
workspace versioning pattern (workspace version 0.2.0).
In `@crates/core/src/lib.rs`:
- Around line 178-180: Add an explicit minimum supported Rust version to
Cargo.toml to document the stabilization requirement for
core::ptr::without_provenance; update Cargo.toml by adding rust-version = "1.84"
in the package section so the toolchain requirement (Rust 1.84.0+) is clear to
consumers and CI; ensure the version string is exactly "1.84" and run cargo
metadata/build to verify no other MSRV-related issues.
---
Duplicate comments:
In `@crates/macros/src/lib.rs`:
- Around line 193-195: The code currently truncates qualified trait paths by
taking only trait_path.segments.last() into trait_name, causing `impl
super::MyTrait for Foo` to be treated as `MyTrait`; change this to explicitly
reject qualified paths: in the code that handles the trait in the impl
(referencing impl_item, trait_path and trait_name and the macro pattern
__devirt_define!/@impl with $trait_name:ident), check trait_path for a
leading_colon or more than one segment and, if found, produce a clear
compile-time error (use syn::Error::new_spanned on trait_path or similar)
telling the user that only plain identifiers are allowed rather than silently
truncating to the last segment.
🪄 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: 223512c7-b82b-4101-9195-956ed3a79f08
📒 Files selected for processing (20)
crates/core/Cargo.tomlcrates/core/src/lib.rscrates/core/tests/kani.rscrates/core/tests/ui.rscrates/core/tests/ui/all_arms.rscrates/core/tests/ui/method_attrs.rscrates/core/tests/ui/missing_method.rscrates/core/tests/ui/missing_method.stderrcrates/core/tests/ui/multi_arg.rscrates/core/tests/ui/multi_hot.rscrates/core/tests/ui/pub_trait.rscrates/core/tests/ui/single_hot.rscrates/core/tests/ui/unsafe_trait.rscrates/core/tests/ui/wrong_signature.rscrates/core/tests/ui_attr.rscrates/core/tests/ui_attr/attr_args_on_impl.rscrates/core/tests/ui_attr/attr_args_on_impl.stderrcrates/core/tests/ui_attr/attr_method_attrs.rscrates/core/tests/ui_attr/attr_unsafe_trait.rscrates/macros/src/lib.rs
Comment on lines
+178
to
180
| let fake: *const __DevirtT = ::core::ptr::without_provenance( | ||
| ::core::mem::align_of::<__DevirtT>(), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check MSRV in Cargo.toml
echo "=== Checking rust-version in Cargo.toml files ==="
grep -r "rust-version" Cargo.toml crates/*/Cargo.toml 2>/dev/null || echo "No rust-version specified"
echo ""
echo "=== Checking for toolchain file ==="
cat rust-toolchain.toml 2>/dev/null || cat rust-toolchain 2>/dev/null || echo "No toolchain file"
echo ""
echo "=== Checking CI for Rust version ==="
cat .github/workflows/*.yml 2>/dev/null | grep -A2 -B2 "toolchain\|rust-version" | head -30Repository: Kab1r/devirt
Length of output: 1892
🌐 Web query:
When was core::ptr::without_provenance stabilized in Rust?
💡 Result:
core::ptr::without_provenance was stabilized in Rust 1.84.0, released on January 9, 2025.
Citations:
- 1: https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance.html
- 2: https://doc.rust-lang.org/std/ptr/fn.without_provenance.html
- 3: https://releases.rs/docs/1.84.0/
- 4: https://github.com/rust-lang/rust/releases/tag/1.84.0
- 5: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0/
- 6: stabilize Strict Provenance and Exposed Provenance APIs rust-lang/rust#130350
Document MSRV in Cargo.toml to reflect core::ptr::without_provenance requirement (1.84.0+).
core::ptr::without_provenance was stabilized in Rust 1.84.0 (January 2025). While the project uses stable toolchain, no explicit MSRV is documented in Cargo.toml. Add rust-version = "1.84" to make this requirement clear.
🤖 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 178 - 180, Add an explicit minimum
supported Rust version to Cargo.toml to document the stabilization requirement
for core::ptr::without_provenance; update Cargo.toml by adding rust-version =
"1.84" in the package section so the toolchain requirement (Rust 1.84.0+) is
clear to consumers and CI; ensure the version string is exactly "1.84" and run
cargo metadata/build to verify no other MSRV-related issues.
- Add devirt-macros as workspace dependency with version for crates.io publishing (path-only deps can't be published) - Add rust-version = "1.85" MSRV (edition 2024 requires 1.85+, core::ptr::without_provenance stabilized in 1.84) - Reject qualified trait paths in #[devirt] expand_impl (e.g., super::MyTrait) with a clear error instead of silently truncating to the last segment https://claude.ai/code/session_01XRfaF7hqTVzJwR8tBrnkom
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/macros/src/lib.rs`:
- Around line 210-218: The current loop over impl_item.items only handles
syn::ImplItem::Fn and silently drops other impl items (consts/types/macros);
update the loop in the impl processing logic to detect non-function items and
emit a syn::Error (new_spanned) or otherwise return/collect an error like the
trait-side checks do, referencing the offending item span so the compiler shows
a helpful message; keep processing of functions using the existing
m.attrs/m.sig/m.block -> method_bodies logic but explicitly reject or report all
non-ImplItem::Fn entries instead of ignoring them.
🪄 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: e95fa573-c413-4bf8-a9a7-d80746b28f09
📒 Files selected for processing (3)
Cargo.tomlcrates/core/Cargo.tomlcrates/macros/src/lib.rs
Comment on lines
+210
to
+218
| for item in &impl_item.items { | ||
| if let syn::ImplItem::Fn(m) = item { | ||
| for a in &m.attrs { | ||
| a.to_tokens(&mut method_bodies); | ||
| } | ||
| m.sig.to_tokens(&mut method_bodies); | ||
| m.block.to_tokens(&mut method_bodies); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Non-function impl items are silently dropped.
The loop only processes ImplItem::Fn, so any associated constants, types, or macro invocations in the impl block will be silently discarded. A user who writes:
#[devirt::devirt]
impl Shape for Circle {
const NAME: &'static str = "Circle"; // silently dropped
fn area(&self) -> f64 { ... }
}...may be surprised when NAME disappears. Consider adding validation to reject non-function items explicitly, similar to the trait-side checks:
🛡️ Proposed validation
let mut method_bodies = proc_macro2::TokenStream::new();
for item in &impl_item.items {
- if let syn::ImplItem::Fn(m) = item {
+ match item {
+ syn::ImplItem::Fn(m) => {
for a in &m.attrs {
a.to_tokens(&mut method_bodies);
}
m.sig.to_tokens(&mut method_bodies);
m.block.to_tokens(&mut method_bodies);
+ }
+ syn::ImplItem::Const(c) => {
+ return syn::Error::new_spanned(
+ c,
+ "#[devirt] does not support associated constants in impl blocks",
+ )
+ .to_compile_error()
+ .into();
+ }
+ syn::ImplItem::Type(t) => {
+ return syn::Error::new_spanned(
+ t,
+ "#[devirt] does not support associated types in impl blocks",
+ )
+ .to_compile_error()
+ .into();
+ }
+ _ => {}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/macros/src/lib.rs` around lines 210 - 218, The current loop over
impl_item.items only handles syn::ImplItem::Fn and silently drops other impl
items (consts/types/macros); update the loop in the impl processing logic to
detect non-function items and emit a syn::Error (new_spanned) or otherwise
return/collect an error like the trait-side checks do, referencing the offending
item span so the compiler shows a helpful message; keep processing of functions
using the existing m.attrs/m.sig/m.block -> method_bodies logic but explicitly
reject or report all non-ImplItem::Fn entries instead of ignoring them.
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.
Replace
devirt::r#trait!/devirt::r#impl!with a singledevirtname that works as either a proc-macro attribute (default) or a
declarative macro (with
default-features = false).Both APIs delegate to
__devirt_define!, a #[doc(hidden)] internalmacro that holds all dispatch expansion logic. The
[hot]marker onimpls is removed — hot-path specialization is driven entirely by the
trait's hot-type list.
Breaking changes:
devirt::r#trait!anddevirt::r#impl!removed[hot]marker on impl blocks no longer acceptedNew crate structure:
macrosfeature (on by default)Also updates LTO guidance: LTO is no longer required since all dispatch
logic expands via macros into the user's crate.