Skip to content

Allow : after pat_param - #162264

Open
coolreader18 wants to merge 2 commits into
rust-lang:mainfrom
coolreader18:pat_param-colon-follow
Open

coolreader18 wants to merge 2 commits into
rust-lang:mainfrom
coolreader18:pat_param-colon-follow

Conversation

@coolreader18

@coolreader18 coolreader18 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

As on the tin. Perhaps this requires a more formal decision process, but in my view this is something of a bug. Originally, pat_param was just the bikeshodden name of pat2015, c.f. #83386 (comment):

  • pat_no_or would be used for closure arguments; can we have something that talks about that (e.g., pat_arg, pat_param -- pattern for a closure argument)
  • pat_param is preferred because it is a parameter, not the argument passed to a call

But if it truly is a matcher for "a pattern you can use for a parameter", there shouldn't be a problem allowing it to be followed by a colon, given that that's kind of the definitional constraint for that grammar. And if, in the distant future, the language decides to separate the name and type of a parameter with something other than a colon, well, that'll need an edition change anyway.

In my opinion, this would resolve #87724: I'd imagine that most-if-not-all use cases for a pat followed by a colon are for matching function signatures or let statements, which don't let you use | patterns anyway. But, we get to have our cake and eat it too by still reserving : for future extensions to match expression (pat) patterns. If it ever is desired to make pat match the whole $pat : $ty syntax, as mentioned in #87724 (comment), that's something that can be done over an edition boundary and/or just for pat matchers.

@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 Sep 3, 2026
@rustbot

rustbot commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

r? @nnethercote

rustbot has assigned @nnethercote.
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

@nnethercote nnethercote added T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. labels Sep 3, 2026
@nnethercote

Copy link
Copy Markdown
Contributor

This definitely would need approval from the lang team.

cc @rust-lang/lang @rust-lang/lang-advisors
cc @petrochenkov

@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels Sep 4, 2026
@traviscross

Copy link
Copy Markdown
Contributor

Makes sense to me. Thanks @coolreader18, @nnethercote.

@rfcbot fcp merge lang

@rust-rfcbot

rust-rfcbot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

@traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 16, 2026
@tmandry

tmandry commented Sep 16, 2026

Copy link
Copy Markdown
Member

It's surprising we don't already allow this. Thanks for the PR.

@rfcbot reviewed

@rust-bors

rust-bors Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #163095) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

@traviscross traviscross added the needs-reference-pr This language change needs an approved Reference PR to proceed. label Sep 23, 2026
@scottmcm

scottmcm commented Sep 23, 2026

Copy link
Copy Markdown
Member

This absolutely makes sense to me. And if for some reason there's an edge case, we have the option later to introduce a new matcher that doesn't allow this.

@rfcbot reviewed


There's potentially some surprises that this could cause if people are writing new macros in e2015 (where param means pat_param), but surprises for people doing that doesn't really bother me.

If future if we do want colons for things like Some(x: u32) that wouldn't be impacted by this either because that inner pattern is a full pat, not a pat param.

@rust-rfcbot rust-rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Sep 23, 2026
@rust-rfcbot

Copy link
Copy Markdown
Collaborator

🔔 This is now entering its final comment period, as per the review above. 🔔

@traviscross

Copy link
Copy Markdown
Contributor

@rfcbot concern extend-migration-lint

Do we need to extend rust_2021_incompatible_or_patterns to handle pat followed by :? Let's please add a test for the migration here.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 23, 2026
@traviscross

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 23, 2026
@rustbot

rustbot commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@coolreader18

Copy link
Copy Markdown
Contributor Author

There's potentially some surprises that this could cause if people are writing new macros in e2015 (where param means pat_param), but surprises for people doing that doesn't really bother me.

Yeah I considered making it such that only explicit pat_param allows this, but figured it didn't matter very much.

@coolreader18

Copy link
Copy Markdown
Contributor Author

Actually maybe I will make it such that only explicit pat_param allows this? That way we don't need to edit a years-old migration lint to be now about "old versions of or-patterns OR param-followed-by-colon"

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-reference-pr This language change needs an approved Reference PR to proceed. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

$pat followed by a : is used in the language, but isn't allowed for MBE

7 participants