Emit dispatch shims for dyn Trait + Send/Sync/Send+Sync#17
Merged
Conversation
Inherent dispatch methods were only emitted on `impl dyn Trait`, so callers holding `&(dyn Trait + Send)` silently fell back to plain vtable dispatch with zero devirt benefit. Emit three additional inherent impl blocks that delegate to the base dispatch via coercion. Auto traits do not change the vtable layout, so the delegation is zero-cost — LLVM eliminates it entirely with `#[inline(always)]`. - Extract `rewrite_sig_with_named_args` shared helper - Add `generate_delegating_method` for auto-trait delegation - Emit impl blocks for Send, Sync, and Send+Sync in `expand_trait` - Add trybuild pass test (attr_dyn_send.rs) - Add equivalence test for auto-trait dispatch - Add shuffled_send benchmark group - Update README with dyn Trait + Send documentation https://claude.ai/code/session_01UvCHSU2321W1zxHppYuyyG
📝 WalkthroughSummary by CodeRabbit
WalkthroughEmits delegating inherent impls for Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
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: 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 306-337: The function rewrite_sig_with_named_args should avoid
generating idents that collide with existing parameter names: when handling
syn::Pat::Wild inside rewrite_sig_with_named_args, detect all existing idents
from sig.inputs (and the arg_names being built) and, instead of directly using
format_ident!("__arg{idx}"), generate a unique ident (e.g., "__arg{idx}",
"__arg{idx}_1", etc.) that does not appear in the existing set, assign that
unique ident into pat_type.pat and push it to arg_names; also apply the same
uniqueness logic for the defensive fallback that uses
format_ident!("__arg{idx}") so no generated name can duplicate an explicit
parameter ident (ensure you reference sig, sig.inputs, arg_names, Pat::Wild, and
the generated ident variable when locating the change).
🪄 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: 5c4fa3dc-24fa-4526-a313-7472f0569bf2
📒 Files selected for processing (6)
README.mdcrates/core/benches/dispatch.rscrates/core/tests/equivalence.rscrates/core/tests/ui_attr.rscrates/core/tests/ui_attr/attr_dyn_send.rscrates/macros/src/lib.rs
Comment on lines
+306
to
+337
| fn rewrite_sig_with_named_args( | ||
| sig: &syn::Signature, | ||
| ) -> (syn::Signature, Vec<syn::Ident>) { | ||
| let mut sig = sig.clone(); | ||
| let mut arg_names = Vec::new(); | ||
| for (idx, arg) in sig.inputs.iter_mut().enumerate() { | ||
| if let syn::FnArg::Typed(pat_type) = arg { | ||
| match &*pat_type.pat { | ||
| syn::Pat::Ident(pat_ident) => { | ||
| arg_names.push(pat_ident.ident.clone()); | ||
| } | ||
| syn::Pat::Wild(_) => { | ||
| let generated = format_ident!("__arg{idx}"); | ||
| *pat_type.pat = syn::Pat::Ident(syn::PatIdent { | ||
| attrs: vec![], | ||
| by_ref: None, | ||
| mutability: None, | ||
| ident: generated.clone(), | ||
| subpat: None, | ||
| }); | ||
| arg_names.push(generated); | ||
| } | ||
| _ => { | ||
| // Validation already rejects this case, but | ||
| // generate a name defensively. | ||
| arg_names.push(format_ident!("__arg{idx}")); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| (sig, arg_names) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd crates/macros && wc -l src/lib.rsRepository: Kab1r/devirt
Length of output: 70
🏁 Script executed:
sed -n '306,337p' crates/macros/src/lib.rsRepository: Kab1r/devirt
Length of output: 1236
🏁 Script executed:
# Look for test files or usages of this function
fd -e rs test | xargs grep -l "rewrite_sig_with_named_args" 2>/dev/null || echo "No test files found"
rg "rewrite_sig_with_named_args" crates/macros --type rs -A 3 -B 3Repository: Kab1r/devirt
Length of output: 119
🏁 Script executed:
# Check the full context of the function and how it's used
sed -n '300,400p' crates/macros/src/lib.rsRepository: Kab1r/devirt
Length of output: 3187
🏁 Script executed:
# Look for how arg_names is used in the generation
sed -n '340,450p' crates/macros/src/lib.rsRepository: Kab1r/devirt
Length of output: 3615
🏁 Script executed:
# Search for all usages of arg_names in the context of generate_dispatch_method
rg "arg_names" crates/macros/src/lib.rs -B 5 -A 5Repository: Kab1r/devirt
Length of output: 4172
🏁 Script executed:
# Check if there are any tests or examples that might demonstrate this issue
find . -name "*.rs" -type f | xargs grep -l "devirt\|Devirt" | head -5Repository: Kab1r/devirt
Length of output: 210
🏁 Script executed:
# Check the equivalence tests
cat crates/core/tests/equivalence.rs | head -100Repository: Kab1r/devirt
Length of output: 2853
🏁 Script executed:
# Look for examples or tests with __arg patterns
rg "__arg" . --type rustRepository: Kab1r/devirt
Length of output: 239
🏁 Script executed:
# Check if there's any validation that prevents this case
rg "validation\|validate" crates/macros/src/lib.rs -B 2 -A 5Repository: Kab1r/devirt
Length of output: 38
🏁 Script executed:
# Look for validation in the entire macros crate
rg "validate\|validation" crates/macros/src/lib.rs -B 3 -A 10Repository: Kab1r/devirt
Length of output: 38
🏁 Script executed:
# Look for where the function is being validated
grep -n "validate" crates/macros/src/lib.rs | head -20Repository: Kab1r/devirt
Length of output: 491
🏁 Script executed:
# Search for the main entry points and validation
sed -n '1,100p' crates/macros/src/lib.rsRepository: Kab1r/devirt
Length of output: 3226
🏁 Script executed:
# Look at the validate_trait_method function
sed -n '109,160p' crates/macros/src/lib.rsRepository: Kab1r/devirt
Length of output: 1680
🏁 Script executed:
# Let's create a test case to see if the collision issue actually occurs
# First, let me understand how the generated code looks
rg "dispatch_sig\|arg_names" crates/macros/src/lib.rs -B 2 -A 2Repository: Kab1r/devirt
Length of output: 38
🏁 Script executed:
# Let's trace through an example manually - look at the full signature generation
sed -n '305,400p' crates/macros/src/lib.rsRepository: Kab1r/devirt
Length of output: 2905
Synthesized argument names can collide with explicit parameters.
rewrite_sig_with_named_args generates __arg{idx} for wildcard parameters without checking if names already exist in the signature. A valid signature like fn f(&self, _: u32, __arg1: u32) creates two __arg1 parameters in the generated method, causing compilation failure.
♻️ Suggested fix
fn rewrite_sig_with_named_args(
sig: &syn::Signature,
) -> (syn::Signature, Vec<syn::Ident>) {
let mut sig = sig.clone();
let mut arg_names = Vec::new();
+ let mut used: std::collections::BTreeSet<String> = sig
+ .inputs
+ .iter()
+ .filter_map(|arg| match arg {
+ syn::FnArg::Typed(pat_type) => match &*pat_type.pat {
+ syn::Pat::Ident(pat_ident) => Some(pat_ident.ident.to_string()),
+ _ => None,
+ },
+ syn::FnArg::Receiver(_) => None,
+ })
+ .collect();
for (idx, arg) in sig.inputs.iter_mut().enumerate() {
if let syn::FnArg::Typed(pat_type) = arg {
match &*pat_type.pat {
syn::Pat::Ident(pat_ident) => {
arg_names.push(pat_ident.ident.clone());
}
syn::Pat::Wild(_) => {
- let generated = format_ident!("__arg{idx}");
+ let generated = {
+ let mut n = idx;
+ loop {
+ let candidate = format_ident!("__devirt_arg{n}");
+ if used.insert(candidate.to_string()) {
+ break candidate;
+ }
+ n += 1;
+ }
+ };
*pat_type.pat = syn::Pat::Ident(syn::PatIdent {🤖 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 306 - 337, The function
rewrite_sig_with_named_args should avoid generating idents that collide with
existing parameter names: when handling syn::Pat::Wild inside
rewrite_sig_with_named_args, detect all existing idents from sig.inputs (and the
arg_names being built) and, instead of directly using
format_ident!("__arg{idx}"), generate a unique ident (e.g., "__arg{idx}",
"__arg{idx}_1", etc.) that does not appear in the existing set, assign that
unique ident into pat_type.pat and push it to arg_names; also apply the same
uniqueness logic for the defensive fallback that uses
format_ident!("__arg{idx}") so no generated name can duplicate an explicit
parameter ident (ensure you reference sig, sig.inputs, arg_names, Pat::Wild, and
the generated ident variable when locating the change).
Owner
Author
There was a problem hiding this comment.
This is quite a contrived case
There was a problem hiding this comment.
@Kab1r, fair point — requiring a user to deliberately name a parameter __devirt_arg{n} (a clearly internal/generated naming convention) to trigger the collision is indeed very contrived. I'll drop this concern.
😿
🧠 Learnings used
Learnt from: CR
Repo: Kab1r/devirt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T14:28:01.121Z
Learning: Limit declarative macro syntax support to plain fn method(&self, arg: Type) -> Ret; signatures only
Learnt from: CR
Repo: Kab1r/devirt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T14:28:01.121Z
Learning: Support unsafe trait/unsafe fn, method lifetimes, method attributes, supertraits, extern ABI fn, and method where clauses in the proc-macro attribute expansion
Learnt from: CR
Repo: Kab1r/devirt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T14:28:01.121Z
Learning: Use recursive macro expansion for dispatch arms instead of $()+ repetition to avoid macro_rules metavar depth conflicts
Learnt from: CR
Repo: Kab1r/devirt PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-12T14:28:01.121Z
Learning: Generate dispatch shim as an inherent impl dyn Trait block rather than trait default method to allow casting self as *const dyn Trait
Rename generated wildcard-replacement idents from `__arg{idx}` to
`__devirt_arg{idx}` to match the crate's naming convention and
eliminate any possibility of colliding with user parameter names.
Removes the `unique_arg_ident` helper since the namespaced prefix
makes collision-detection machinery unnecessary.
https://claude.ai/code/session_01UvCHSU2321W1zxHppYuyyG
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 328-332: The defensive fallback currently pushes a generated name
into arg_names but fails to rewrite the pattern stored in pat_type.pat, leaving
the signature and call out of sync; update the `_ => { ... }` branch so after
generating the identifier with format_ident!("__devirt_arg{idx}") you also
replace pat_type.pat with a Pat that binds that identifier (so the generated
name is actually used in the signature), referencing the existing symbols
arg_names, pat_type.pat, and the generated "__devirt_arg{idx}" identifier.
🪄 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: c231d5f5-cd16-4f8c-b1e7-0a6ad9ffb8cc
📒 Files selected for processing (1)
crates/macros/src/lib.rs
The `_ =>` branch in `rewrite_sig_with_named_args` pushed a generated ident into arg_names but did not rewrite the pattern in the signature, leaving the signature and call site out of sync. Now rewrites pat_type.pat to match, consistent with the Wild branch. https://claude.ai/code/session_01UvCHSU2321W1zxHppYuyyG
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 317-340: The Pat::Wild(_) arm and the fallback _ arm contain
identical logic; replace them with a single match arm that handles both cases
(e.g., match on syn::Pat::Wild(_) | _ ) and perform the shared generation/update
steps (create generated via format_ident!("__devirt_arg{idx}"), assign to
*pat_type.pat using syn::Pat::Ident(syn::PatIdent { ... }), and push generated
into arg_names) in that one arm to remove duplication while preserving behavior
for syn::Pat::Wild, the defensive fallback, and the same references to
pat_type.pat and arg_names.
🪄 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: 1850278b-bbe3-43ac-b007-c6194a0038f0
📒 Files selected for processing (1)
crates/macros/src/lib.rs
Comment on lines
+317
to
+340
| syn::Pat::Wild(_) => { | ||
| let generated = format_ident!("__devirt_arg{idx}"); | ||
| *pat_type.pat = syn::Pat::Ident(syn::PatIdent { | ||
| attrs: vec![], | ||
| by_ref: None, | ||
| mutability: None, | ||
| ident: generated.clone(), | ||
| subpat: None, | ||
| }); | ||
| arg_names.push(generated); | ||
| } | ||
| _ => { | ||
| // Validation already rejects this case, but | ||
| // generate a name defensively. | ||
| let generated = format_ident!("__devirt_arg{idx}"); | ||
| *pat_type.pat = syn::Pat::Ident(syn::PatIdent { | ||
| attrs: vec![], | ||
| by_ref: None, | ||
| mutability: None, | ||
| ident: generated.clone(), | ||
| subpat: None, | ||
| }); | ||
| arg_names.push(generated); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider deduplicating Wild and fallback branches.
The Pat::Wild branch (lines 317-326) and the fallback _ branch (lines 328-340) contain identical logic. While the separation documents intent (expected vs. defensive), combining them would reduce maintenance burden.
♻️ Optional: Combine identical branches
syn::Pat::Ident(pat_ident) => {
arg_names.push(pat_ident.ident.clone());
}
- syn::Pat::Wild(_) => {
- let generated = format_ident!("__devirt_arg{idx}");
- *pat_type.pat = syn::Pat::Ident(syn::PatIdent {
- attrs: vec![],
- by_ref: None,
- mutability: None,
- ident: generated.clone(),
- subpat: None,
- });
- arg_names.push(generated);
- }
- _ => {
- // Validation already rejects this case, but
- // generate a name defensively.
+ // Pat::Wild is expected; other patterns are rejected by
+ // validation but handled defensively.
+ _ => {
let generated = format_ident!("__devirt_arg{idx}");
*pat_type.pat = syn::Pat::Ident(syn::PatIdent {
attrs: vec![],
by_ref: None,
mutability: None,
ident: generated.clone(),
subpat: None,
});
arg_names.push(generated);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| syn::Pat::Wild(_) => { | |
| let generated = format_ident!("__devirt_arg{idx}"); | |
| *pat_type.pat = syn::Pat::Ident(syn::PatIdent { | |
| attrs: vec![], | |
| by_ref: None, | |
| mutability: None, | |
| ident: generated.clone(), | |
| subpat: None, | |
| }); | |
| arg_names.push(generated); | |
| } | |
| _ => { | |
| // Validation already rejects this case, but | |
| // generate a name defensively. | |
| let generated = format_ident!("__devirt_arg{idx}"); | |
| *pat_type.pat = syn::Pat::Ident(syn::PatIdent { | |
| attrs: vec![], | |
| by_ref: None, | |
| mutability: None, | |
| ident: generated.clone(), | |
| subpat: None, | |
| }); | |
| arg_names.push(generated); | |
| } | |
| syn::Pat::Ident(pat_ident) => { | |
| arg_names.push(pat_ident.ident.clone()); | |
| } | |
| // Pat::Wild is expected; other patterns are rejected by | |
| // validation but handled defensively. | |
| _ => { | |
| let generated = format_ident!("__devirt_arg{idx}"); | |
| *pat_type.pat = syn::Pat::Ident(syn::PatIdent { | |
| attrs: vec![], | |
| by_ref: None, | |
| mutability: None, | |
| ident: generated.clone(), | |
| subpat: None, | |
| }); | |
| arg_names.push(generated); | |
| } |
🤖 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 317 - 340, The Pat::Wild(_) arm and
the fallback _ arm contain identical logic; replace them with a single match arm
that handles both cases (e.g., match on syn::Pat::Wild(_) | _ ) and perform the
shared generation/update steps (create generated via
format_ident!("__devirt_arg{idx}"), assign to *pat_type.pat using
syn::Pat::Ident(syn::PatIdent { ... }), and push generated into arg_names) in
that one arm to remove duplication while preserving behavior for syn::Pat::Wild,
the defensive fallback, and the same references to pat_type.pat and arg_names.
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.
Inherent dispatch methods were only emitted on
impl dyn Trait, socallers holding
&(dyn Trait + Send)silently fell back to plainvtable dispatch with zero devirt benefit. Emit three additional
inherent impl blocks that delegate to the base dispatch via coercion.
Auto traits do not change the vtable layout, so the delegation is
zero-cost — LLVM eliminates it entirely with
#[inline(always)].rewrite_sig_with_named_argsshared helpergenerate_delegating_methodfor auto-trait delegationexpand_trait