fix(semantic): reject a macro rule pattern that reuses a placeholder name

[orizi]

A pattern using one name for two placeholders leaves the nesting depth of the
values it stands for ambiguous, so nothing can decide which of the two the
expansion means. It is now a declaration-time error, E2204, reported once per
reused name at that name's second use in the pattern. The rule's err is set,
so it is not expanded: neither the expression-position path (rule.err?) nor
the item-position one emits a second diagnostic on top of it. This resolves the
TODO(Dean): Verify uniqueness of param names..

The expansion check is skipped for such a rule. Its placeholder paths keep the
last occurrence of a reused name, so E2198/E2199 would measure the expansion
against an arbitrary depth and report the pattern's ambiguity as a defect of the
expansion. ([$([$($p:ident),*]),*] [$($p:ident),*]) => { fn $p() {} } reported
E2198: ... 'p' requires 1 repetition level(s) ... before this change - p is
at depth 2 in its first occurrence, and the "1" came from the second. That
misfire is replaced, not doubled up on.

Base re-verification. The witness ($($x:expr),* ; $x:expr) => { $x } on
m!(1, 2; 3) no longer expands to 1: driving an expansion block by the
repetition at its own depth already turned it into an expansion-time E2202
("has no captured value in this repetition") with no expansion. The
declaration-time defect is untouched, and other shapes still generate silently
wrong code - ([$($x:ident),*] [$($x:ident),*]) => { 0 $(+ $x)* } on
dup!([a, b][c, d]) expanded to 0 + a + b + c + d, concatenating both sibling
groups, and ($p:ident, $p:ident, $p:ident) => { $p } on dup!(a, b, c)
expanded to a. Both are E2204 now.

A hard error rather than a softer rollout: no Cairo macro rule in corelib/,
tests/, crates/ or examples/ reuses a placeholder name within one pattern -
the only hit was the golden re-blessed below. cairo-test -- tests/bug_samples --starknet (68 passed) and cairo-test -- corelib --filter macro (49 passed)
confirm it against the real corelib.

rustc's macro_rules! agrees, rejecting all four shapes at definition time with
error: duplicate matcher binding, its caret on the whole duplicate matcher
($x:expr) - the same anchor E2204 uses. ($($a:ident),* ; $b:expr) is
accepted there as it is here.

The re-blessed golden, "a repetition whose placeholder shares its name with a
non-repeating one", was the repro of the E2202 added when the expansion failure
paths stopped panicking, and its trigger was a duplicate name
(($y:ident, $($y:ident),*)). It now reports E2204 at the pattern's second
$y:ident instead of E2202 at the expansion's $($y),* block, so it was
retitled and its comment - which asserted that names are not verified to be
unique - rewritten. It doubles as the different-nesting-depths case. E2202 is
left without a golden as a result; no non-duplicate-name trigger for it is
known, and its arms are kept because a rule with a reused name is still
matched - every rule is matched before its err is honored - so the
value-dropping paths in the matcher stay reachable and stay defensive.

Names are unique per pattern, not per macro: a later rule of the same macro may
reuse them, which the negative-control golden pins.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

[orizi]

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

• fix(semantic): reject a + expansion repetition over zero groups #10315
• doc(semantic): document the macro expansion error arms as defensive #10314
• fix(semantic): reject a trailing macro repetition separator like rustc #10313
• feat(semantic): match and emit non-comma macro repetition separators #10312
• feat(parser): accept any single-token macro repetition separator #10311
• refactor(syntax): generalize the macro repetition separator node #10310
• fix(semantic): parenthesize an expression capture spliced into an expansion #10309
• fix(semantic): keep the trailing trivia of an expansion's placeholders and blocks #10308
• fix(semantic): capture an expr placeholder by what the parser consumed #10307
• test(semantic): pin the macro matcher behaviors that already agree with rustc #10306
• fix(semantic): reject a pattern that follows an expr placeholder with a token it would swallow #10305
• fix(semantic): reject the malformed macro rule declarations that silently do nothing #10304
• fix(semantic): match a pattern subtree only against a call subtree with the same delimiters #10303
• fix(semantic): reject a macro rule pattern that reuses a placeholder name #10302 👈 (View in Graphite)
• fix(semantic): expand a macro repetition over its own group of captures #10301
• test(semantic): pin the macro repetition operator and trailing separator behaviors #10300
• refactor(semantic): build a nested capture tree alongside the flat macro captures #10299
• fix(semantic): reject a macro expansion block with nothing repeating at its depth #10298
• fix(formatter): stop adding trailing separators in macro call token trees #10354
• main

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

[reviewable-StarkWare]

This change is 

[cursor]

PR Summary

Medium Risk
Changes macro declaration semantics and error reporting for user-defined inline macros; invalid rules no longer expand, which may surface new compile errors but aligns with rustc and fixes previously silent wrong expansions.

Overview
User-defined inline macro rules now reject patterns that bind the same placeholder name more than once, emitting E2204 at the second binding and marking the rule invalid so it never expands (calls stay as written).

Declaration checking uses a new PatternPlaceholders walk to detect reused names and skips expansion-side placeholder validation for those rules, avoiding misleading depth/driver errors that came from treating the last occurrence as authoritative. Expansion repetition checking is refactored to precompute per-$() block driving depths via collect_block_driving_depths instead of scanning subtrees on the fly.

Tests are updated/added for sibling repetitions, post-repetition reuse, triple reuse (single diagnostic), and the case where distinct names across rules in one macro remain valid.

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

[orizi]

@orizi+AGNT made 1 comment and resolved 1 discussion.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on eytan-starkware and TomerStarkware).

[orizi]

Fixed at the source - the guard is #10297's; it now uses contains_missing, and this PR inherits it.

[eytan-starkware]

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

---

a discussion (no related file):
Note: the comments below are from an automatic orizi-review run (Claude agents reviewing in Ori's style, findings adversarially verified before posting). Treat with the usual bot skepticism.

---

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

    reused_names: OrderedHashMap<SmolStrId<'db>, SyntaxStablePtrId<'db>>,
    /// The path of the pattern elements being traversed.
    current_path: Vec<usize>,

current_path and next_rep_id are recursion scratch, not "placeholders the pattern defines" - after collect one is empty and the other meaningless, and a second collect on the same value keeps numbering from the old counter. keep them as params like the free function did and hand back the result:

impl<'db> PatternPlaceholders<'db> {
    fn collect(
        db: &'db dyn Database,
        elements: impl IntoIterator<Item = ast::MacroElement<'db>>,
    ) -> Self {
        let mut res = Self::default();
        res.collect_elements(db, elements, &mut vec![], &mut 0);
        res
    }
}

caller becomes let placeholders = PatternPlaceholders::collect(db, pattern_elements.elements(db));.

---

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

            SemanticDiagnosticKind::DuplicateMacroPlaceholder(name) => {
                format!(
                    "Macro placeholder '{}' is already captured by this rule's pattern. A \

second sentence restates the first, and it's the only two-sentence message among its neighbours (Undefined macro placeholder: 'x'.).

                format!(
                    "Macro placeholder '{}' is already captured by this rule's pattern.",
                    name.long(db)
                )

[orizi]

@orizi+AGNT made 2 comments and resolved 2 discussions.
Reviewable status: 0 of 7 files reviewed, all discussions resolved (waiting on TomerStarkware).

---

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

Previously, eytan-starkware+AGNT (Agent AGNT for eytan-starkware) wrote…

second sentence restates the first, and it's the only two-sentence message among its neighbours (Undefined macro placeholder: 'x'.).

                format!(
                    "Macro placeholder '{}' is already captured by this rule's pattern.",
                    name.long(db)
                )

Trimmed to the one sentence.

---

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

Previously, eytan-starkware+AGNT (Agent AGNT for eytan-starkware) wrote…

current_path and next_rep_id are recursion scratch, not "placeholders the pattern defines" - after collect one is empty and the other meaningless, and a second collect on the same value keeps numbering from the old counter. keep them as params like the free function did and hand back the result:

impl<'db> PatternPlaceholders<'db> {
    fn collect(
        db: &'db dyn Database,
        elements: impl IntoIterator<Item = ast::MacroElement<'db>>,
    ) -> Self {
        let mut res = Self::default();
        res.collect_elements(db, elements, &mut vec![], &mut 0);
        res
    }
}

caller becomes let placeholders = PatternPlaceholders::collect(db, pattern_elements.elements(db));.

Done, as suggested - collect builds and returns, collect_elements carries the scratch as params.

[cursor]

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

There are 2 total unresolved issues (including 1 from previous review).

^{❌ 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 c3c400b. Configure here.}