fix(semantic): capture an expr placeholder by what the parser consumed

[orizi]

as_expr_macro_token_tree parsed the call's remaining text and threw the
parser's diagnostics away, and the caller then re-found the end of the
capture by summing token text lengths until it reached the parsed
expression's length. Both halves are replaced: the helper takes the
peekable iterator by &mut and advances it over the token trees the
expression it built actually spans, and returns None - the rule does not
match - when the parser reported an error on that prefix.

The dead duplicate let expr_length = expr_text.len(); and the explicit
zero-length guard are gone with it; an expression with nothing to parse
comes back as a parse diagnostic, which the new check already rejects (the
pair!(, 5) golden pins that it still ends in E2158).

Not honoring the diagnostics let a syntax error in the call be captured as
if it were an expression. Spliced into the expansion, its parse error was
reported again as E2117 "Parser error in macro-expanded code" against code
the user did not write; dropped by the expansion, it was not reported at
all - macro m { ($a:expr, $b:expr) => { $a }; } expanded m!(1, (a b)) to
1 with no diagnostics, where rustc rejects the call with "expected one of
!, ), ,, ., ::, ?, {, or an operator, found b".

The re-advance loop itself turns out not to be reachable-wrong: with the
expr follow set restricted to ,/;/=>, a diagnostic-free expression
always ends on a token tree boundary, so the length sum always landed
right. The loop is replaced anyway, and the "expression did not end on a
boundary" case is now None rather than an over-consumed token.

Re-derivation of the reported trigger. The defect was reported as a comment
in or next to an $x:expr capture corrupting it, with one!(2 /* c */)
yielding E2117 and a leading /* c */ yielding E2158. Cairo has no block
comments - Lexer::match_trivia matches whitespace, newlines and // runs
only - so /* c */ lexes as the tokens / * c * /, and those two symptoms
are the tokens failing to parse, not a comment being mishandled. Cairo's
one comment form, //, is trivia and is handled correctly both before,
inside and after a capture; the three new line-comment goldens pin that and
pass unchanged with this commit reverted. The /* */ goldens are pinned as
they behave, with the rustc divergence noted in each: it is the missing
lexer support, not the matcher.

Goldens, in crates/cairo-lang-semantic/src/expr/expansion_test_data/inline_macros.
Cross-checked against rustc 1.96.0 by compiling and running the same
macro_rules! calls:

• "two multi token expressions captured by two placeholders",
  pair!(1 + 2, 3 * 4) -> (1 + 2, 3 * 4); rustc (3, 12). Pin.
• "an expression capture opening with a parenthesized subtree",
  pair!((1 + 2) * 3, 4) -> ((1 + 2) * 3, 4); rustc (9, 4). Pin.
• "a line comment before an expression capture", pair!(// c\n1, 2) ->
  (1, 2); rustc (1, 2). Pin.
• "a line comment inside an expression capture", pair!(1 + // c\n2, 3) ->
  (1 + // c\n2, 3); rustc (3, 3). Pin.
• "a line comment after an expression capture", pair!(1 // c\n, 2) ->
  (1 // c\n, 2); rustc (1, 2). Pin.
• "a /* */ run inside an expression capture", pair!(2 /* c */, 5) ->
  E2158; rustc, where it is a comment, (2, 5). Flips.
• "a /* */ run before an expression capture", pair!(/* c */ 2, /* d */ 5)
  -> E2158; rustc (2, 5). Pin - it was already E2158, from the zero-length
  guard this commit removes.
• "a repetition whose first group holds a /* */ run",
  sum!(1 /* a */, 2 /* b */, 3) -> E2158; rustc 0 + 1 + 2 + 3 = 6.
  Flips.
• "an expression capture the parser reports an error on rejecting the rule",
  m!(1, (a b)) -> E2158; rustc errors. Flips.
• "an expression capture with no tokens to parse", pair!(, 5) -> E2158;
  rustc errors with "no rules expected ,". Pin.

With only the two source files reverted and the goldens kept, exactly the
three "flips" fail, and none of them panics:

Test "Test a `/* */` run inside an expression capture, which Cairo does
not lex as a comment." failed.
Output tag 'diagnostics' does not match:
<error[E2117]: Parser error in macro-expanded code: Missing tokens.
 Expected an expression.
< --> lib.cairo:10:7
>error[E2158]: No matching rule found in inline macro `pair`.
> --> lib.cairo:10:1

Test "Test a repetition whose first group holds a `/* */` run." failed.
Output tag 'diagnostics' does not match:
<error[E2117]: Parser error in macro-expanded code: Missing tokens.
 Expected an expression.  (twice, at 9:6 and 9:17)
>error[E2158]: No matching rule found in inline macro `sum`.

Test "Test an expression capture the parser reports an error on
rejecting the rule." failed.
`expect_diagnostics` is true, but no diagnostics were generated.

The last one is the silent-acceptance case, so it fails on the diagnostics
expectation rather than on an output tag - its expanded_code differs as
well (1 against m!(1, (a b))), but the runner stops at the expectation.

No existing golden changed - the whole diff to the test data file is
appended blocks. "Test expansion of macro with inner parse errors."
(array![format!]) is untouched: it goes through
token_tree_as_wrapped_arg_list, not through the expr placeholder path.

Green: cargo test --profile=ci-dev --workspace, cairo-test on corelib (737
passed) and on tests/bug_samples --starknet (68 passed), rust_fmt.sh,
clippy.sh --profile=ci-dev.

[reviewable-StarkWare]

This change is 

[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 👈 (View in Graphite)
• 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
• 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.

[cursor]

PR Summary

Medium Risk
Changes user-defined inline macro matching for $expr captures (error reporting and which rule matches), but the logic is centralized in one helper and one matcher path; risk is behavioral divergence from prior silent/wrong expansions rather than broad infra impact.

Overview
Inline macro $expr placeholders now only match when the call’s token trees start with a valid expression, and the matcher advances input by exactly the token trees that span the parsed expression—not by summing token text lengths.

as_expr_macro_token_tree takes the peekable iterator by &mut, returns None when the parser reports diagnostics on that prefix (so the rule fails to match instead of splicing bad text into expansions), and consumes token trees until the expression’s span end. If that end would fall inside a token tree, the capture is rejected rather than splitting a token. The matcher in macro_declaration.rs drops the old length-walking loop and empty-expression guard; failed or empty parses are handled by the helper’s diagnostic check.

Behavior changes: invalid second arguments (e.g. m!(1, (a b))) no longer expand silently to 1; they get E2158 instead of missing errors or E2117 on macro-expanded code. Failed captures can fall through to later rules (e.g. pick!(, my_ident)). /* */ still lexes as tokens in Cairo, so related cases stay E2158 (not a matcher bug). New expansion goldens cover multi-token captures, // comments, /* */, parse failures, and rule fallthrough.

^{Reviewed by Cursor Bugbot for commit 705b401. 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 3 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/items/macro_declaration.rs line 901 at r1 (raw file):

                        // Advances `input_iter` past the tokens of the captured expression, and
                        // rejects the rule when the input does not start with one.
                        let expr_node = as_expr_macro_token_tree(input_iter, file_id, db)?;

the TODO(Dean) about driving the parser off the iterator is dropped, but it still applies - we still parse the text and then walk the token trees a second time to find the boundary. keep it (moved into the helper).

---

crates/cairo-lang-parser/src/macro_helpers.rs line 52 at r1 (raw file):

/// not an expression, and the caller would splice it into generated code, where the very same
/// parse error is reported again - against code the user did not write.
pub fn as_expr_macro_token_tree<'a, TokenTrees>(

the name is now a lie - this doesn't convert a token tree to an expr, it consumes from the iterator and rejects on a parse error. rename to something that says it, e.g. take_leading_expr / parse_expr_prefix.

---

crates/cairo-lang-parser/src/macro_helpers.rs line 72 at r1 (raw file):

    let mut parser = Parser::new(db, file_id, span.take(file_content), &mut diagnostics);
    let expr_green = parser.parse_expr();
    if !diagnostics.build().is_empty() {

the parse error is thrown away here, so a single-rule macro reports only "No matching rule found" over the whole call and never says what is actually wrong - rustc points at the offending token. worth a TODO to surface the parser diagnostic when no rule matched.

---

crates/cairo-lang-parser/src/macro_helpers.rs line 85 at r1 (raw file):

    let mut consumed = token_trees.clone();
    let mut consumed_end = start;
    while consumed_end < expr_end {

the in-loop > check is the same as a single post-loop != - the loop already stops at the first consumed_end >= expr_end.

    let mut consumed = token_trees.clone();
    let mut consumed_end = start;
    while consumed_end < expr_end {
        consumed_end = consumed.next()?.as_syntax_node().span(db).end;
    }
    // An expression that parsed with no diagnostics is not expected to end inside a token tree,
    // but a rule is conservatively taken as unmatched rather than capturing a token in half.
    if consumed_end != expr_end {
        return None;
    }
    *token_trees = consumed;

also - the consumed clone exists only to keep the iterator untouched on None, and no caller relies on that (every one of them drops the iterator when the match fails). advancing token_trees in place drops a clone and the Clone bound gets one less user.

---

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

//! > module_code
// The first group's capture is not an expression, so the repetition matches no group and the
// three tokens left over reject the rule. `rustc`, where the runs are comments, expands the same

"the three tokens left over" is wrong - the repetition matches zero groups, so all 15 token trees of 1 /* a */, 2 /* b */, 3 are left over and that is what rejects the rule.

---

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

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

//! > Test an expression capture the parser reports an error on rejecting the rule.

missing the test that matters most for this change: None means "this rule does not match", not "error". add a macro with two rules where the first one's $a:expr fails to parse and a later rule matches, so a parse error in a capture is pinned as not poisoning the whole call.

[orizi]

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

---

crates/cairo-lang-parser/src/macro_helpers.rs line 52 at r1 (raw file):

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

the name is now a lie - this doesn't convert a token tree to an expr, it consumes from the iterator and rejects on a parse error. rename to something that says it, e.g. take_leading_expr / parse_expr_prefix.

Renamed to take_leading_expr.

---

crates/cairo-lang-parser/src/macro_helpers.rs line 72 at r1 (raw file):

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

the parse error is thrown away here, so a single-rule macro reports only "No matching rule found" over the whole call and never says what is actually wrong - rustc points at the offending token. worth a TODO to surface the parser diagnostic when no rule matched.

Added the TODO on surfacing the parser diagnostic when no rule matches.

---

crates/cairo-lang-parser/src/macro_helpers.rs line 85 at r1 (raw file):

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

the in-loop > check is the same as a single post-loop != - the loop already stops at the first consumed_end >= expr_end.

    let mut consumed = token_trees.clone();
    let mut consumed_end = start;
    while consumed_end < expr_end {
        consumed_end = consumed.next()?.as_syntax_node().span(db).end;
    }
    // An expression that parsed with no diagnostics is not expected to end inside a token tree,
    // but a rule is conservatively taken as unmatched rather than capturing a token in half.
    if consumed_end != expr_end {
        return None;
    }
    *token_trees = consumed;

also - the consumed clone exists only to keep the iterator untouched on None, and no caller relies on that (every one of them drops the iterator when the match fails). advancing token_trees in place drops a clone and the Clone bound gets one less user.

Done, as suggested - post-loop != with the comment moved out.

---

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

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

the TODO(Dean) about driving the parser off the iterator is dropped, but it still applies - we still parse the text and then walk the token trees a second time to find the boundary. keep it (moved into the helper).

Kept, moved into the helper next to the new TODO.

[orizi]

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

---

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

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

"the three tokens left over" is wrong - the repetition matches zero groups, so all 15 token trees of 1 /* a */, 2 /* b */, 3 are left over and that is what rejects the rule.

Fixed - the comment now says the repetition matches zero groups and all fifteen token trees are left over.

---

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

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

missing the test that matters most for this change: None means "this rule does not match", not "error". add a macro with two rules where the first one's $a:expr fails to parse and a later rule matches, so a parse error in a capture is pinned as not poisoning the whole call.

Added - pick with two rules: ($a:expr) fails to parse on , my_ident and the (, $x:ident) rule matches, pinning that a capture parse error means unmatched, not a broken call.