test(semantic): pin the macro repetition operator and trailing separator behaviors - #10300
Conversation
|
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.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
6851ea4 to
3ec02c5
Compare
251468b to
056032c
Compare
|
No good reason for the rust divergence - specifically as adding "trailing handler" |
3ec02c5 to
2110b60
Compare
056032c to
08895c8
Compare
|
Agreed on the end state - and the stack already gets there: #10313 adopts rustc's rejection of trailing separators, with |
PR SummaryLow Risk Overview The new cases lock in inline-macro matching behavior ahead of a planned per-group expansion rewrite: nested outer A separate golden documents that Reviewed by Cursor Bugbot for commit 3dbde2a. Bugbot is set up for automated code reviews on this repo. Configure here. |
orizi
left a comment
There was a problem hiding this comment.
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.
2110b60 to
f2bd7c2
Compare
5854cff to
a2276bd
Compare
29f0079 to
fec42a2
Compare
aee8917 to
8ce391a
Compare
7020fef to
ba82901
Compare
8ce391a to
2e8d65e
Compare
eytan-starkware
left a comment
There was a problem hiding this comment.
@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.
…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 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>
2e8d65e to
46f6b76
Compare
ba82901 to
3dbde2a
Compare
orizi
left a comment
There was a problem hiding this comment.
@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 matchm!([], [a, b])in the sibling tests. same at 2973.
Done, both call sites (plus a third with the same shape).

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 thewhole 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),+]),*)onm!([], [a, b])-> E2158 no-matching-rule.($([$($x:ident),*]),*)onm!([], [a, b])-> accepted.($([$($x:ident)?]),*)onm!([a b], [c])-> E2158 no-matching-rule.($([$($x:ident)*]),*)onm!([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:identmacro_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
bnote: 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 nottake 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 byconstruction. The
+invocation ism!([], [a, b])rather thanm!([], [a])tokeep >= 2 captures per repetition level.
Trailing separator - deliberate divergence from rustc
($($x:ident),*)matchesm!(a, b,): the matcher consumes a separator afterevery 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)fixesthe 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:exprThe 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 evenemits 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:
if rep_id != RepetitionId(0) { continue; }, so only the outermost repetition is validated. Reddens exactlythe
+and?pins; the 92 other tests in the file, including thepre-existing flat
+/?ones, still pass.*must match exactlyonce,
ZeroOrMore if rep_id != RepetitionId(0) && count != 1 => return false.Reddens both
*pins, plus the pre-existing "expansion nesting matching thepattern nesting" test, which pins the same property.
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