Conversation
PR SummaryLow Risk Overview A semantic inline-macro test documents the intended behavior: a user Reviewed by Cursor Bugbot for commit 106d3f8. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
💡 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".
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.
106d3f8 to
3037636
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).
orizi
left a comment
There was a problem hiding this comment.
@orizi resolved 1 discussion.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on eytan-starkware and TomerStarkware).
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on eytan-starkware).

Summary
Adds support for
$defsitevariant patterns in user-defined macros. The parser now recognizesTerminalDollaras 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:
Why is this change needed?
Previously, using
$defsite::SomeTypeas a pattern in amatchexpression inside a user-defined macro body would fail to parse, becauseTerminalDollarwas 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:
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
TerminalDollaras a valid start of a path pattern, alongsideTerminalIdentifier. A semantic test covering the$defsitevariant pattern case is included.