Add new `PatKind::Missing` variants #139035

nnethercote · 2025-03-27T23:05:57Z

To avoid some ugly uses of kw::Empty when handling "missing" patterns, e.g. in bare fn tys. Helps with #137978. Details in the individual commits.

r? @oli-obk

"Missing" patterns are possible in bare fn types (`fn f(u32)`) and similar places. Currently these are represented in the AST with `ast::PatKind::Ident` with no `by_ref`, no `mut`, an empty ident, and no sub-pattern. This flows through to `{hir,thir}::PatKind::Binding` for HIR and THIR. This is a bit nasty. It's very non-obvious, and easy to forget to check for the exceptional empty identifier case. This commit adds a new variant, `PatKind::Missing`, to do it properly. The process I followed: - Add a `Missing` variant to `{ast,hir,thir}::PatKind`. - Chang `parse_param_general` to produce `ast::PatKind::Missing` instead of `ast::PatKind::Missing`. - Look through `kw::Empty` occurrences to find functions where an existing empty ident check needs replacing with a `PatKind::Missing` check: `print_param`, `check_trait_item`, `is_named_param`. - Add a `PatKind::Missing => unreachable!(),` arm to every exhaustive match identified by the compiler. - Find which arms are actually reachable by running the test suite, changing them to something appropriate, usually by looking at what would happen to a `PatKind::Ident`/`PatKind::Binding` with no ref, no `mut`, an empty ident, and no subpattern. Quite a few of the `unreachable!()` arms were never reached. This makes sense because `PatKind::Missing` can't happen in every pattern, only in places like bare fn tys and trait fn decls. I also tried an alternative approach: modifying `ast::Param::pat` to hold an `Option<P<Pat>>` instead of a `P<Pat>`, but that quickly turned into a very large and painful change. Adding `PatKind::Missing` is much easier.

Thanks to the introduction of `PatKind::Missing`.

nnethercote · 2025-03-28T04:07:07Z

@bors try @rust-timer queue

Add new `PatKind::Missing` variants To avoid some ugly uses of `kw::Empty` when handling "missing" patterns, e.g. in bare fn tys. Helps with rust-lang#137978. Details in the individual commits. r? `@ghost`

bors · 2025-03-28T04:08:19Z

⌛ Trying commit 909f449 with merge b1bc47e...

bors · 2025-03-28T06:18:04Z

☀️ Try build successful - checks-actions
Build commit: b1bc47e (b1bc47effce412da5d78921d97b88415d6cbc1d4)

rust-timer · 2025-03-28T08:11:06Z

Finished benchmarking commit (b1bc47e): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary -0.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-0.9%, -0.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.9%	[-0.9%, -0.9%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: missing data
Artifact size: 365.91 MiB -> 365.92 MiB (0.00%)

nnethercote · 2025-03-28T09:06:42Z

Perf-neutral, which is good.

@bors rollup=maybe

rustbot · 2025-03-28T09:06:47Z

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in match checking

cc @Nadrieril

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

oli-obk · 2025-03-28T12:19:39Z

modifying ast::Param::pat to
hold an Option<P<Pat>> instead of a P<Pat>, but that quickly turned
into a very large and painful change. Adding PatKind::Missing is much
easier.

I want to try making FnDecl generic over the Param or Ty before committing to this PR. If you want to give this a try, I will likely start doing this mid-next week otherwise.

nnethercote · 2025-03-28T22:17:18Z

I want to try making FnDecl generic over the Param or Ty before committing to this PR

I tried that too, it got ugly pretty quickly, but maybe you can find a way to do it that's nice.

ytmimi · 2025-03-29T02:42:22Z

src/tools/rustfmt/src/patterns.rs

@@ -42,6 +42,7 @@ pub(crate) fn is_short_pattern(

 fn is_short_pattern_inner(context: &RewriteContext<'_>, pat: &ast::Pat) -> bool {
    match &pat.kind {
+        ast::PatKind::Missing => unreachable!(),


Could you briefly explain how we know this is unreachable.

Gonna assume "no tests were hit" 😆

oli-obk · 2025-04-07T09:46:59Z

Ok turns out we actually support parameter names in function pointer types!?!

type Foo = fn(foo: ());

fn foo(f: fn(foo: ())) {}

we just do not support patterns. what the heck. Can we break that. Is like anyone actually writing that

nnethercote · 2025-04-07T09:52:36Z

Ok turns out we actually support parameter names in function pointer types!?!
we just do not support patterns.

Same is true for fn decls in extern blocks, and maybe bodyless fn decls in trait blocks (not certain about that last one).

oli-obk · 2025-04-07T09:54:38Z

Same is true for fn decls in extern blocks, and maybe bodyless fn decls in trait blocks (not certain about that last one).

those make sense. But function pointers are types, the information is useless even to callers as they never see it across crates.

oli-obk · 2025-04-07T09:58:11Z

Ok, finally convinced

@bors r+

it got ugly pretty quickly

it was nice (very subjectively), but obviously not feasible

bors · 2025-04-07T09:58:14Z

📌 Commit 909f449 has been approved by oli-obk

It is now in the queue for this repository.

…-obk Add new `PatKind::Missing` variants To avoid some ugly uses of `kw::Empty` when handling "missing" patterns, e.g. in bare fn tys. Helps with rust-lang#137978. Details in the individual commits. r? `@oli-obk`

Rollup of 8 pull requests Successful merges: - rust-lang#138603 (Report line number of test when should_panic test failed) - rust-lang#139035 (Add new `PatKind::Missing` variants) - rust-lang#139112 (Implement `super let`) - rust-lang#139365 (Default auto traits: fix perf) - rust-lang#139397 (coverage: Build the CGU's global file table as late as possible) - rust-lang#139455 ( Remove support for `extern "rust-intrinsic"` blocks) - rust-lang#139461 (Stop calling `source_span` query in significant drop order code) - rust-lang#139466 (Trivial tweaks to stop tracking source span directly) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 9 pull requests Successful merges: - rust-lang#139035 (Add new `PatKind::Missing` variants) - rust-lang#139108 (Simplify `thir::PatKind::ExpandedConstant`) - rust-lang#139112 (Implement `super let`) - rust-lang#139365 (Default auto traits: fix perf) - rust-lang#139397 (coverage: Build the CGU's global file table as late as possible) - rust-lang#139455 ( Remove support for `extern "rust-intrinsic"` blocks) - rust-lang#139461 (Stop calling `source_span` query in significant drop order code) - rust-lang#139465 (add sret handling for scalar autodiff) - rust-lang#139466 (Trivial tweaks to stop tracking source span directly) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139035 - nnethercote:PatKind-Missing, r=oli-obk Add new `PatKind::Missing` variants To avoid some ugly uses of `kw::Empty` when handling "missing" patterns, e.g. in bare fn tys. Helps with rust-lang#137978. Details in the individual commits. r? ``@oli-obk``

…ng, r=GuillaumeGomez rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) **(0)** PR rust-lang#136411 aimed to stop rendering unnamed params of fn ptr types as underscores in the common case (e.g., `fn(_: i32)` → `fn(i32)`) to make the rendered output stylistically more conventional. **(0.a)** However, since the cleaning fn that the PR modified is also used for lowering the HIR params of foreign fns and required assoc fns in traits, it accidentally butchered the rendering of the latter two: ```rs pub trait Trait { fn assoc_fn(_: i32); } // as well as (Rust 2015 only): fn assoc_fn(i32); unsafe extern "C" { pub fn foreign_fn(_: i32); } // Since 1.86 the fns above gets mis-rendered as: pub fn assoc_fn(: i32) // <-- BUTCHERED pub unsafe extern "C" fn foreign_fn(: i32) // <-- BUTCHERED ``` **(0.b)** Furthermore, it broke parity with middle cleaning (which includes inlined cross-crate re-exports) re-regressing parts of rust-lang#44306 I once fixed in PR rust-lang#103885. **(1)** Lastly, PR rust-lang#139035 introduced an ICE triggered by the following input file: ```rs trait Trait { fn anon(()) {} } // internal error: entered unreachable code ``` --- This PR fixes all of these regressions and in the first commit renames several types and fns to be more ~~correct~~ descriptive and legible. ~~It also refactors `Param.name` to be of type `Option<Symbol>` instead `Symbol` (where `None` ~ `kw::Empty`), so rendering mistakes like that can no longer creep in like that (ignoring tests). CC rust-lang#137978.~~ Independently done in PR rust-lang#139846 a day prior.

nnethercote added 2 commits March 28, 2025 09:18

Remove kw::Extra checks that are no longer necessary.

909f449

Thanks to the introduction of `PatKind::Missing`.

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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 27, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 28, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 28, 2025

nnethercote marked this pull request as ready for review March 28, 2025 09:06

rustbot assigned oli-obk Mar 28, 2025

ytmimi reviewed Mar 29, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2025

Zalathar mentioned this pull request Apr 7, 2025

Rollup of 8 pull requests #139478

Closed

Zalathar mentioned this pull request Apr 7, 2025

Rollup of 9 pull requests #139482

Merged

bors merged commit 82df622 into rust-lang:master Apr 7, 2025
7 checks passed

rustbot added this to the 1.88.0 milestone Apr 7, 2025

nnethercote deleted the PatKind-Missing branch April 7, 2025 22:31

This was referenced Apr 8, 2025

ICE: delegation: liveness: index out of bounds #139512

Closed

ICE: unreachable #139633

Closed

fmease mentioned this pull request Apr 16, 2025

rustdoc/clean: Fix lowering of fn params (fixes correctness & HIR vs. middle parity regressions) #139913

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new `PatKind::Missing` variants #139035

Add new `PatKind::Missing` variants #139035

nnethercote commented Mar 27, 2025 •

edited

Loading

nnethercote commented Mar 28, 2025

This comment has been minimized.

bors commented Mar 28, 2025

bors commented Mar 28, 2025

This comment has been minimized.

rust-timer commented Mar 28, 2025

nnethercote commented Mar 28, 2025

rustbot commented Mar 28, 2025

oli-obk commented Mar 28, 2025

nnethercote commented Mar 28, 2025

ytmimi Mar 29, 2025

oli-obk Apr 7, 2025

oli-obk commented Apr 7, 2025

nnethercote commented Apr 7, 2025

oli-obk commented Apr 7, 2025

oli-obk commented Apr 7, 2025

bors commented Apr 7, 2025

Add new PatKind::Missing variants #139035

Add new PatKind::Missing variants #139035

Conversation

nnethercote commented Mar 27, 2025 • edited Loading

nnethercote commented Mar 28, 2025

This comment has been minimized.

bors commented Mar 28, 2025

bors commented Mar 28, 2025

This comment has been minimized.

rust-timer commented Mar 28, 2025

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

nnethercote commented Mar 28, 2025

rustbot commented Mar 28, 2025

oli-obk commented Mar 28, 2025

nnethercote commented Mar 28, 2025

ytmimi Mar 29, 2025

Choose a reason for hiding this comment

oli-obk Apr 7, 2025

Choose a reason for hiding this comment

oli-obk commented Apr 7, 2025

nnethercote commented Apr 7, 2025

oli-obk commented Apr 7, 2025

oli-obk commented Apr 7, 2025

bors commented Apr 7, 2025

Add new `PatKind::Missing` variants #139035

Add new `PatKind::Missing` variants #139035

nnethercote commented Mar 27, 2025 •

edited

Loading