Skip to content

Allow range-to-inclusive macro patterns#158836

Open
chenyukang wants to merge 1 commit into
rust-lang:mainfrom
chenyukang:yukang-fix-120737-range-to-inclusive-pat-macro
Open

Allow range-to-inclusive macro patterns#158836
chenyukang wants to merge 1 commit into
rust-lang:mainfrom
chenyukang:yukang-fix-120737-range-to-inclusive-pat-macro

Conversation

@chenyukang

@chenyukang chenyukang commented Jul 6, 2026

Copy link
Copy Markdown
Member

Fixes #120737

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 6, 2026
@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 20 candidates

@chenyukang chenyukang force-pushed the yukang-fix-120737-range-to-inclusive-pat-macro branch from c413d1d to 72c1568 Compare July 6, 2026 04:06
@rustbot

rustbot commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@folkertdev

Copy link
Copy Markdown
Contributor

r? @nnethercote

Looks fine but it's macro parsing to there might be something subtle I'm missing

@rustbot rustbot assigned nnethercote and unassigned folkertdev Jul 7, 2026

/// Returns `true` if the token can appear at the start of a pattern.
///
/// Shamelessly borrowed from `can_begin_expr`, only used for diagnostics right now.

@nnethercote nnethercote Jul 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the "only used for diagnostics" is not true; can_begin_pattern is used in both nonterminal_may_begin_with and parse_pat_with_range_pat which affect code parsing.

View changes since the review

@chenyukang chenyukang Jul 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this was added only for diagnostic.

the parse_pat_with_range_pat invocation is also emitting error:

if self.token.is_keyword(kw::Let)
&& self.look_ahead(1, |tok| {
tok.can_begin_pattern(token::NtPatKind::PatParam { inferred: false })
})
{
self.bump();
// Trim extra space after the `let`
let span = lo.with_hi(self.token.span.lo());
self.dcx().emit_err(RemoveLet { span: lo, suggestion: span });
lo = self.token.span;
}

the nonterminal_may_begin_with is indeed expand its usage, with a comment here:
https://github.com/rust-lang/rust/pull/120221/changes#r1669391427
and
#120221 (comment)

@nnethercote

Copy link
Copy Markdown
Contributor

The change you've made is the obvious fix, but I'm uncertain about whether we can just accept the PR, or if we need some extra verification (e.g. crater run, or lang team involvement). @petrochenkov, what do you think?

@petrochenkov petrochenkov self-assigned this Jul 8, 2026
@petrochenkov

Copy link
Copy Markdown
Contributor

Any changes to fn can_begin_* functions can cause observable changes in macro matching, from what I remember.
I'd start with constructing an example showing how it changes in this specific case.

I probably wouldn't bother running crater for this specific change, unless it's rolled up with something else or as a part of beta run.

@petrochenkov petrochenkov removed their assignment Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(..=0 | 2) in macro does not compile without parentheses but generates warning for unnecessary parentheses

5 participants