fix(semantic): parenthesize an expression capture spliced into an expansion

[orizi]

A macro capture is spliced into the expansion as text, so the operators the
rule wrote around a placeholder bound into the value rather than around it:
macro neg { ($x:expr) => { 0 - $x }; } on neg!(1 + 2) expanded to
0 - 1 + 2, which is 1, where rustc prints -3. A value captured by an
expr placeholder is now wrapped in parentheses, which is how the atomicity
rustc gets from an expr fragment being one AST node is spelled in text.

The declared kind reaches the expansion through the capture leaves: PlaceholderKind
is recorded on CapturedValue at match time, next to a flag for the value being a
bare path, both read by ExpansionContext::expand_placeholder. An ident value is
never wrapped - it stands for a name, and a path segment, a struct field or an item
name does not accept a parenthesized expression. A bare path captured by an expr
placeholder is not wrapped either: it has no top-level operator for the expansion's
operators to bind into, and it is the one expression shape that reaches the
non-expression positions a rule can splice a capture into - Cairo has no path
placeholder kind, so expr is how a rule captures the path of a use item, and
use (a::bar); does not parse. rustc rejects such a rule outright rather than
supporting the exception; keeping it is a deliberate divergence, pinned by
"Test use-path as placeholder." in expr/test_data/inline_macros.

A placeholder's CodeMapping now spans its parentheses along with the value.
Everything the expanded code makes of a value is looked up by an offset that has to
land inside the mapping, and the offset of a node starting at the value includes the
parenthesis in front of it; excluding them broke corelib's
accessing_expanded_placeholders, where one rule passes its capture on to another,
with error[E0006]: Identifier not found. CodeOrigin::Span collapses to the span's
start, so widening the mapping does not move where anything inside it resolves to.

Five goldens added to expr/expansion_test_data/inline_macros - neg!(1 + 2) giving
0 - (1 + 2), two captures around a * giving (1 + 2) * (3 + 4), a doubly nested
expansion block with two groups at each level, and the two negatives: ident
captures in a path and a callee, and an expr capture that is a bare path, both
spliced with no parentheses. One more pins a capture passed through a second macro
call, wrapped once per splice. An executed regression in
tests/bug_samples/macro_expr_capture_grouping.cairo asserts the runtime values,
with the captures built from locals so that resolving them goes through the mapping
of the inserted parentheses.

Churned expansion goldens, all in expr/expansion_test_data/inline_macros, all
because a capture the case does not otherwise care about is now parenthesized:
"nested expansion blocks each expanding over their own group of captures",
"a non-repeating placeholder broadcast to every group of a repeating one",
"an inner group that matched nothing expanding to nothing, next to one that did",
"nested expansion blocks flattening a nested pattern into one list",
"a bracketed call of a rule written with parentheses (valid)",
"a braced call of a rule written with parentheses (valid)",
"an expansion repetition whose operator differs from the pattern's (valid)",
"a repeating placeholder broadcast into a deeper expansion block",
"a deeper expansion block whose non-final broadcast group has no captures",
"a doubly nested expansion block flattened into one sum",
"a doubly nested expansion block whose non-final group matched nothing",
"two multi token expressions captured by two placeholders",
"an expression capture opening with a parenthesized subtree",
"a line comment before / inside / after an expression capture",
"a placeholder followed by a keyword in the expansion",
"an expansion block whose group ends every placeholder against a keyword",
"that the spacing of the call does not reach the expansion".

One rule changed rather than re-blessed: "Test rep placeholder used inside a nested
repetition in expansion (valid)." in expr/test_data/inline_macros asserts a
placeholder's repetition level is accepted, and its expansion $($x)* only parsed
while the captures glued into the single literal 123; (1)(2)(3) is not a call, so
it is now 0 $(+ $x)* - the move made for grid! when the expansion stopped gluing.

An assignment target stays untouched for free: $x = 5 only makes sense with $x
a path, and a path is exempt.

Bless-then-revert, with only macro_declaration.rs reverted and both golden files and
the bug_samples case kept - < is the expansion produced, > the one pinned:

Test "Test an expression capture parenthesized against the operator the
expansion writes before it." failed.
<0 - 1 + 2
>0 - (1 + 2)

Test "Test two expression captures parenthesized against an operator between
them." failed.
<1 + 2 * 3 + 4
>(1 + 2) * (3 + 4)

Test "Test every group of a nested expansion block parenthesizing its own
captures." failed.
<((0 + 1 + 2+ 3),(0 + 4+ 5 * 6))
>((0 + (1 + 2)+ (3)),(0 + (4)+ (5 * 6)))

Test "Test an expression capture passed on to another macro call, parenthesized
once per splice." failed.
<(1 + 2, 2)
>(((1 + 2)), (2))

test bug_samples::macro_expr_capture_grouping::test_expr_capture_is_grouped
  ... fail
  Panicked with "assertion failed: `neg!(a + b) == -3`.".

The two negative goldens pin that nothing is added, so reverting cannot flip them.
They were verified the other way, with the gate replaced by let parenthesize = true;:
"Test identifier captures spliced into a path and a callee, gaining no parentheses."
then fails with diagnostics generated, and "Test an expression capture that is a bare
path, which is spliced without parentheses." fails with (holder::VALUE) + 5.

The rewritten rule in expr/test_data/inline_macros passes either way, being a rule
change rather than a behavior pin.

Go-to-definition spot-check: this repo has no language server crate, so the check
could not be run against one. The consumer of code_mappings that go-to-definition
resolves through is Resolution::new (crates/cairo-lang-semantic/src/resolve/mod.rs)
via ExpansionOffset::mapped, which is exercised by the executed regression - its
captures are call site locals reached through the inserted parentheses, and pass!
crosses a widened mapping twice - and by corelib's accessing_expanded_placeholders
and expose_mappings_shift. Both are green. A jump in a real editor was not observed.

[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 👈 (View in Graphite)
• 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
• 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 default macro expansion text and source mappings for all user expr captures; behavior is intentional and heavily tested but could surprise macros that depended on the old operator binding.

Overview
Expr captures are parenthesized on splice when their top-level form has operator precedence risk (binaries, closures, blocks, if/match/loops, most unaries). That stops expansion operators from binding into the captured text—e.g. 0 - $x with 1 + 2 becomes 0 - (1 + 2) instead of (0 - 1) + 2. Atoms stay bare (paths, literals, tuples, calls), including bare paths and @ snapshot types in type-position splices where () would not parse; ident captures are never wrapped.

CodeMapping spans include the added parentheses so go-to-definition and nested macro passes (e.g. pass! forwarding a capture) still resolve through the wrapper.

Tests: refreshed expansion goldens, new expansion/diagnostic cases, one nested-repetition fixture rule switched to 0 $(+ $x)*, and a bug_samples runtime regression (neg!, mul!, pass!, define!).

^{Reviewed by Cursor Bugbot for commit 1a0a27c. 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 5 files reviewed, all discussions resolved.

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

[orizi]

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

[eytan-starkware]

@eytan-starkware+AGNT made 8 comments.
Reviewable status: 0 of 5 files reviewed, 7 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.

---

tests/bug_samples/macro_expr_capture_grouping.cairo line 26 at r5 (raw file):

// An `ident` capture is not parenthesized - an item name does not accept a parenthesized
// expression, so `fn (nine)()` would not even parse.
macro define {

define!/expose! and assert!(nine() == 9) assert nothing at runtime - the ident case is a parse-level fact, already pinned by the named! golden. drop it; this file is for the values.

---

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

//! > cairo_code
// The `+` in every group is what makes the expansion an expression: this rule is about the

revert this - it's churn left over from the is_path design.

literals are atoms under needs_parens, so $($x)* on m!(1, 2, 3) still glues to 123; there is no (1)(2)(3). i reverted the rule and the comment and expr::test::expr_diagnostics::inline_macros is green. the test loses its original shape - a juxtaposing expansion - for nothing.

---

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

/// The possible kinds of placeholders in a macro rule.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PlaceholderKind {

pub and Copy are leftovers from the abandoned "record PlaceholderKind on CapturedValue" design - nothing outside this file names the type, and the single match at line 879 doesn't need Copy. reverting both compiles clean.

#[derive(Debug, Clone, PartialEq, Eq)]
enum PlaceholderKind {

---

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

                                    .to_string(db),
                                stable_ptr: peek_token.stable_ptr(db).untyped(),
                                needs_parens: match &expr_node {

a 21-line match with a 7-line comment inside a struct literal, eight levels deep in an already long function. pull it out into a helper after is_macro_rule_match_ex, and let the field read needs_parens: expr_needs_parens(db, &expr_node),.

---

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

                                    ast::Expr::Binary(_)
                                    | ast::Expr::Closure(_)
                                    | ast::Expr::Block(_)

the brace-delimited forms must not be wrapped - this breaks expansions that work today.

a }-terminated expression is already atomic in text: no operator the expansion writes can bind into it. wrapping buys nothing for them and costs the body positions, exactly the class of break the two earlier bug reports were about. verified against the base of this PR:

macro m { ($b:expr) => { loop $b }; }
fn foo() { m!({ break; }) }

compiles on base; here it gives error[E2117]: Parser error in macro-expanded code: Skipped tokens. Expected: '{'. same for if c $b, and for an if capture in else position:

macro m { ($e:expr) => { if true { 1 } else $e }; }
fn foo() -> felt252 { m!(if false { 2 } else { 3 }) }

also green on base, two parser errors here. while c $b, for x in y $b and fn f() $b under expose! are the same shape.

the doc's own argument - "atoms are the expression shapes that reach the non-expression positions a rule can splice a capture into" - is just wrong for Block and If.

                                    ast::Expr::Binary(_) | ast::Expr::Closure(_) => true,

and goldens for loop $b and else $e.

---

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

        // starting at the value includes the parenthesis in front of it.
        let start = TextWidth::from_str(&self.res_buffer).as_offset();
        let parenthesize = value.needs_parens;

        if value.needs_parens {

and value.needs_parens again below - the local buys nothing.

---

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

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

//! > Test an expression capture passed on to another macro call, parenthesized once per splice.

"once per splice" is not what the golden shows. the inner call captures (1 + 2), which is Expr::Parenthesized - an atom - so the second splice adds nothing: two splices, one wrap. same claim in the module comment below ("wrapped once per macro it travels through") and in tests/bug_samples/macro_expr_capture_grouping.cairo:15. what the case actually pins is that the wrap survives a second capture and the mapping still covers it - name it that.