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. Verify before reporting -- Load beagle-rust:review-verification-protocol before submitting 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

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::call_site() used for identifiers that should be visible to the caller
• Span::mixed_site() used for identifiers private to the macro (matches macro_rules! hygiene)
• Error reporting uses syn::Error with proper spans, not panic!
• Multiple errors collected and reported together via syn::Error::combine
• 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 -- Generating repetitive test modules from a list of types/configs
• macro_rules! for trait impls -- Implementing a trait for many concrete types with identical bodies
• TT munching -- Valid advanced pattern for recursive token processing
• Push-down accumulation -- Valid pattern for building output incrementally across recursive calls
• #[macro_export] with $crate -- Correct way to make macros usable outside the defining crate
• Span::call_site() for generated functions -- Intentionally making generated items visible to callers
• syn::Error::to_compile_error() -- Correct error reporting pattern in proc macros
• trybuild tests for proc macros -- Standard compile-fail testing approach
• Attribute macros on test functions -- Common pattern for test setup/teardown
• compile_error! in impossible match arms -- Good practice for catching invalid macro input

Before Submitting Findings

Load and follow beagle-rust:review-verification-protocol before reporting any issue.