fix(semantic): reject a pattern that follows an expr placeholder with a token it would swallow

[orizi]

An expr placeholder captures by parsing the longest expression the call's
tokens start with, so whatever the pattern puts after it is only ever reached
when the expression grammar cannot extend over it. Only ,, ; and => can
never continue an expression, so only they may follow one - the same set rustc
allows after its own expr fragment. Anything else makes the rule match far
less than it reads as matching, or nothing at all.

Reported as E2209 at the offending follower, as rustc reports it, and folded
into the rule's err so a diagnosed rule does not still expand.

The follow set is computed the way rustc's check_matcher_core does, including
its one incompleteness: a separator-less repetition does not wrap its body's
FIRST set around, so ($($x:expr)*) stays accepted. The third negative-control
golden pins that parity.

In-tree violations, grepped over corelib/, tests/bug_samples/ and the semantic
golden dirs (and, beyond those, the whole repo): 3, all in the one macro
add_exprs of corelib/src/test/language_features/macro_test.cairo -
($x:expr 2), ($x:expr $y:expr) and (abc $x:expr $y:expr). Zero in
production corelib code, zero in tests/bug_samples/, zero in the goldens. All
three are given a legal separator here, keeping every rule and every assertion
of test_add_exprs; the two illegal shapes are pinned as E2209 goldens instead.
A hard error rather than a warning is warranted because user-defined inline
macros are still behind are_user_defined_inline_macros_enabled, nothing on the
stable surface breaks, and a warning could not satisfy the requirement that the
rule stop expanding.

Note for future changes here: no gate in CI other than the corelib suite itself
compiles corelib/src/test/, so cairo-test -- corelib/ has to be run
explicitly when macro declaration semantics change.

[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 👈 (View in Graphite)
• 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
• 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 for user-defined inline macros (feature-gated); miswritten patterns that previously compiled may now error, but production corelib usage was reportedly unaffected.

Overview
Macro rule patterns are now validated so a $name:expr placeholder may only be followed by ,, ;, or =>—tokens that cannot extend an expression—mirroring rustc’s expr fragment rules. Anything else (another placeholder, a literal, etc.) is reported as E2209 and the rule is treated as defective so it does not expand.

Follow-set analysis walks pattern structure via first_followers, including repetitions, subtrees, and optional/zero repetitions. Separator-less ($($x:expr)*) remains allowed (rustc parity).

The corelib add_exprs test macro is updated to use legal separators; semantic inline macro goldens cover invalid and valid patterns.

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

[eytan-starkware]

@eytan-starkware+AGNT made 7 comments.
Reviewable status: 0 of 4 files reviewed, 6 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/diagnostic.rs line 1258 at r1 (raw file):

            SemanticDiagnosticKind::MacroExprPlaceholderFollower { name, follower } => {
                format!(
                    "Macro placeholder '{}' captures an expression, so the pattern may only \

the allowed set is spelled out here and in EXPR_FOLLOW_SET - two places to keep in sync. make the const pub(crate) and build this from it, e.g. EXPR_FOLLOW_SET.map(|t| format!("{t}")).join(", ").

---

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

// An `expr` placeholder captures the longest expression the call starts with, so the `2` after it
// is only ever reached when the expression cannot extend over it - `add_two!(base 1 2)` captures
// `1 2` as one expression instead and the rule does not match at all. The placeholder is not the

this contradicts the golden right below it: there is no E2158 here, so the rule does match. parse_expr on 1 2 stops at 1, the 2 token then matches the pattern's 2, and the call only fails to expand because err is set. rejecting $x:expr 2 is forward-compat (as in rustc), not "this call breaks today" - say that instead.

---

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

/// the placeholder before it consumes, so every follow set allows it.
#[derive(Clone, Debug)]
struct Follower<'db> {

Follower holds nothing that isn't already in the node, and SyntaxNode is Copy. drop the struct and its impl, use Vec<SyntaxNode<'db>>, and take the text/ptr at the single place they're used:

if EXPR_FOLLOW_SET.contains(&follower.get_text_without_trivia(db).long(db).as_str()) { continue; }
res = Err(diagnostics.report(
    follower.stable_ptr(db),
    SemanticDiagnosticKind::MacroExprPlaceholderFollower {
        name,
        follower: follower.get_text_without_trivia(db),
    },
));

---

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

                let body_first =
                    first_followers(db, &repetition.elements(db).elements_vec(db), &[]);
                let may_match_nothing = body_first.is_empty()

body_first.is_empty() is not "the body can match nothing". a body made only of zero-able repetitions has a non-empty FIRST and still matches nothing, so a + over it is treated as guaranteed non-empty and the scan stops early. ($x:expr $($(a)?)+ b) never reports b. this is a second incompleteness beyond the one the PR description documents - rustc doesn't have it, it carries maybe_empty on the set. return it here too:

/// ... and whether `elements` can match no input at all.
fn first_followers<'db>(
    db: &'db dyn Database,
    elements: &[ast::MacroElement<'db>],
    outer: &[Follower<'db>],
) -> (Vec<Follower<'db>>, bool) {
    let mut res = vec![];
    for element in elements {
        match element {
            ast::MacroElement::Token(token) => {
                res.push(Follower::new(db, token.as_syntax_node()));
                return (res, false);
            }
            ast::MacroElement::Param(param) => {
                res.push(Follower::new(db, param.as_syntax_node()));
                return (res, false);
            }
            ast::MacroElement::Subtree(subtree) => {
                res.push(Follower::new(db, subtree_open_delimiter(db, &subtree.subtree(db))));
                return (res, false);
            }
            ast::MacroElement::Repetition(repetition) => {
                let (body_first, body_may_match_nothing) =
                    first_followers(db, &repetition.elements(db).elements_vec(db), &[]);
                res.extend(body_first);
                if !body_may_match_nothing
                    && !matches!(
                        repetition.operator(db),
                        ast::MacroRepetitionOperator::ZeroOrOne(_)
                            | ast::MacroRepetitionOperator::ZeroOrMore(_)
                    )
                {
                    return (res, false);
                }
            }
        }
    }
    res.extend(outer.iter().cloned());
    (res, true)
}

---

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

/// `Err` if any was reported, for the caller to mark the rule with - the placeholder would swallow
/// the tokens the pattern puts after it, so the rule must not expand.
fn check_expr_follow_set<'db>(

third recursive walk over MacroElement in this file with the same four arms (collect_pattern_placeholder_names, check_pattern_elements, now this). fold it into check_pattern_elements - take &[ast::MacroElement<'db>] plus outer there - so a pattern is walked once and the call site stays a single .and.

---

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

                // The subtree's closing delimiter bounds its last element, so nothing of the
                // pattern outside the subtree can follow it.
                res = res.and(check_expr_follow_set(

nothing covers this arm actually reporting - grid only exercises the valid path, and test 3 covers a subtree as a follower, not a violation inside one. dropping this recursive call fails no test. add a negative golden, e.g. macro m { ([$x:expr 2]) => { $x }; }.

[orizi]

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

---

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

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

the allowed set is spelled out here and in EXPR_FOLLOW_SET - two places to keep in sync. make the const pub(crate) and build this from it, e.g. EXPR_FOLLOW_SET.map(|t| format!("{t}")).join(", ").

Done - EXPR_FOLLOW_SET is pub(crate) and the message is built from it.

---

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

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

this contradicts the golden right below it: there is no E2158 here, so the rule does match. parse_expr on 1 2 stops at 1, the 2 token then matches the pattern's 2, and the call only fails to expand because err is set. rejecting $x:expr 2 is forward-compat (as in rustc), not "this call breaks today" - say that instead.

Rewrote the comment to the forward-compat rationale - the call matches today and only fails to expand because the rule is marked defective.

---

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

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

Follower holds nothing that isn't already in the node, and SyntaxNode is Copy. drop the struct and its impl, use Vec<SyntaxNode<'db>>, and take the text/ptr at the single place they're used:

if EXPR_FOLLOW_SET.contains(&follower.get_text_without_trivia(db).long(db).as_str()) { continue; }
res = Err(diagnostics.report(
    follower.stable_ptr(db),
    SemanticDiagnosticKind::MacroExprPlaceholderFollower {
        name,
        follower: follower.get_text_without_trivia(db),
    },
));

Dropped Follower; bare SyntaxNodes, text/ptr taken at the report site.

---

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

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

body_first.is_empty() is not "the body can match nothing". a body made only of zero-able repetitions has a non-empty FIRST and still matches nothing, so a + over it is treated as guaranteed non-empty and the scan stops early. ($x:expr $($(a)?)+ b) never reports b. this is a second incompleteness beyond the one the PR description documents - rustc doesn't have it, it carries maybe_empty on the set. return it here too:

/// ... and whether `elements` can match no input at all.
fn first_followers<'db>(
    db: &'db dyn Database,
    elements: &[ast::MacroElement<'db>],
    outer: &[Follower<'db>],
) -> (Vec<Follower<'db>>, bool) {
    let mut res = vec![];
    for element in elements {
        match element {
            ast::MacroElement::Token(token) => {
                res.push(Follower::new(db, token.as_syntax_node()));
                return (res, false);
            }
            ast::MacroElement::Param(param) => {
                res.push(Follower::new(db, param.as_syntax_node()));
                return (res, false);
            }
            ast::MacroElement::Subtree(subtree) => {
                res.push(Follower::new(db, subtree_open_delimiter(db, &subtree.subtree(db))));
                return (res, false);
            }
            ast::MacroElement::Repetition(repetition) => {
                let (body_first, body_may_match_nothing) =
                    first_followers(db, &repetition.elements(db).elements_vec(db), &[]);
                res.extend(body_first);
                if !body_may_match_nothing
                    && !matches!(
                        repetition.operator(db),
                        ast::MacroRepetitionOperator::ZeroOrOne(_)
                            | ast::MacroRepetitionOperator::ZeroOrMore(_)
                    )
                {
                    return (res, false);
                }
            }
        }
    }
    res.extend(outer.iter().cloned());
    (res, true)
}

Fixed as suggested - first_followers returns (Vec, bool) with maybe_empty carried per rustc; a body of zero-able repetitions no longer stops the scan.

---

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

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

third recursive walk over MacroElement in this file with the same four arms (collect_pattern_placeholder_names, check_pattern_elements, now this). fold it into check_pattern_elements - take &[ast::MacroElement<'db>] plus outer there - so a pattern is walked once and the call site stays a single .and.

Folded - check_pattern_elements takes the slice plus outer and carries the expr-follow check; single walk, single .and at the call site.

---

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

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

nothing covers this arm actually reporting - grid only exercises the valid path, and test 3 covers a subtree as a follower, not a violation inside one. dropping this recursive call fails no test. add a negative golden, e.g. macro m { ([$x:expr 2]) => { $x }; }.

Added ([$x:expr 2]) - E2209 from inside the subtree.