Sitelet https://github.com/starkware-libs/cairo/pull/10149
Skip to content

bugfix(parser): Make macro a valid recovery terminal and tree token. - #10149

Merged
orizi merged 1 commit into
mainfrom
orizi/06-22-bugfix_parser_make_macro_a_valid_recovery_terminal_and_tree_token
Jun 23, 2026
Merged

orizi merged 1 commit into
mainfrom
orizi/06-22-bugfix_parser_make_macro_a_valid_recovery_terminal_and_tree_token

Conversation

@orizi

@orizi orizi commented Jun 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes an ICE (Internal Compiler Error) that occurred when the macro keyword was used as a token inside an inline macro call (e.g., foo!(macro)). The TerminalMacro syntax kind was missing from the parser's token-tree leaf handling, causing a panic. Additionally, TerminalMacro was not included in the error recovery set of module-level keywords, meaning a macro declaration following a malformed item would not correctly stop error recovery.


Type of change

Please check one:

  • Bug fix (fixes incorrect behavior)
  • New feature
  • Performance improvement
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

Why is this change needed?

TerminalMacro was absent from the parser's token-tree leaf dispatch in parser.rs, so encountering macro as a leaf token inside an inline macro invocation caused an ICE. It was also missing from the module_item_kw! recovery macro in recovery.rs, so error recovery would not stop at a macro declaration boundary when recovering from a prior parse error.


What was the behavior or documentation before?

  • Using macro as an argument to an inline macro call (e.g., foo!(macro)) caused an internal compiler error (panic).
  • A macro declaration following a malformed item (e.g., const X with no type or value) would not act as a recovery boundary, potentially producing confusing cascading diagnostics.

What is the behavior or documentation after?

  • foo!(macro) is parsed correctly without panicking; macro is treated as a TokenTreeLeaf with kind TokenMacro.
  • Error recovery now stops at a macro keyword at module level, consistent with other module-item keywords like struct, fn, impl, etc.

Related issue or discussion (if any)

Regression fix for #10144.


Additional context

Two new test cases are added: one verifying the inline macro regression no longer ICEs, and one verifying that error recovery halts correctly at a following macro declaration.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

orizi commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

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

@orizi
orizi marked this pull request as ready for review June 22, 2026 11:44
@cursor

cursor Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Narrow parser/recovery dispatch changes with regression tests only; no semantic or runtime behavior beyond fixing panic and diagnostics.

Overview
Fixes an ICE when macro appears inside an inline macro invocation (e.g. foo!(macro)) by handling TerminalMacro in take_token_node, so the keyword is parsed as a TokenTreeLeaf instead of hitting unreachable!.

Error recovery at module level now treats macro like other item keywords (fn, struct, etc.) via module_item_kw!, so a following macro { ... } stops recovery after a broken item (e.g. incomplete const X) instead of swallowing or mis-parsing further tokens.

Regression tests cover foo!(macro) parse tree shape and diagnostics for const X + macro m { ... }.

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

@orizi orizi linked an issue Jun 22, 2026 that may be closed by this pull request

@TomerStarkware TomerStarkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

@TomerStarkware reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).

@orizi
orizi added this pull request to the merge queue Jun 23, 2026
Merged via the queue into main with commit 0077583 Jun 23, 2026
55 checks passed
@orizi
orizi deleted the orizi/06-22-bugfix_parser_make_macro_a_valid_recovery_terminal_and_tree_token branch June 23, 2026 17:12
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.

bug: Block comment in macro args panics compiler

3 participants