fix(semantic): select a macro repetition's driver placeholder correctly

[orizi]

Summary

Adds a new semantic diagnostic E2202 (MacroRepetitionWithoutDriver) that is emitted when a macro expansion repetition block $(...) contains no placeholder that can drive the iteration — i.e., no placeholder bound to a pattern repetition at the required depth.

Previously, find_first_repetition_param would locate the first MacroParam node in a repetition block and use it as the driver, regardless of whether that placeholder was actually bound to a pattern repetition. This meant blocks containing only plain tokens, $defsite/$callsite, or depth-0 placeholders would silently misbehave. Now, find_repetition_driver skips non-driving elements and returns the first placeholder that is actually bound to a pattern repetition, and the declaration-time check (ExpansionCheckCtx) reports E2202 if no such placeholder exists.

Additionally, register_repetition_placeholders is introduced to pre-bind all placeholders in a pattern repetition block (including those in nested blocks) to their rep_id before matching begins. This ensures that when an outer repetition matches zero times, inner placeholders are still bound and the expansion correctly produces zero iterations rather than failing to find a driver.

The max_path_len_in_block field is added to ExpansionCheckCtx to track the deepest placeholder path seen within the current expansion block. A block whose maximum path length stays below curr_rep_depth triggers E2202. The value merges upward into the parent block on exit, so a depth-2 placeholder also satisfies the enclosing depth-1 block.

---

Type of change

Please check one:

• Bug fix (fixes incorrect behavior)
• New feature
• Performance improvement
• Documentation change with concrete technical impact
• Style, wording, formatting, or typo-only change

---

Why is this change needed?

Macro expansion repetition blocks with no repeating placeholder would previously either silently expand incorrectly or fail in an opaque way at expansion time. There was no compile-time diagnostic to catch patterns like $(1 +)* 0 or $($defsite::bar() +)* 0 where the block has nothing to iterate over.

---

What was the behavior or documentation before?

A repetition block in a macro expansion template that contained no placeholder bound to a pattern repetition would not be caught at declaration time. The expander would attempt to use the first MacroParam it found as the driver, which could be a non-repeating placeholder, $defsite, or nothing at all, leading to silent incorrect expansion.

---

What is the behavior or documentation after?

At macro declaration time, any expansion repetition block that contains no placeholder bound to a pattern repetition at the required depth is rejected with:

error[E2202]: Macro expansion repetition block has no placeholder to repeat over.

Repetition blocks that are driven by a non-first placeholder (e.g., $($x + $y +)* where $x is depth-0 and $y is the repeating one) now correctly identify $y as the driver. Repetitions matching zero times now correctly expand to nothing rather than failing to find a driver.

---

Related issue or discussion (if any)

---

Additional context

The existing test for $($x + $y),* was adjusted to wrap the expansion in an array [$($x + $y),*] so the comma-separated result is syntactically valid as an expression.

[reviewable-StarkWare]

This change is 

[orizi]

• fix(semantic): drive a macro expansion block by the repetition at its own depth #10282
• fix(semantic): expand a nested repetition over its own group of captures #10281
• fix(semantic): select a macro repetition's driver placeholder correctly #10270 👈 (View in Graphite)
• main

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

[cursor]

PR Summary

Medium Risk
Changes core user-defined macro matching and expansion behavior; incorrect driver selection could alter expanded code, though extensive tests cover nested and edge cases.

Overview
Macro expansion $() blocks no longer use the first textual placeholder as the iteration driver. find_repetition_driver picks a placeholder actually bound to a pattern repetition, preferring the outermost repetition not already driven by an enclosing block. rep_depths is recorded during matching and threaded through MatcherContext for that ranking.

Declaration-time validation now reports E2202 (MacroRepetitionWithoutDriver) when an expansion repetition has no valid driver (e.g. only literals, $defsite, or depth-0 placeholders). ExpansionCheckCtx::check_node tracks the deepest drivable placeholder depth per block to emit that diagnostic.

Pattern matching pre-binds placeholders via register_repetition_placeholders before a repetition is matched (including zero matches), so inner placeholders stay bound and zero-length repetitions expand correctly instead of losing driver information.

^{Reviewed by Cursor Bugbot for commit 04f3094. Bugbot is set up for automated code reviews on this repo. Configure here.}

[eytan-starkware]

@eytan-starkware reviewed 3 files and all commit messages, and made 3 comments.
Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on orizi and TomerStarkware).

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 444 at r1 (raw file):

0

//! > diagnostics

Can we have nested repitition with different drivers when they both are not first in either rep?

---

crates/cairo-lang-semantic/src/expr/test_data/inline_macros line 2865 at r1 (raw file):

//! > expected_diagnostics
error[E2202]: Macro expansion repetition block has no placeholder to repeat over.

Maybe improve error with action suggestion: Make sure there is a repitition-bounded variable being used in ...

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 247 at r1 (raw file):

    placeholder_paths: &'a OrderedHashMap<SmolStrId<'db>, Vec<usize>>,
    /// Number of `$()` expansion blocks currently entered. Serves as the required depth for
    /// E2198, the threshold a block must be driven up to for E2202, and the length

Error message codes in comment?

[orizi]

@orizi made 3 comments.
Reviewable status: 0 of 4 files reviewed, 3 unresolved discussions (waiting on eytan-starkware and TomerStarkware).

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 444 at r1 (raw file):

Previously, eytan-starkware wrote…

Can we have nested repitition with different drivers when they both are not first in either rep?

Done.

---

crates/cairo-lang-semantic/src/expr/test_data/inline_macros line 2865 at r1 (raw file):

Previously, eytan-starkware wrote…

Maybe improve error with action suggestion: Make sure there is a repitition-bounded variable being used in ...

Done.

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 247 at r1 (raw file):

Previously, eytan-starkware wrote…

Error message codes in comment?

Done.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit cfc4bf9. Configure here.}

[cursor]

Active driver re-loops captures

Medium Severity

When find_repetition_driver falls back to a repetition already present in repetition_indices, expansion still iterates the full capture list. For a deeper-only driver shared by outer and inner blocks, that re-loops every capture on every outer step and duplicates output whenever more than one value is captured. The new deeper_only!([1]) case hides this the same way a single-element input hid earlier driver-count bugs.

Additional Locations (2)

• crates/cairo-lang-semantic/src/items/macro_declaration.rs#L749-L757
• crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros#L573-L589

 

^{Reviewed by Cursor Bugbot for commit cfc4bf9. Configure here.}

[orizi]

The behaviour is real, but it is not introduced here — it reproduces identically on main:

($([$($x:expr),*]),*) => { $($($x +)*)* 0 }
deeper_only!([1, 2])
  main:          1+2+1+2+0
  this branch:   1+2+1+2+0

Same root cause I flagged on the earlier threads: repetition_len comes from the flat capture list, and with one repetition_indices entry per repetition id there is no way to express per-group windows. When one repetition legitimately spans two blocks, both loops run the full list, hence NxN. Fixing it means changing the capture representation, which is its own change.

The test criticism is fair, though, and I want to be straight about it. [1] is a single-element input, and single-element inputs have now masked two driver bugs on this PR. Here it is deliberate rather than incidental, for two reasons:

• That golden exists to guard a regression I introduced in 3a857685c and fixed in the next commit: the hard "skip already-driven repetitions" filter starved this shape entirely, so deeper_only!([1]) expanded to the literal m!([1]) — no expansion at all. [1] reproduces that, and the golden is verified failing against 3a857685c.
• [1] is the only input for which this shape is currently correct. A [1, 2] golden would encode 1+2+1+2+0, i.e. commit known-wrong output to the expectations, which I would rather not do — that is exactly the trap the driven at all levels golden fell into.

So the coverage gap you are pointing at is real and it is the pre-existing NxN bug, not something a golden on this PR can close. Happy to add a [1, 2] case the moment the capture representation is fixed, or to add it now marked as documenting current-broken behaviour if you would rather have it visible in the expectations than absent.

[orizi]

Now fixed, in the separate change I said it would need — #10282.

I also have to walk back the framing of my earlier reply. I claimed the shape had to be rejected because macro_rules! refuses it. It does not:

macro_rules! deeper_only { ($([$($x:expr),*]),*) => { $($($x +)*)* 0 }; }
deeper_only!([1, 2], [3, 4])       // 10
deeper_only!([1, 2], [3, 4, 5])    // 15

Both compile and expand correctly under rustc 1.96, at any nesting depth. What rustc does reject is the adjacent shape — ($($a:expr),*) => { $($a + $($a +)*)* 0 } — with "attempted to repeat an expression containing no syntax variables matched as repeating at this depth", which is precisely our E2202. So the existing max_path_len_in_block >= curr_rep_depth check is a faithful port of rustc's rule, and it is correct for it to let the deeper-only shape through.

The fix is therefore to make it iterate, not to reject. A $() block at depth d now drives the pattern repetition at depth d, reached by climbing rep_parents from the placeholder's own repetition — so the outer and inner block no longer share one driver and no longer both walk the flat list:

($([$($x:expr),*]),*) => { $($($x +)*)* 0 }
deeper_only!([1, 2], [3, 4])
  before: 1+2+1+2+0    // group [3,4] dropped entirely, group [1,2] emitted twice
  after:  1+2+3+4+0

Your criticism of the [1] golden was right and I should not have defended it: it is now [1, 2], [3, 4], with companions at three nesting levels, for sibling blocks driven by different deeper repetitions, and for zero-match groups. Every expected value is cross-checked against rustc.

The depth rule also makes the (is_iterated, pattern_depth) ranking dead — a block at depth d drives a repetition no enclosing block can be iterating — so find_repetition_driver loses the ranking entirely.

[eytan-starkware]

@eytan-starkware reviewed 5 files and all commit messages, made 5 comments, and resolved 2 discussions.
Reviewable status: 5 of 6 files reviewed, 7 unresolved discussions (waiting on orizi and TomerStarkware).

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 477 at r5 (raw file):

#[feature("user_defined_inline_macros")]
macro nested_drivers {
    ($n:expr; $($a:expr => [$($b:expr),*]),*) => { $($a + $($n + $b +)*)* 0 };

redundant imo

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 498 at r5 (raw file):

#[feature("user_defined_inline_macros")]
macro nested_drivers {
    ($n:expr; $($a:expr => [$($b:expr),*]),*) => { $n $( + $n + $a $( + $n + $a + $b)*)* };

Add a test where lhs is not sorted by our repetition depth
Also test cases where these is more then one depth one/two variable

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 519 at r5 (raw file):

#[feature("user_defined_inline_macros")]
macro three_levels {
    ($($a:expr => [$($b:expr => ($($c:expr),*)),*]),*) => { $($a + $($b + $($c +)*)*)* 0 };

Without actual repition we tested nothing. Each of them should repeat at least once

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 582 at r5 (raw file):

#[feature("user_defined_inline_macros")]
macro deeper_only {
    ($([$($x:expr),*]),*) => { $($($x +)*)* 0 };

Some repetition is required

---

crates/cairo-lang-semantic/src/expr/test_data/inline_macros line 3148 at r5 (raw file):

//! > ==========================================================================

//! > Test expansion repetition block whose only placeholder drives an enclosing block (E2202).

E2202?

[eytan-starkware]

Could max_path_len_in_block be a return value of check_node instead of a field on the ctx?

The value bubbles up — from placeholders, through the generic get_children branch, to the nearest enclosing repetition — so fn check_node(&mut self, node) -> usize returning the deepest pattern-path length in the subtree fits it directly:

• param, defined: path.len(); undefined: curr_rep_depth
• repetition: let inner = elements.map(|e| self.check_node(e)).max().unwrap_or(0);, then check inner < curr_rep_depth and return inner
• other node: children.map(...).max().unwrap_or(0); terminal: 0

That drops both the mem::take and the outer_max_path_len.max(..) merge — the parent's own .max() fold is the merge, and each block gets a fresh accumulator by construction.

What makes it stand out as a field is that its scoping is the opposite of the rest of ExpansionCheckCtx: known_path deliberately persists across siblings within a block (trimmed on exit, not restored), while this one must not leak between siblings. The mem::take is emulating a local.

Non-blocking, the current code is correct either way.

[orizi]

Done — the field is gone and check_node returns the value, exactly as you sketched:

• param, defined: path.len(); undefined: curr_rep_depth
• repetition: elements.map(|e| self.check_node(e)).max().unwrap_or(0), check inner < curr_rep_depth, return inner
• other node: children.map(..).max().unwrap_or(0); terminal: 0

Both the mem::take and the outer_max_path_len.max(..) merge are dropped — the parent's fold is the merge, and each block gets a fresh accumulator by construction. You were right that the field's scoping was the odd one out: known_path deliberately persists across siblings, this one must not, and the mem::take was emulating a local.

The local is named max_drivable_depth_in_block, per your other comment.

One behaviour difference worth naming, since it isn't a pure refactor. The old code assigned on the undefined-placeholder path:

self.max_path_len_in_block = self.curr_rep_depth;   // assign, not max

so an undefined placeholder could lower a value a sibling had already raised (reachable only after an E2198, which sets it above curr_rep_depth). As a fold it can only raise. That is confined to a rule that is already reporting a diagnostic — rule_err is set either way, so expansion is skipped — and the only effect is that it can no longer suppress a redundant second diagnostic. No golden moved.

[eytan-starkware]

@eytan-starkware made 6 comments.
Reviewable status: 5 of 6 files reviewed, 14 unresolved discussions (waiting on orizi and TomerStarkware).

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 537 at r5 (raw file):

test_expand_expr(expect_diagnostics: false)

//! > module_code

Add a test for a repetition depth 2 var opened in a repition block depth 1

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 289 at r5 (raw file):

                    None => {
                        // The placeholder's depth is unknown - assume it could have driven the
                        // enclosing block, to avoid piling E2202 on top of this error.

E2202!

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 324 at r5 (raw file):

        if let Some(repetition) = ast::MacroRepetition::cast(db, node) {
            let outer_max_path_len = std::mem::take(&mut self.max_path_len_in_block);

max_drivable_depth_in_block

Code quote:

max_path_len_in_block

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 695 at r5 (raw file):

/// Binds every placeholder in a pattern repetition block to its `rep_id`, nested repetitions
/// included - otherwise a nested block's placeholders stay unbound when the outer block matches

what is this otherwise?

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 696 at r5 (raw file):

/// Binds every placeholder in a pattern repetition block to its `rep_id`, nested repetitions
/// included - otherwise a nested block's placeholders stay unbound when the outer block matches
/// zero times. A nested repetition is entered after this and rebinds its own placeholders, so for a

after in this function?

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 723 at r5 (raw file):

/// bound to.
///
/// A placeholder carries the iteration count of the repetition it is bound to, so the outermost

prosy?

[eytan-starkware]

@eytan-starkware made 1 comment.
Reviewable status: 5 of 6 files reviewed, 15 unresolved discussions (waiting on orizi and TomerStarkware).

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 751 at r5 (raw file):

        };
        let Some((name, rep_id)) = found else { continue };
        const BEST_DRIVER_RANK: (bool, usize) = (false, 1);

I believe the bool is not being used correctly

[eytan-starkware]

@eytan-starkware reviewed 1 file.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on orizi and TomerStarkware).

[orizi]

@eytan-starkware — all open discussions addressed. Posting as one comment because most of them exist only as Reviewable discussions with no GitHub anchor; the two that do have threads are answered in place.

Three of your test requests could not go on this PR — details at the bottom.

On this PR (claude/macro-repetition-driver)

r	comment	what changed
r5 :324	max_drivable_depth_in_block	renamed, and it is now a check_node return value rather than a field — answered in the thread
r5 :289	E2202!	reworded, no code in the comment
r1 :247	Error message codes in comment?	that text is already gone; no error codes remain in comments in this file
r1 test_data:2865	improve error with action suggestion	now: "Macro expansion repetition block has no placeholder to repeat over. Consider using a placeholder declared inside a $() repetition in the pattern - only such a placeholder can drive one."
r5 :695	what is this otherwise?	spelled out: nested placeholders are bound here because their own repetition is only entered while matching its body, so a zero-match enclosing block leaves them with no binding at all — indistinguishable from an unknown placeholder
r5 :696	after in this function?	no — it now names is_macro_rule_match_ex explicitly
r5 :723	prosy?	cut from 4 sentences to a stated rule
r5 golden:477	$n redundant imo	dropped; the test is now ($($a:expr => [$($b:expr),*]),*) => { $($a + $($b +)*)* 0 }
r5 test_data:3148	E2202?	(E2199)/(E2202) stripped from all 5 test titles — the expected_diagnostics block below each already shows the code. If you instead meant "is E2202 right for this case": yes. $($a + $($a +)*)* has only a depth-1 placeholder in a depth-2 block, and rustc rejects the same shape with "attempted to repeat an expression containing no syntax variables matched as repeating at this depth".
r1 golden:444	nested repetition with different drivers, neither first in either rep	covered by "Test nested repetitions with different drivers, neither of them first in its block"
r5 :751	the bool is not being used correctly	the whole ranking is deleted in #10282. For the record I read the tuple ordering as correct — (false, d) sorts before (true, d') for every d, d', so a non-iterated candidate always wins, and the early return on (false, 1) is just a shortcut. But it is moot: under the depth rule a block at depth d drives the repetition at depth d, which no enclosing block can be iterating, so both components become dead.

Moved to #10281 — and why

r	comment
r5 golden:519	three_levels: without actual repetition we tested nothing	now 1 => [2 => (3, 4), 5 => (6, 7)], 8 => [9 => (10, 11)]
r5 golden:498	lhs not sorted by repetition depth; more than one depth-one/two variable	two new goldens, unsorted_lhs and multi_vars

These three cannot live on this PR. I wrote them here first and they fail: with more than one group at a level, this commit still walks the flat capture list, so it emits e.g. 1 + 2 + 3+4+6+7+10+11+5 + 3+4+... for the strengthened three_levels. Landing them here would mean blessing that output and then correcting it one commit later. They sit on #10281, which is the commit that gives each repetition its own group window, and they pass there. All three cross-checked against rustc (66, 15, 55).

Two more of your test notes are answered by #10282 rather than here:

• r5 golden:582 deeper_only — some repetition is required → now [1, 2], [3, 4], and it expands correctly (1+2+3+4+0) instead of 1+2+1+2+0.
• r5 golden:537 add a test for a repetition depth 2 var opened in a repetition block depth 1 → that is exactly deeper_only; it was silently wrong until fix(semantic): drive a macro expansion block by the repetition at its own depth #10282 and is now covered at two and three levels, plus zero-match groups.

[eytan-starkware]

@eytan-starkware reviewed 4 files and all commit messages, made 6 comments, and resolved 3 discussions.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on orizi and TomerStarkware).

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 444 at r1 (raw file):

Previously, orizi wrote…

Done.

where?

---

crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros line 477 at r5 (raw file):

Previously, eytan-starkware wrote…

redundant imo

The whole test

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 723 at r5 (raw file):

Previously, eytan-starkware wrote…

prosy?

still

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 751 at r5 (raw file):

Previously, eytan-starkware wrote…

I believe the bool is not being used correctly

When is it checked for false vs true? I dont understand it

---

crates/cairo-lang-semantic/src/diagnostic.rs line 1207 at r6 (raw file):

            SemanticDiagnosticKind::MacroRepetitionWithoutDriver => {
                "Macro expansion repetition block has no placeholder to repeat over. Consider \
                 using a placeholder declared inside a `$()` repetition in the pattern - only such \

revert to r2

---

crates/cairo-lang-semantic/src/items/macro_declaration.rs line 697 at r6 (raw file):

/// placeholder. A nested repetition that *is* entered rebinds its own placeholders back in
/// [`is_macro_rule_match_ex`], so a non-empty match ends up with the innermost binding.
fn register_repetition_placeholders<'db>(

If is_macro_rule_match_ex will fill in placeholders what is the point of this function?