macros-code-review

Macros Code Review

Review Workflow

1. Check Cargo.toml -- Note Rust edition (2024 reserves gen keyword, affecting macro output), proc-macro crate dependencies (syn, quote, proc-macro2), and feature flags (e.g., syn with minimal features)
2. Check macro type -- Determine if reviewing declarative (macro_rules!), function-like proc macro, attribute macro, or derive macro
3. Check if a macro is needed -- If the transformation is type-based, generics are better. Macros are for structural/repetitive code generation that generics cannot express
4. Scan macro definitions -- Read full macro bodies including all match arms, not just the invocation site
5. Check each category -- Work through the checklist below, loading references as needed
6. Gates -- Complete Gates below before reporting; do not substitute informal “I verified.”

Gates (before reporting findings)

Complete in order. Do not emit findings until Gate 4 passes for each issue.

Gate 1 — Crate context (on disk) PASS when: You opened the reviewed crate’s Cargo.toml (workspace member path if applicable) and recorded Rust edition, whether the crate is proc-macro = true, and relevant proc-macro dependencies or syn / quote feature flags. Blocks rationalization: Edition 2024 findings (gen, unsafe extern, generated unsafe bodies) and syn “full” vs minimal flags require this — do not flag edition-specific macro output without matching edition from the file.

Gate 2 — Macro definitions read PASS when: For every macro you critique, you read the full definition (all macro_rules! arms, or the proc-macro entry plus helpers you rely on), not only call sites or partial expansions. Artifact: At least one path per macro to the defining .rs file(s) you used.

Gate 3 — Per-finding evidence PASS when: Each planned issue has [FILE:LINE] from the current tree for the macro definition, attribute/derive site, or generated code location you are discussing (not from memory, docs-only, or another branch).

Gate 4 — Pre-report protocol PASS when: You loaded and applied beagle-rust:review-verification-protocol, including Macro-Specific Verification for hygiene, fragment type, and proc-macro performance claims. Then add findings.

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

| Issue Type | Reference | |------------|-----------| | Fragment types, repetition, hygiene boundaries (vars vs types), $crate paths, TT-muncher pattern + recursion limits, fragment-matcher follow restrictions, decl-vs-proc decision tree | [references/declarative-macros.md](references/declarative-macros.md) | | Proc macro types, syn/quote, span hygiene (call_site vs def_site vs mixed_site), syn::Error::new_spanned + combine, parse_quote! vs quote!, syn feature audit, trybuild UI tests | [references/procedural-macros.md](references/procedural-macros.md) |

Review Checklist

Declarative Macros (macro_rules!)

• [ ] Correct fragment types used (:expr vs :tt vs :ident -- wrong choice causes unexpected parsing)
• [ ] Repetition separators match intended syntax (, vs ; vs none, * vs +)
• [ ] Trailing comma/semicolon handled (add $(,)? or $(;)? at end of repetition)
• [ ] Matchers ordered from most specific to least specific (first match wins)
• [ ] No ambiguous expansions -- each metavariable appears in the correct repetition depth in the transcriber
• [ ] Variables defined in the macro use macro-internal names (hygiene protects variables, not types/modules/functions)
• [ ] Exported macros (#[macro_export]) use $crate:: for crate-internal paths, never crate:: or self::
• [ ] Standard library paths use ::core:: and ::alloc:: (not ::std::) for no_std compatibility
• [ ] compile_error! used for meaningful error messages on invalid input patterns
• [ ] Macro placement respects textual scoping (defined before use) unless #[macro_export]

Procedural Macros

• [ ] syn features minimized (don't enable full when derive suffices -- reduces compile time)
• [ ] Spans propagated from input tokens to output tokens (errors point to user code, not macro internals)
• [ ] Span::mixed_site() is the default for introduced helper variables — call_site only when intentionally pointing at user code; def_site is nightly-only on proc_macro::Span
• [ ] Error reporting uses syn::Error::new_spanned(node, msg) with the offending AST node, never panic!
• [ ] Multiple errors collected and reported together via syn::Error::combine (good UX vs first-failure)
• [ ] parse_quote! (not quote!) used when the result needs to be a syn::T for further AST manipulation; syn::parse2(quote!{...}) for fallible cases
• [ ] Public types re-exported from proc_macro2, not proc_macro (compatibility with downstream consumers)
• [ ] proc-macro2 used for testing (testable outside of compiler context)
• [ ] Generated code volume is proportionate -- proc macros that emit large amounts of code bloat compile times

Derive Macros

• [ ] Derivation is obvious -- a developer could guess what it does from the trait name alone
• [ ] Helper attributes (#[serde(skip)] style) are documented
• [ ] Trait implementation is correct for all variant shapes (unit, tuple, struct variants)
• [ ] Generated impl blocks use fully qualified paths (::core::, $crate::)

Attribute Macros

• [ ] Input item is preserved or intentionally transformed (not accidentally dropped)
• [ ] Attribute arguments are validated with clear error messages
• [ ] Test generation patterns (#[test_case] style) produce unique test names
• [ ] Framework annotations document what code they generate

Edition 2024 Awareness

• [ ] Macro output does not use gen as an identifier (reserved keyword -- use r#gen or rename)
• [ ] Generated unsafe fn bodies use explicit unsafe {} blocks around unsafe ops
• [ ] Generated extern blocks use unsafe extern

Generics vs Macros

Flag a macro when the same result is achievable with generics or trait bounds. Macros are appropriate when:

• The generated code varies structurally (not just by type)
• Repetitive trait impls for many concrete types
• Test batteries with configuration variants
• Compile-time computation that const fn cannot express

Severity Calibration

Critical (Block Merge)

• Macro generates unsound unsafe code
• Hygiene violation in macro that outputs unsafe blocks (caller's variables leak into unsafe context)
• Proc macro panics instead of returning compile_error! (crashes the compiler)
• Derive macro generates incorrect trait implementation (violates trait contract)

Major (Should Fix)

• Exported macro uses crate:: or self:: instead of $crate:: (breaks for downstream users)
• Exported macro uses ::std:: instead of ::core::/::alloc:: (breaks no_std users)
• Wrong fragment type causing unexpected parsing (:expr where :tt needed, or vice versa)
• Proc macro enables syn full features unnecessarily (compile time cost)
• Missing span propagation (errors point to macro definition, not invocation)
• No error handling in proc macro (panics on bad input instead of compile_error!)

Minor (Consider Fixing)

• Missing trailing comma/semicolon tolerance in repetition patterns
• Matcher arms not ordered most-specific-first
• Macro used where generics would be clearer and equally expressive
• Missing compile_error! fallback arm for invalid patterns
• Helper attributes undocumented

Informational (Note Only)

• Suggestions to split complex macro_rules! into a proc macro
• Suggestions to reduce generated code volume
• TT munching or push-down accumulation patterns that could be simplified

Valid Patterns (Do NOT Flag)

• **macro_rules! for test batteries