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]

• 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)
• main

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

[reviewable-StarkWare]

This change is 

[cursor]

PR Summary

High Risk
Large rewrite of macro matching and expansion in the semantic layer; incorrect repetition or capture logic could silently change expanded code or break corelib/tests that rely on user-defined macros.

Overview
User-defined inline macros now store pattern captures in a CaptureTree (leaf/seq) that mirrors $() nesting, so expansion can iterate the right groups, broadcast non-repeating placeholders, and handle empty matches. expand_macro_rule takes CaptureTrees and returns Result with MacroExpansionError instead of mutating a shared matcher context; expression- and item-position call sites report those failures instead of panicking.

Declaration-time checks add E2204 when a pattern binds the same placeholder twice (expansion validation is skipped for that rule so depth errors are not mis-attributed). E2203 flags expansion $() blocks with no repeating placeholder at that depth; E2202 covers runtime expansion failures (missing driver, conflicting drivers, missing capture).

Item macro calls skip expansion when the call syntax has missing/parser errors, so only parse diagnostics surface. Extensive golden tests cover nested repetition, flatten/broadcast, +/? vs *, and duplicate-name patterns.

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

[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 75c3eb4. Configure here.}

[cursor]

Redundant missing-node walk

Low Severity

The new parse-error skip walks descendants and checks is_missing by hand, duplicating contains_missing, which the expression-position path and this PR’s own pattern check already use. That helper walks the green tree on purpose so this check does not materialize a red SyntaxNode per descendant.

Additional Locations (1)

• crates/cairo-lang-semantic/src/expr/compute.rs#L811-L813

 

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

[orizi]

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

[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)
                )