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

fix(parser): accept resolver-modifier paths in pattern position - #10039

Merged
orizi merged 1 commit into
mainfrom
orizi/06-04-fix_parser_accept_resolver-modifier_paths_in_pattern_position
Jun 7, 2026
Merged

orizi merged 1 commit into
mainfrom
orizi/06-04-fix_parser_accept_resolver-modifier_paths_in_pattern_position

Conversation

@orizi

@orizi orizi commented Jun 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds support for $defsite variant patterns in user-defined macros. The parser now recognizes TerminalDollar as a valid start token when parsing patterns, allowing $defsite::... paths to be used in match arms within macro bodies.


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?

Previously, using $defsite::SomeType as a pattern in a match expression inside a user-defined macro body would fail to parse, because TerminalDollar was not recognized as a valid pattern start token. This prevented macros from referencing definition-site paths in match arms.


What was the behavior or documentation before?

A macro like:

macro unwrap_or_zero {
    ($opt: expr) => {
        match $opt {
            $defsite::Option::Some(x) => x,
            $defsite::Option::None => 0,
        }
    };
}

would fail to parse because $defsite::... was not a valid pattern.


What is the behavior or documentation after?

$defsite::... paths are now valid in pattern position within macro bodies, enabling macros to match against definition-site enum variants without diagnostics.


Related issue or discussion (if any)


Additional context

The fix is a one-line change in the pattern parser to also accept TerminalDollar as a valid start of a path pattern, alongside TerminalIdentifier. A semantic test covering the $defsite variant pattern case is included.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

orizi commented Jun 4, 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 6, 2026 16:25
@cursor

cursor Bot commented Jun 6, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Single-arm extension in pattern parsing aligned with existing $ path handling elsewhere; covered by a new inline-macro diagnostic test.

Overview
Pattern parsing now accepts paths that start with $ (resolver-modifier / macro-site paths), not only plain identifiers. In try_parse_pattern, TerminalDollar is handled like TerminalIdentifier and routed through parse_path(), so constructs such as $defsite::Option::Some(x) in match arms inside expanded macros parse correctly.

A semantic inline-macro test documents the intended behavior: a user unwrap_or_zero! macro whose body matches on $defsite::Option::Some / None with no expected diagnostics.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 106d3f8a57

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/cairo-lang-parser/src/parser.rs
@orizi
orizi marked this pull request as draft June 7, 2026 07:39
  try_parse_pattern only began path parsing on TerminalIdentifier, so a pattern
  starting with `$` (a `$defsite::`/`$callsite::` resolver-modifier path, e.g. in
  macro-expanded code) fell through to `Err(SkipToken)` and produced
  "Skipped tokens. Expected: pattern". Expression-position path parsing already
  handles the leading `$`, and the pattern code already destructures the path's
  [dollar, inner] children - so just match TerminalDollar alongside
  TerminalIdentifier. Add an inline-macros regression test using a
  `$defsite::Option::Some(x)` pattern.
@orizi
orizi force-pushed the orizi/06-04-fix_parser_accept_resolver-modifier_paths_in_pattern_position branch from 106d3f8 to 3037636 Compare June 7, 2026 08:05

@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 made 1 comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).

Comment thread crates/cairo-lang-parser/src/parser.rs

@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 resolved 1 discussion.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on eytan-starkware and TomerStarkware).

@orizi
orizi marked this pull request as ready for review June 7, 2026 08:51

@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 3 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 7, 2026
Merged via the queue into main with commit 2dfb215 Jun 7, 2026
53 checks passed
@orizi
orizi deleted the orizi/06-04-fix_parser_accept_resolver-modifier_paths_in_pattern_position branch June 7, 2026 11:25
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