fix(parser): don't panic on $(...) without operator in macro calls (#9993) - #10005
Conversation
…9993) Macro-call token trees previously hit unreachable!() when a $(...) group lacked a repetition operator. Replace with MacroRepetitionOperator::missing and leave validation to the semantic layer — the macro declaration is the authoritative source of truth on whether the call shape is legal. Definition-site parsing keeps its existing E1031 diagnostic, now also exercised on the RHS body of a definition by a new regression test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR SummaryLow Risk Overview Macro definitions still use structured parsing in New parser, expansion, and semantic regression tests cover opaque call-site tokens, literal Reviewed by Cursor Bugbot for commit 8d3f874. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
This change is |
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status:
Summary
In the parser, a missing repetition operator inside a macro call site (
$(...)without?,+, or*) previously hit anunreachable!()panic. The operator arm now produces aMacroRepetitionOperator::missingnode instead, deferring validation to the semantic layer. Macro definitions continue to emit a hard parse error (E1031) when the operator is absent.Type of change
Please check one:
Why is this change needed?
At a macro call site,
$(x)without a trailing operator is syntactically ambiguous but not necessarily illegal — the semantic layer is responsible for checking whether the shape matches the macro declaration. Hittingunreachable!()in the parser caused a panic instead of a recoverable missing-node, preventing any downstream diagnostic or recovery.What was the behavior or documentation before?
Parsing a macro invocation containing
$(...)with no repetition operator caused the parser to panic viaunreachable!().What is the behavior or documentation after?
The parser emits a
MacroRepetitionOperator::missingnode and continues without a diagnostic. The semantic layer is then responsible for deciding whether the missing operator is valid given the macro declaration. A new test case (expect_diagnostics: false) confirms thatfoo!($(x))parses cleanly. Macro definitions still produceE1031when the operator is missing, as validated by an additional test case (expect_diagnostics: true).Related issue or discussion (if any)
Additional context
The distinction between call-site leniency and definition-site strictness is now documented with an inline comment in
parser.rs.