Skip to content

test(semantic): pin the macro repetition operator and trailing separator behaviors - #10300

Open
orizi wants to merge 1 commit into
graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structurefrom
graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins
Open

orizi wants to merge 1 commit into
graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structurefrom
graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins

Conversation

@orizi

@orizi orizi commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Tests only - no production code changes. These goldens pass unchanged on this
branch; their value is pinning behaviors that the upcoming per-group expansion
rewrite must preserve, so that its review shows them as zero-churn.

Repetition operator constraints on a nested, NON-final group

Cairo enforces +/? in validate_repetition_operator_constraints(), after the
whole pattern matched, over every repetition occurrence - including the inner
repetition's occurrence in each group of an outer repetition. The new pins cover
the case where the violating group is not the last one, each with a *
counterpart proving the rejection comes from the operator constraint and not from
a plain match failure:

  • ($([$($x:ident),+]),*) on m!([], [a, b]) -> E2158 no-matching-rule.
  • ($([$($x:ident),*]),*) on m!([], [a, b]) -> accepted.
  • ($([$($x:ident)?]),*) on m!([a b], [c]) -> E2158 no-matching-rule.
  • ($([$($x:ident)*]),*) on m!([a b], [c]) -> accepted.

Cross-checked on rustc 1.96.0 (ac68faa20 2026-05-25), every program run with an
invocation because macro_rules! transcriber errors are lazy:

macro_rules! m { ($([$($x:ident),+]),*) => { 0 }; }
fn main() { let _ = m!([], [a, b]); }
error: no rules expected ]
note: while trying to match meta-variable $x:ident

macro_rules! m { ($([$($x:ident),]),) => { 0 }; }
fn main() { let _ = m!([], [a, b]); }
compiles, rustc exit 0

macro_rules! m { ($([$($x:ident)?]),*) => { 0 }; }
fn main() { let _ = m!([a b], [c]); }
error: no rules expected b
note: while trying to match ]

macro_rules! m { ($([$($x:ident)]),) => { 0 }; }
fn main() { let _ = m!([a b], [c]); }
compiles, rustc exit 0

The ? cases carry no separator because rustc rejects one at definition time:
($([$($x:ident),?]),*) gives "error: the ? macro repetition operator does not
take a separator". Cairo accepts ,?, so the pins use the rustc-legal form.

The outer level holds 2 groups in every case, and the non-violating group holds 2
captures wherever the operator allows it - ? caps its own group at 1 by
construction. The + invocation is m!([], [a, b]) rather than m!([], [a]) to
keep >= 2 captures per repetition level.

Trailing separator - deliberate divergence from rustc

($($x:ident),*) matches m!(a, b,): the matcher consumes a separator after
every match and only then tries the next one, so a trailing separator is absorbed
and does not add an empty match. The pinned tuple type (felt252, felt252) fixes
the match count at 2, so the golden also fails if the count changes.

rustc rejects the equivalent call:

macro_rules! m { ($($x:expr),) => { [$($x),] }; }
fn main() { let _arr: [i32; 2] = m!(1, 2,); }
error: unexpected end of macro invocation
note: while trying to match meta-variable $x:expr
The same program with m!(1, 2) compiles, rustc exit 0.

This pins the current behavior, not the desired one. Aligning with rustc - a
trailing separator no longer matching a $(...),* repetition - is a separate,
already scheduled change (graph node separator-grammar.sep-trailing-f13), which
will re-bless this golden into a rejection.

The call is written over several lines because the Cairo formatter deletes a
trailing separator from a single-line macro call and the golden framework formats
function_code; the multi-line form is the only one the formatter keeps - it even
emits the trailing separator itself when breaking a macro call, so the divergence
is reachable from formatted source.

Perturbations used to verify each pin fails when the pinned behavior changes. All
were run locally and reverted before the gates; none is committed:

  1. validate_repetition_operator_constraints(): if rep_id != RepetitionId(0) { continue; }, so only the outermost repetition is validated. Reddens exactly
    the + and ? pins; the 92 other tests in the file, including the
    pre-existing flat +/? ones, still pass.
  2. validate_repetition_operator_constraints(): a nested * must match exactly
    once, ZeroOrMore if rep_id != RepetitionId(0) && count != 1 => return false.
    Reddens both * pins, plus the pre-existing "expansion nesting matching the
    pattern nesting" test, which pins the same property.
  3. is_macro_rule_match_ex()'s repetition loop: reject a consumed separator that is
    not followed by a match, i.e. rustc's rule. Reddens exactly the
    trailing-separator pin - no other test in the file depends on a trailing
    separator being absorbed.

Gates: cargo test --profile=ci-dev -p cairo-lang-semantic; ./scripts/rust_fmt.sh;
./scripts/clippy.sh --profile=ci-dev; cargo run --profile=ci-dev --bin cairo-test
-- tests/bug_samples --starknet.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

orizi commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

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

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

This was referenced Aug 4, 2026
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 6851ea4 to 3ec02c5 Compare August 4, 2026 08:41
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 251468b to 056032c Compare August 4, 2026 08:41

orizi commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

No good reason for the rust divergence - specifically as adding "trailing handler" ,? is very easy.

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 3ec02c5 to 2110b60 Compare August 5, 2026 10:54
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 056032c to 08895c8 Compare August 5, 2026 10:54
@orizi

orizi commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Agreed on the end state - and the stack already gets there: #10313 adopts rustc's rejection of trailing separators, with $(,)? as the trailing handler (corelib's call sites are migrated to it there). This PR only pins the then-current acceptance so the behavior change lands as its own reviewable step; happy to drop the transitional pin if you'd rather not record it.

@orizi
orizi marked this pull request as ready for review August 5, 2026 11:10
@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only additions to semantic inline-macro goldens; no runtime or matching logic changes.

Overview
Adds golden diagnostic tests only in inline_macros to lock in current inline-macro repetition behavior ahead of a planned per-group expansion rewrite—no compiler changes in this PR.

New cases cover nested repetitions where the failing group is not the last outer match: inner + or ? violations yield E2158 on m!([], [a, b]) and m!([a b], [c]), while paired inner * rules accept the same invocations so failures are attributed to operator constraints, not plain match failure.

A separate pin documents that $(...),* accepts a trailing comma in m!(a, b,) and still expands to a 2-tuple—behavior that differs from macro_rules! and is explicitly marked as current semantics, not the long-term target.

Reviewed by Cursor Bugbot for commit 659ca65. Bugbot is set up for automated code reviews on this repo. Configure here.

@orizi orizi left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on the end state - the stack already gets there: #10313 adopts rustc's rejection of trailing separators, with $(,)? as the trailing handler (corelib call sites migrated there). This PR only pins the then-current acceptance so the behavior change lands as its own reviewable step; happy to drop the transitional pin if you'd prefer.

@orizi+AGNT made 1 comment.
Reviewable status: 0 of 1 files reviewed, all discussions resolved.

@eytan-starkware eytan-starkware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eytan-starkware+AGNT made 2 comments.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (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/expr/test_data/inline_macros line 2952 at r2 (raw file):

}
fn foo() -> felt252 {
    m!([a b],[c])

m!([a b], [c]) — space after the comma, to match m!([], [a, b]) in the sibling tests. same at 2973.

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 2e8d65e to 46f6b76 Compare August 16, 2026 11:37
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from ba82901 to 3dbde2a Compare August 16, 2026 11:37

@orizi orizi left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

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

m!([a b], [c]) — space after the comma, to match m!([], [a, b]) in the sibling tests. same at 2973.

Done, both call sites (plus a third with the same shape).

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 46f6b76 to 9aa44af Compare August 18, 2026 14:07
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 3dbde2a to 3096be4 Compare August 18, 2026 14:07

@orizi orizi left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Previously, orizi+AGNT (Agent AGNT for orizi) wrote…

Done, both call sites (plus a third with the same shape).

Done, all three call sites of that shape.

@eytan-starkware eytan-starkware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Previously, orizi+AGNT (Agent AGNT for orizi) wrote…

Done, all three call sites of that shape.

this was marked Done but the current head still has m!([a b],[c]) at all three call sites — was the update pushed? (possibly sitting in a local commit that missed the restack)

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 9aa44af to 5ce8cd9 Compare August 19, 2026 10:11
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 3096be4 to 285f03e Compare August 19, 2026 10:11

@orizi orizi left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orizi+AGNT made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on TomerStarkware).


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

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

this was marked Done but the current head still has m!([a b],[c]) at all three call sites — was the update pushed? (possibly sitting in a local commit that missed the restack)

Good catch - and the trail led somewhere interesting: the fix was applied, but CAIRO_FIX_TESTS reverts it on every bless, because the golden fixer runs the input through cairo-format, whose macro token-tree handling strips the space after ],. Same root cause as the formatter finding on the trailing-separator PR. The spacing will land together with the in-stack formatter fix (approved, in progress), whose re-bless then normalizes these calls correctly.

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 5ce8cd9 to 6714f62 Compare August 23, 2026 08:43
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 285f03e to 1231afc Compare August 23, 2026 08:43

@orizi orizi left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Previously, orizi+AGNT (Agent AGNT for orizi) wrote…

Good catch - and the trail led somewhere interesting: the fix was applied, but CAIRO_FIX_TESTS reverts it on every bless, because the golden fixer runs the input through cairo-format, whose macro token-tree handling strips the space after ],. Same root cause as the formatter finding on the trailing-separator PR. The spacing will land together with the in-stack formatter fix (approved, in progress), whose re-bless then normalizes these calls correctly.

Follow-up: the formatter fix (#10354) covers the trailing-separator half but not this one - the fixer's token-tree pass also normalizes spacing, which strips the space after ], on every re-bless. Making the formatter preserve token-tree spacing verbatim is a bigger change than this nit warrants, so the goldens keep the fixer's normalized form; happy to open a follow-up on token-tree formatting fidelity if you want it.

@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 1231afc to 92c5951 Compare August 23, 2026 10:32
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch 2 times, most recently from 4cc5112 to d3d7da6 Compare August 25, 2026 12:30
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 92c5951 to a08ae4f Compare August 25, 2026 12:30
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from d3d7da6 to e365173 Compare August 26, 2026 19:59
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch 5 times, most recently from b980694 to d92b547 Compare August 28, 2026 12:58
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from d92b547 to 86e3593 Compare September 14, 2026 04:51
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 94376d1 to 8d01682 Compare September 14, 2026 04:51
…tor behaviors

Tests only - no production code changes. These goldens pass unchanged on this
branch; their value is pinning behaviors that the upcoming per-group expansion
rewrite must preserve, so that its review shows them as zero-churn.

Repetition operator constraints on a nested, NON-final group
------------------------------------------------------------
Cairo enforces `+`/`?` in validate_repetition_operator_constraints(), after the
whole pattern matched, over every repetition occurrence - including the inner
repetition's occurrence in each group of an outer repetition. The new pins cover
the case where the violating group is not the last one, each with a `*`
counterpart proving the rejection comes from the operator constraint and not from
a plain match failure:

* `($([$($x:ident),+]),*)` on `m!([], [a, b])` -> E2158 no-matching-rule.
* `($([$($x:ident),*]),*)` on `m!([], [a, b])` -> accepted.
* `($([$($x:ident)?]),*)`  on `m!([a b], [c])` -> E2158 no-matching-rule.
* `($([$($x:ident)*]),*)`  on `m!([a b], [c])` -> accepted.

Cross-checked on rustc 1.96.0 (ac68faa20 2026-05-25), every program run with an
invocation because macro_rules! transcriber errors are lazy:

  macro_rules! m { ($([$($x:ident),+]),*) => { 0 }; }
  fn main() { let _ = m!([], [a, b]); }
    error: no rules expected `]`
      note: while trying to match meta-variable `$x:ident`

  macro_rules! m { ($([$($x:ident),*]),*) => { 0 }; }
  fn main() { let _ = m!([], [a, b]); }
    compiles, rustc exit 0

  macro_rules! m { ($([$($x:ident)?]),*) => { 0 }; }
  fn main() { let _ = m!([a b], [c]); }
    error: no rules expected `b`
      note: while trying to match `]`

  macro_rules! m { ($([$($x:ident)*]),*) => { 0 }; }
  fn main() { let _ = m!([a b], [c]); }
    compiles, rustc exit 0

The `?` cases carry no separator because rustc rejects one at definition time:
`($([$($x:ident),?]),*)` gives "error: the `?` macro repetition operator does not
take a separator". Cairo accepts `,?`, so the pins use the rustc-legal form.

The outer level holds 2 groups in every case, and the non-violating group holds 2
captures wherever the operator allows it - `?` caps its own group at 1 by
construction. The `+` invocation is `m!([], [a, b])` rather than `m!([], [a])` to
keep >= 2 captures per repetition level.

Trailing separator - deliberate divergence from rustc
-----------------------------------------------------
`($($x:ident),*)` matches `m!(a, b,)`: the matcher consumes a separator after
every match and only then tries the next one, so a trailing separator is absorbed
and does not add an empty match. The pinned tuple type `(felt252, felt252)` fixes
the match count at 2, so the golden also fails if the count changes.

rustc rejects the equivalent call:

  macro_rules! m { ($($x:expr),*) => { [$($x),*] }; }
  fn main() { let _arr: [i32; 2] = m!(1, 2,); }
    error: unexpected end of macro invocation
      note: while trying to match meta-variable `$x:expr`
  The same program with `m!(1, 2)` compiles, rustc exit 0.

This pins the current behavior, not the desired one. Aligning with rustc - a
trailing separator no longer matching a `$(...),*` repetition - is a separate,
already scheduled change (graph node separator-grammar.sep-trailing-f13), which
will re-bless this golden into a rejection.

The golden framework formats `cairo_code`, and the formatter keeps a macro
call's trailing separator exactly as written, so the single-line call with its
trailing comma is formatter-stable.

Perturbations used to verify each pin fails when the pinned behavior changes. All
were run locally and reverted before the gates; none is committed:

1. validate_repetition_operator_constraints(): `if rep_id != RepetitionId(0) {
   continue; }`, so only the outermost repetition is validated. Reddens exactly
   the `+` and `?` pins; the 92 other tests in the file, including the
   pre-existing flat `+`/`?` ones, still pass.
2. validate_repetition_operator_constraints(): a nested `*` must match exactly
   once, `ZeroOrMore if rep_id != RepetitionId(0) && count != 1 => return false`.
   Reddens both `*` pins, plus the pre-existing "expansion nesting matching the
   pattern nesting" test, which pins the same property.
3. is_macro_rule_match_ex()'s repetition loop: reject a consumed separator that is
   not followed by a match, i.e. rustc's rule. Reddens exactly the
   trailing-separator pin - no other test in the file depends on a trailing
   separator being absorbed.

Gates: cargo test --profile=ci-dev -p cairo-lang-semantic; ./scripts/rust_fmt.sh;
./scripts/clippy.sh --profile=ci-dev; cargo run --profile=ci-dev --bin cairo-test
-- tests/bug_samples --starknet.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.nested-capture-structure branch from 8d01682 to f621b25 Compare September 14, 2026 06:56
@orizi
orizi force-pushed the graph-plan/2026-08-03-macro-fixes/foundation.behavior-pins branch from 86e3593 to 659ca65 Compare September 14, 2026 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants