Skip to content

fix(semantic): reject a macro expansion block with nothing repeating at its depth - #10298

Open
orizi wants to merge 1 commit into
graph-plan/2026-08-03-macro-fixes/foundation.harden-macro-expansion-errorsfrom
graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder
Open

fix(semantic): reject a macro expansion block with nothing repeating at its depth#10298
orizi wants to merge 1 commit into
graph-plan/2026-08-03-macro-fixes/foundation.harden-macro-expansion-errorsfrom
graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder

Conversation

@orizi

@orizi orizi commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

ExpansionCheckCtx already tracks, for every placeholder, its pattern repetition depth
(placeholder_paths[name].len()) and the expansion depth it is used at (curr_rep_depth).
It now also validates each $() block of the expansion itself: a block nested in
enclosing_depth other blocks is legal only if it holds a placeholder whose pattern depth
exceeds enclosing_depth, i.e. something that actually repeats at this level and can drive
the block. Otherwise the number of repetitions is undetermined, and the new
declaration-time error E2203 is reported on the offending MacroRepetition and sets
rule.err, so the rule never expands.

Placeholders nested deeper inside the block count as drivers: in $($($x),*),* with $x at
pattern depth 1, $x sits in the inner block but is consumed by the outer repetition too, so
the outer block is legal and only the inner one is rejected.

E2203 was unallocated (main's general band ended at E2201, the parent commit took E2202;
E2300-E2315 is the InferenceError sub-band and is not part of this band). The next free
general semantic code is E2204.

Semantics mirrored from rustc

Probed with rustc 1.96.0 (ac68faa20 2026-05-25) in this change. rustc's transcriber-side
definition errors are LAZY, so every probe below invokes the macro; rustc then reports the
definition-site error upon invocation. Probe sources are transcribed inline.

REJECTED - all four report
attempted to repeat an expression containing no syntax variables matched as repeating at this depth:

  • no metavariable: macro_rules! m1 { () => { $(foo)* }; } + m1!();
    -> caret on (foo).
  • depth-0-only metavariable: macro_rules! m2 { ($a:ident) => { $($a)* }; } + m2!(x);
    -> caret on ($a), plus this similarly named macro metavariable is unrepeatable.
  • over-deep inner block: macro_rules! m3 { ($($x:ident),*) => { $($($x),*),* }; }
    • m3!(a, b) -> caret on the INNER ($x), not the outer block.
  • offender not first: macro_rules! m9 { ($($x:ident),*) => { stringify!($($x),* ; $(bar)*) }; }
    • m9!(a, b) -> caret on (bar); the leading well-formed block is silent.

ACCEPTED:

  • broadcast: macro_rules! m4 { ($p:ident, $($x:ident),*) => { stringify!($($p $x),*) }; }
    • m4!(f, a, b) prints f a, f b.
  • properly nested:
    macro_rules! m5 { ($([$($x:ident),*]),*) => { stringify!($([$($x),*]),*) }; }
    • m5!([a, b], [c, d]) prints [a, b], [c, d].
  • zero-match calls of those same two rules: m4!(f,) and m5!() print nothing, m5!([], [])
    prints [], [].

Two rustc behaviours copied deliberately:

  • macro_rules! m7 { ($($x:ident),*) => { stringify!($($(foo),*),*) }; } + m7!(a, b) reports
    exactly ONE error, anchored at the OUTER block. A block nested in a failing block always
    fails for the same reason (inner_max <= outer_max <= d < d+1), so in_non_repeating_block
    suppresses nested E2203s. E2193/E2198/E2199 are still reported inside a suppressed block.
  • macro_rules! m10 { ($($x:ident),*) => { stringify!($($q)*) }; } + m10!(a, b) reports one
    error, not a separate "unbound metavariable" one. So an undefined placeholder counts as a
    driver here and $($undef)* keeps reporting E2193 alone rather than E2193 + E2203.

STRICTER THAN RUSTC, deliberately: Cairo reports E2203 from priv_macro_declaration_data, so a
bad macro is rejected with no call anywhere, unlike rustc's lazy transcriber check. This
matches the neighbouring declaration-time checks E2193/E2198/E2199.

$defsite/$callsite are not placeholders (extract_placeholder filters them), so a block
holding only those is now E2203 too; it previously expanded to nothing silently. Grepping every
$( under corelib/ and all crate test_data found no such block - corelib's only
expansion-side repetitions (macro_test.cairo:239/241/281/321) are all driven by a depth-1
placeholder.

Goldens

crates/cairo-lang-semantic/src/expr/test_data/inline_macros, re-blessed with a narrow
CAIRO_FIX_TESTS=1 + --lib expr::test::expr_diagnostics::inline_macros. Hunk by hunk:

  1. :3067 title Test item-position macro call whose expansion repeats a placeholder-free block. -> Test placeholder-free repetition in an expansion, with an item-position call.
    The diagnostic is no longer produced by the call.
  2. :3086 E2202 -> E2203 with the new message. The caret is byte-identical: both the old
    expansion-time error and the new check anchor on the same MacroRepetition node in the
    declaration, and nothing else appears or disappears - m!(); in item position adds no
    fallout.
  3. :3093 title renamed for the same reason as hunk 1.
  4. :3112 E2202 -> E2203, same caret. The expression-position call stays silent because
    compute.rs propagates rule.err without reporting again.
  5. :3188 (cross-module mod helpers) E2202 -> E2203, same caret in the declaring module,
    title unchanged. The location survives for a new reason: the diagnostic used to live on the
    root module's MacroCallData with a stable ptr into helpers; it now lives on helpers's
    own macro-declaration diagnostics, which the runner collects via
    get_recursive_module_semantic_diagnostics (inline submodules).
  6. seven new sections appended - three error shapes and three legal controls, plus an
    offender-not-first variant modelled on rustc probe m9. Every repetition level with a
    repetition carries >= 2 captures (m!(first, second), m!(base, one, two), m!(1, 2),
    m!(10, 1, 2), m!([1, 2], [3, 4])).

Bless-then-revert: with only this commit's report disabled on this branch (if false && ...),
all six error goldens fail as output-tag mismatches, no panics -

  • the three re-homed goldens and the offender-not-first golden fall back to E2202 (the
    expansion-time path the parent commit added),
  • $($base)* falls back to error[E0006]: Identifier not found. - the block silently
    expanded to nothing,
  • $($($x),*),* falls back to
    error[E2117]: Parser error in macro-expanded code: Skipped tokens. Expected: statement.

The three legal controls pass with and without the check; a golden that emits no diagnostic
cannot change when the check is removed, so revert-mode evidence does not apply to them.

Those last two reverted outputs are the evidence for rule.err: E0006 and E2117 are produced
by the expansion, and they are gone once E2203 sets rule.err. The rule genuinely stops
expanding rather than expanding to nothing.

E2202 reachability

MacroExpansionFailure::RepetitionWithoutPlaceholder needs find_first_repetition_param to
return None, i.e. an expansion $() block with zero MacroParam descendants. Such a block
can never have a driving placeholder, so E2203 now rejects the rule at declaration time and
that arm is unreachable through Cairo source. MacroExpansionFailure::MissingCapture is
untouched; per the plan it should still be reachable via duplicate placeholder names in a
pattern (see the TODO(Dean): Verify uniqueness of param names.), which a later change rejects
at declaration time. E2202 stays either way as defensive hardening - neither arm may panic.

Gates: cargo test --profile=ci-dev -p cairo-lang-semantic (106 passed),
./scripts/rust_fmt.sh, ./scripts/clippy.sh --profile=ci-dev,
./scripts/validate_error_codes.sh, scripts/check_comment_punctuation.py crates,
cargo run --profile=ci-dev --bin cairo-test -- corelib/ (737 passed),
cargo run --profile=ci-dev --bin cairo-test -- tests/bug_samples --starknet (68 passed), plus
cargo test --profile=ci-dev over lowering / doc / starknet / sierra-generator / defs / plugins
as a declaration-time-fallout check.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

orizi commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

This was referenced Aug 4, 2026
///
/// Two kinds of errors are reported:
/// Three kinds of errors are reported:
/// * Depth mismatch (E2198): placeholder used at fewer expansion levels than its pattern depth.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not write error codes in comments - these are just for exernal consumption.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed across the stack — including the preexisting E2198/E2199 mentions this doc block builds on, and the E2206 one in the pr4a follow-up. Also renamed the juxt test macro to juxtaposed to fix the typos CI.

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder branch from 54de8ab to e2df0dd Compare August 4, 2026 08:41
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.harden-macro-expansion-errors branch from 99a511f to ea21fb3 Compare August 5, 2026 10:54
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder branch from e2df0dd to 0f4a094 Compare August 5, 2026 10:54
@orizi
orizi marked this pull request as ready for review August 5, 2026 11:10
@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes macro declaration validation and error codes (E2202 → E2203 for these cases); behavior is stricter and earlier than before but scoped to user-defined inline macros, not security-critical paths.

Overview
Adds declaration-time validation so each $() block in a macro rule’s expansion must contain a placeholder whose pattern repetition depth exceeds the block’s nesting depth—otherwise repetition count is undefined and E2203 is reported on the MacroRepetition, and the rule is marked invalid so it never expands.

This replaces the previous E2202 expansion-time path for the same class of bugs for normal macro source; has_driving_placeholder and in_non_repeating_block mirror rustc-style rules (including a single error on the outermost bad block and treating undefined placeholders as “drivers” to avoid double diagnostics). Inline macro diagnostic goldens were updated and extended with invalid/valid repetition cases (placeholder-free blocks, non-first offenders, depth-0-only vars, over-nested expansions, broadcast, zero-match).

Reviewed by Cursor Bugbot for commit ff9e3a9. Bugbot is set up for automated code reviews on this repo. Configure here.

@orizi orizi left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi resolved 1 discussion.
Reviewable status: 0 of 3 files reviewed, all discussions resolved.

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.harden-macro-expansion-errors branch from ea21fb3 to df6b1f5 Compare August 5, 2026 15:15
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder branch from 0f4a094 to 86a5bfc Compare August 5, 2026 15:15
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.harden-macro-expansion-errors branch from df6b1f5 to dfca0ac Compare August 5, 2026 16:58
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder branch from 86a5bfc to af3c023 Compare August 5, 2026 16:58
…at its depth

`ExpansionCheckCtx` already tracks, for every placeholder, its pattern repetition depth
(`placeholder_paths[name].len()`) and the expansion depth it is used at (`curr_rep_depth`).
It now also validates each `$()` block of the expansion itself: a block nested in
`enclosing_depth` other blocks is legal only if it holds a placeholder whose pattern depth
exceeds `enclosing_depth`, i.e. something that actually repeats at this level and can drive
the block. Otherwise the number of repetitions is undetermined, and the new
declaration-time error E2203 is reported on the offending `MacroRepetition` and sets
`rule.err`, so the rule never expands.

Placeholders nested deeper inside the block count as drivers: in `$($($x),*),*` with `$x` at
pattern depth 1, `$x` sits in the inner block but is consumed by the outer repetition too, so
the outer block is legal and only the inner one is rejected.

E2203 was unallocated (main's general band ended at E2201, the parent commit took E2202;
E2300-E2315 is the `InferenceError` sub-band and is not part of this band). The next free
general semantic code is E2204.

## Semantics mirrored from rustc

Probed with rustc 1.96.0 (ac68faa20 2026-05-25) in this change. rustc's transcriber-side
definition errors are LAZY, so every probe below invokes the macro; rustc then reports the
definition-site error upon invocation. Probe sources are transcribed inline.

REJECTED - all four report
`attempted to repeat an expression containing no syntax variables matched as repeating at this
depth`:

* no metavariable: `macro_rules! m1 { () => { $(foo)* }; }` + `m1!();`
  -> caret on `(foo)`.
* depth-0-only metavariable: `macro_rules! m2 { ($a:ident) => { $($a)* }; }` + `m2!(x);`
  -> caret on `($a)`, plus `this similarly named macro metavariable is unrepeatable`.
* over-deep inner block: `macro_rules! m3 { ($($x:ident),*) => { $($($x),*),* }; }`
  + `m3!(a, b)` -> caret on the INNER `($x)`, not the outer block.
* offender not first: `macro_rules! m9 { ($($x:ident),*) => { stringify!($($x),* ; $(bar)*) }; }`
  + `m9!(a, b)` -> caret on `(bar)`; the leading well-formed block is silent.

ACCEPTED:

* broadcast: `macro_rules! m4 { ($p:ident, $($x:ident),*) => { stringify!($($p $x),*) }; }`
  + `m4!(f, a, b)` prints `f a, f b`.
* properly nested:
  `macro_rules! m5 { ($([$($x:ident),*]),*) => { stringify!($([$($x),*]),*) }; }`
  + `m5!([a, b], [c, d])` prints `[a, b], [c, d]`.
* zero-match calls of those same two rules: `m4!(f,)` and `m5!()` print nothing, `m5!([], [])`
  prints `[], []`.

Two rustc behaviours copied deliberately:

* `macro_rules! m7 { ($($x:ident),*) => { stringify!($($(foo),*),*) }; }` + `m7!(a, b)` reports
  exactly ONE error, anchored at the OUTER block. A block nested in a failing block always
  fails for the same reason (`inner_max <= outer_max <= d < d+1`), so `in_non_repeating_block`
  suppresses nested E2203s. E2193/E2198/E2199 are still reported inside a suppressed block.
* `macro_rules! m10 { ($($x:ident),*) => { stringify!($($q)*) }; }` + `m10!(a, b)` reports one
  error, not a separate "unbound metavariable" one. So an undefined placeholder counts as a
  driver here and `$($undef)*` keeps reporting E2193 alone rather than E2193 + E2203.

STRICTER THAN RUSTC, deliberately: Cairo reports E2203 from `priv_macro_declaration_data`, so a
bad `macro` is rejected with no call anywhere, unlike rustc's lazy transcriber check. This
matches the neighbouring declaration-time checks E2193/E2198/E2199.

`$defsite`/`$callsite` are not placeholders (`extract_placeholder` filters them), so a block
holding only those is now E2203 too; it previously expanded to nothing silently. Grepping every
`$(` under corelib/ and all crate `test_data` found no such block - corelib's only
expansion-side repetitions (macro_test.cairo:239/241/281/321) are all driven by a depth-1
placeholder.

## Goldens

crates/cairo-lang-semantic/src/expr/test_data/inline_macros, re-blessed with a narrow
`CAIRO_FIX_TESTS=1` + `--lib expr::test::expr_diagnostics::inline_macros`. Hunk by hunk:

1. `:3067` title `Test item-position macro call whose expansion repeats a placeholder-free
   block.` -> `Test placeholder-free repetition in an expansion, with an item-position call.`
   The diagnostic is no longer produced by the call.
2. `:3086` `E2202` -> `E2203` with the new message. The caret is byte-identical: both the old
   expansion-time error and the new check anchor on the same `MacroRepetition` node in the
   declaration, and nothing else appears or disappears - `m!();` in item position adds no
   fallout.
3. `:3093` title renamed for the same reason as hunk 1.
4. `:3112` `E2202` -> `E2203`, same caret. The expression-position call stays silent because
   `compute.rs` propagates `rule.err` without reporting again.
5. `:3188` (cross-module `mod helpers`) `E2202` -> `E2203`, same caret in the declaring module,
   title unchanged. The location survives for a new reason: the diagnostic used to live on the
   root module's `MacroCallData` with a stable ptr into `helpers`; it now lives on `helpers`'s
   own macro-declaration diagnostics, which the runner collects via
   `get_recursive_module_semantic_diagnostics` (inline submodules).
6. seven new sections appended - three error shapes and three legal controls, plus an
   offender-not-first variant modelled on rustc probe m9. Every repetition level with a
   repetition carries >= 2 captures (`m!(first, second)`, `m!(base, one, two)`, `m!(1, 2)`,
   `m!(10, 1, 2)`, `m!([1, 2], [3, 4])`).

Bless-then-revert: with only this commit's report disabled on this branch (`if false && ...`),
all six error goldens fail as output-tag mismatches, no panics -

* the three re-homed goldens and the offender-not-first golden fall back to E2202 (the
  expansion-time path the parent commit added),
* `$($base)*` falls back to `error[E0006]: Identifier not found.` - the block silently
  expanded to nothing,
* `$($($x),*),*` falls back to
  `error[E2117]: Parser error in macro-expanded code: Skipped tokens. Expected: statement.`

The three legal controls pass with and without the check; a golden that emits no diagnostic
cannot change when the check is removed, so revert-mode evidence does not apply to them.

Those last two reverted outputs are the evidence for `rule.err`: E0006 and E2117 are produced
*by the expansion*, and they are gone once E2203 sets `rule.err`. The rule genuinely stops
expanding rather than expanding to nothing.

## E2202 reachability

`MacroExpansionFailure::RepetitionWithoutPlaceholder` needs `find_first_repetition_param` to
return `None`, i.e. an expansion `$()` block with zero `MacroParam` descendants. Such a block
can never have a driving placeholder, so E2203 now rejects the rule at declaration time and
that arm is unreachable through Cairo source. `MacroExpansionFailure::MissingCapture` is
untouched; per the plan it should still be reachable via duplicate placeholder names in a
pattern (see the `TODO(Dean): Verify uniqueness of param names.`), which a later change rejects
at declaration time. E2202 stays either way as defensive hardening - neither arm may panic.

Gates: `cargo test --profile=ci-dev -p cairo-lang-semantic` (106 passed),
`./scripts/rust_fmt.sh`, `./scripts/clippy.sh --profile=ci-dev`,
`./scripts/validate_error_codes.sh`, `scripts/check_comment_punctuation.py crates`,
`cargo run --profile=ci-dev --bin cairo-test -- corelib/` (737 passed),
`cargo run --profile=ci-dev --bin cairo-test -- tests/bug_samples --starknet` (68 passed), plus
`cargo test --profile=ci-dev` over lowering / doc / starknet / sierra-generator / defs / plugins
as a declaration-time-fallout check.
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.decl-check-block-repeating-placeholder branch from af3c023 to ff9e3a9 Compare August 6, 2026 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants