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
• 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 pattern validation and rejects previously accepted rules; impact is limited because user-defined inline macros are feature-gated and in-tree violations were fixed in tests only.

Overview
Macro rule patterns are now validated so a $name:expr placeholder may only be followed by ,, ;, or => (matching rustc’s expr fragment rules). Anything else—literal tokens, another placeholder, or tokens reached across optional repetitions—is reported as E2209 at the offending follower, and the rule is marked defective so it does not expand.

Pattern well-formedness uses a new first_followers walk (repetitions, subtrees, optional groups) to decide what can follow each placeholder. Separator-less ($($x:expr)*) stays allowed, aligned with rustc.

add_exprs in corelib macro tests is updated to use legal separators; semantic inline-macro goldens add positive and negative cases for the new check.

^{Reviewed by Cursor Bugbot for commit 304290b. 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 }; }.