fix(semantic): reject the malformed macro rule declarations that silently do nothing

[orizi]

A pattern capturing into $defsite / $callsite, a ? repetition taking a
separator and an empty $() body are all accepted today and then contribute
nothing, and a rule whose pattern has a parse error is dropped silently, so
every call written for it fails with a bare "no matching rule".

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

[orizi]

the explaination seems like too much.

[orizi]

Shortened both messages to the bare statement.

[orizi]

Shortened to the bare statement.

[orizi]

the explaination seems like too much.

[orizi]

Shortened.

[orizi]

Shortened to the bare statement.

[cursor]

PR Summary

Medium Risk
Changes macro declaration and rule registration behavior; macros that previously compiled may now error at definition, but scope is limited to user-defined inline macros in semantic analysis.

Overview
User-defined inline macro rules are now validated at declaration time so broken patterns no longer fail silently at call sites with only “no matching rule.”

New diagnostics (E2205–E2208): pattern placeholders named $defsite or $callsite (captures that can never be read in expansions); ? repetitions with a separator (in both pattern and expansion); literally empty $() pattern bodies; and rules whose pattern did not parse (explicit E2208 while the rule is dropped, instead of only a call-site mismatch).

Declaration logic in macro_declaration.rs walks pattern repetitions/subtrees via check_pattern_elements, records modifier-named params in PatternPlaceholders::modifier_named, and applies the same separator check during expansion traversal.

^{Reviewed by Cursor Bugbot for commit fb771d3. Bugbot is set up for automated code reviews on this repo. Configure here.}

[orizi]

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

[orizi]

Shortened to the bare statement.

[orizi]

Shortened to the bare statement.

[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/expr/test_data/inline_macros line 3282 at r3 (raw file):

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

//! > Test a pattern capturing into the name `$defsite`.

every new case here is a top-level pattern element. check_pattern_elements recurses into repetitions and subtrees and none of that recursion is pinned - add ($($defsite:ident)*), ([$defsite:ident]) and a ?-with-separator nested inside an outer repetition.

---

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

/// `Err` if any was reported, for the caller to mark the rule with - a defective element makes the
/// rule's meaning unclear, so it must not expand.
fn check_pattern_elements<'db>(

third full traversal of the pattern with the same four match arms - PatternPlaceholders::collect right below already visits every Param/Repetition/Subtree and already computes the placeholder name (so check_placeholder_name re-interns it). fold the name check into collect - it has both the name and the ptr - and this shrinks to the two repetition checks.

---

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

    diagnostics: &mut SemanticDiagnostics<'db>,
) -> Maybe<()> {
    let ast::OptionTerminalComma::TerminalComma(separator) = repetition.separator(db) else {

let-chain:

    if let ast::OptionTerminalComma::TerminalComma(separator) = repetition.separator(db)
        && matches!(repetition.operator(db), ast::MacroRepetitionOperator::ZeroOrOne(_))
    {
        return Err(diagnostics.report(
            separator.stable_ptr(db).untyped(),
            SemanticDiagnosticKind::MacroRepetitionSeparatorWithZeroOrOne,
        ));
    }
    Ok(())

---

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

        // which a reused name leaves ambiguous - checking it then reports the ambiguity as
        // a defect of the expansion, which it is not.
        if placeholders.reused_names.is_empty() {

this was rule_err.is_ok(), which for an unparsed pattern was always true, so the expansion was checked. now it's reused_names.is_empty(), which for an unparsed pattern depends on whether recovery happened to leave two nameless placeholders - i.e. a coin flip on whether a dropped rule's expansion gets diagnostics.

make it explicit: hoist the !pattern_is_parsed report + continue to the top of the loop, right after let pattern = rule_syntax.lhs(db);. then the if pattern_is_parsed wrapper, the nameless-placeholders comment above it and this whole question go away.

---

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

            // it. It is reported so that a call written for it points at the parse error rather
            // than failing with a bare "no matching rule".
            diagnostics.report(

the comment says this is so "a call written for it points at the parse error rather than failing with a bare 'no matching rule'" - but it doesn't. look at the pair golden: E1008 from the parser on the same rule, then this E2208 on an overlapping span, then the same E2158 on the call, unchanged. three diagnostics for one typo, and the confusing one is still there.

fix the comment, or make it actually replace the E2158.

---

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

        if let Some(repetition) = ast::MacroRepetition::cast(db, node) {
            self.rule_err =

expand_repetition never looks at the operator - in an expansion ? is just *. so $(...),? on the rhs today expands over all the driver's groups and emits the separator between them, correctly. this call turns working code into an error, and it isn't "silently does nothing" as the PR title claims. the doc's "allows at most one group" rationale only holds for the pattern.

either make the expansion honor ? (fail when the driver has more than one group) and then reject the separator, or keep the check pattern-only and drop this call.