Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose #137409

estebank · 2025-02-22T01:32:17Z

Better track trailing commas in match arms. Do not suggest adding trailing comma to match arm with block body. Better heuristic for "is this match in one line".

When encountering an unreachable match arm, (correctly) suggest removing the entire arm:

error: a never pattern is always unreachable
  --> $DIR/ICE-130779-never-arm-no-oatherwise-block.rs:10:20
   |
LL |         ! if true => {}
   |                      ^^ this will never be executed
   |
help: remove the unreachable match arm
   |
LL -         ! if true => {}
   |

Noticed in #137343 (comment).

r? @compiler-errors

The first commit is independent of the second, but to make the second one produce accurate suggestions the span needs to include the trailing comma, hence the grouping of both changes in this PR.

rustbot · 2025-02-22T01:32:26Z

Some changes occurred in match checking

cc @Nadrieril

tests/ui/pattern/usefulness/empty-match.exhaustive_patterns.stderr

tests/ui/pattern/usefulness/empty-types.never_pats.stderr

rustbot · 2025-02-22T05:00:10Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler-errors · 2025-02-28T03:26:07Z

compiler/rustc_ast_lowering/src/expr.rs

@@ -676,6 +676,23 @@ impl<'hir> LoweringContext<'_, 'hir> {
        {
            self.lower_expr(body)
        } else {
+            let removal_span = |mut removal_span: Span| {


Is this necessary? Can you use find_ancestor_in_same_ctxt instead of cooking it yourself?

You should be able to do like... pat_span.find_ancestor_in_same_ctxt(arm_span).

Done (amended last commit).

compiler-errors · 2025-02-28T17:08:54Z

r=me when CI is green

compiler-errors · 2025-02-28T17:10:23Z

Also @estebank please for this PR and all others in the future, can you make sure to note that you're making these suggestions verbose. Seems like a major thing to leave out when that's presumably your underlying motivation here. Thx

estebank · 2025-02-28T17:17:25Z

tests/ui/pattern/usefulness/empty-match.normal.stderr

+error[E0004]: non-exhaustive patterns: `[]` not covered
+  --> $DIR/empty-match.rs:81:15
   |
-LL ~                 _ if false => {},
-LL +                 [] => todo!()
+LL |     indirect!(arrayN_of_empty);
+   |               ^^^^^^^^^^^^^^^ pattern `[]` not covered
   |
+   = note: the matched value is of type `[!; N]`
+note: within macro `match_guarded_arm`, this `match` expression doesn't expand to cover all patterns
+  --> $DIR/empty-match.rs:16:13
+   |
+LL | /      macro_rules! match_guarded_arm {
+LL | |          ($e:expr) => {
+LL | |/             match $e {
+LL | ||                 _ if false => {}
+LL | ||             }
+   | ||_____________^
+LL | |          };
+LL | |      }
+   | |______-
+...
+LL |        indirect!(arrayN_of_empty);
+   |        -------------------------- in this macro invocation
+   = note: match arms with guards don't count towards exhaustivity
+   = help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern
+   = note: this error originates in the macro `indirect` (in Nightly builds, run with -Z macro-backtrace for more info)


Despite the verbosity of this output, I am happy with how much context it provides in a really tricky case. With macro-backtrace, the output is outright great (IMO):

error[E0004]: non-exhaustive patterns: `[]` not covered --> tests/ui/pattern/usefulness/empty-match.rs:81:15 | 81 | indirect!(arrayN_of_empty); //~ ERROR `[]` not covered | ^^^^^^^^^^^^^^^ pattern `[]` not covered | = note: the matched value is of type `[!; N]` note: within macro `match_guarded_arm`, this `match` expression doesn't expand to cover all patterns --> tests/ui/pattern/usefulness/empty-match.rs:16:13 | 14 | / macro_rules! match_guarded_arm { 15 | | ($e:expr) => { 16 | |/ match $e { 17 | || _ if false => {} 18 | || } | ||_____________^ 19 | | }; 20 | | } | |______- 21 | 22 | / macro_rules! indirect { 23 | | ($e:expr) => { 24 | | match_guarded_arm!($e) 25 | | }; 26 | | } | |_____- in this expansion of `indirect!` ... 81 | indirect!(arrayN_of_empty); //~ ERROR `[]` not covered | -------------------------- in this macro invocation = note: match arms with guards don't count towards exhaustivity = help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern

estebank · 2025-02-28T17:19:11Z

tests/ui/pattern/bindings-after-at/borrowck-pat-ref-mut-and-ref.stderr

 LL |     let ref mut a @ (ref b @ ref mut c) = u(); // sub-in-sub
-   |         ^^^^^^^^^    -----   --------- value is mutably borrowed by `c` here
-   |         |            |
-   |         |            value is borrowed by `b` here
+   |         ^^^^^^^^^   ------   --------- value is mutably borrowed by `c` here
+   |         |           |
+   |         |           value is borrowed by `b` here


This is technically a regression that I haven't figured out how to get rid of without breaking anything else :-/

estebank · 2025-02-28T18:54:33Z

@bors r=compiler-errors

bors · 2025-02-28T18:54:36Z

📌 Commit 88398eb has been approved by compiler-errors

It is now in the queue for this repository.

Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose Better track trailing commas in match arms. Do not suggest adding trailing comma to match arm with block body. Better heuristic for "is this match in one line". When encountering an unreachable match arm, (correctly) suggest removing the entire arm: ``` error: a never pattern is always unreachable --> $DIR/ICE-130779-never-arm-no-oatherwise-block.rs:10:20 | LL | ! if true => {} | ^^ this will never be executed | help: remove the unreachable match arm | LL - ! if true => {} | ``` Noticed in rust-lang#137343 (comment). r? `@compiler-errors` The first commit is independent of the second, but to make the second one produce accurate suggestions the span needs to include the trailing comma, hence the grouping of both changes in this PR.

matthiaskrgr · 2025-03-01T10:33:34Z

@bors r-
needs rebase for tests/ui/never_type/unused_trait_in_never_pattern_body.rs from what I see

Better track trailing commas in match arms. Do not suggest adding trailing comma to match arm with block body. Better heuristic for "is this match in one line".

When encountering an unreachable match arm, (correctly) suggest removing the entire arm: ``` error: a never pattern is always unreachable --> $DIR/ICE-130779-never-arm-no-oatherwise-block.rs:10:20 | LL | Some(!) if true => {} | ^^ this will never be executed | help: remove the unreachable match arm | LL - Some(!) if true => {} LL + Some(!), | ``` Noticed in rust-lang#137343 (comment).

``` error[E0004]: non-exhaustive patterns: `u8::MAX` not covered --> $DIR/exhaustiveness.rs:47:8 | LL | m!(0u8, 0..255); | ^^^ pattern `u8::MAX` not covered | = note: the matched value is of type `u8` note: within macro `m`, this `match` expression doesn't expand to cover all patterns --> $DIR/exhaustiveness.rs:7:9 | LL | / macro_rules! m { LL | | ($s:expr, $($t:tt)+) => { LL | | match $s { $($t)+ => {} } | | ^^^^^^^^^^^^^^^^^^^^^^^^^ LL | | } LL | | } | |_- = help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern ```

Parentheses surrounding patterns are not ketp in the HIR (and are warned against in the AST). In order to avoid having some suggestions break, we keep the outer span (including parentheses) when lowering the patterns. ``` error: a never pattern is always unreachable --> $DIR/ICE-133117-duplicate-never-arm.rs:9:26 | LL | (!|!) if true => {} | ^^ this will never be executed | help: remove the match arm expression | LL - (!|!) if true => {} LL + (!|!), | ``` ``` error[E0308]: mismatched types --> $DIR/well-typed-edition-2024.rs:135:15 | LL | let [&mut &(mut x)] = &mut [&0]; | ^^^^^^^^ --------- this expression has type `&mut [&{integer}; 1]` | | | expected integer, found `&_` | = note: expected type `{integer}` found reference `&_` help: consider removing `&` from the pattern | LL - let [&mut &(mut x)] = &mut [&0]; LL + let [&mut (mut x)] = &mut [&0]; | ```

``` error: a never pattern is always unreachable --> $DIR/pattern-behind-macro.rs:13:21 | LL | never!() => {} | ^^ this will never be executed | help: remove the match arm expression | LL - never!() => {} LL + never!(), | ``` Look up the macro backtrace call sites to see if we find where the macro was used as a pattern, to properly suggest removing match arm guard and body.

compiler-errors · 2025-03-01T18:06:54Z

tests/ui/never_type/unused_trait_in_never_pattern_body.stderr

+LL -         ! => || {
+LL -
+LL -
+LL -             use std::ops::Add;
+LL -             0.add(1)
+LL -         },
+LL +         !,


This is a lot of lines, to the point that it's a bit hard to even see (without coloration) that this is even suggesting to leave the plain ! and remove everything else. we should probably make this suggestion hidden?

estebank · 2025-03-01T19:51:24Z

tests/ui/rfcs/rfc-0000-never_patterns/ICE-130779-never-arm-no-oatherwise-block.stderr

-   |                    help: remove this expression
+   |                    ^^ this will never be executed
+   |
+   = help: remove the match arm expression


This probably be "remove the match arm guard and expression" now.

estebank · 2025-03-01T19:51:52Z

tests/ui/rfcs/rfc-0000-never_patterns/ICE-130779-never-arm-no-oatherwise-block-fix.stderr

+help: remove the `|`
+   |
+LL -         ! |
+LL +         !
+   |


Fix for the left-over whitespace coming soon. Edit: #137872.

compiler-errors · 2025-03-02T19:01:37Z

Do you plan on tweaking this further? If not, please open a tracking issue (https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fwg-diagnostics/topic/Why.20are.20we.20making.20suggestions.20verbose.3F/near/502691049) and it would be nice to cc that on this PR and any further PRs having to do with verbose-ifying diagnostics. r=me afterwards.

estebank · 2025-03-03T18:28:40Z

@bors r=compiler-errors

bors · 2025-03-03T18:28:43Z

📌 Commit 7e1f8f2 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2025-03-03T18:28:43Z

🌲 The tree is currently closed for pull requests below priority 102. This pull request will be tested once the tree is reopened.

compiler-errors · 2025-03-03T18:50:27Z

@bors r-

needs tracking issue

rustbot assigned compiler-errors Feb 22, 2025

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 Feb 22, 2025

estebank commented Feb 22, 2025

View reviewed changes

tests/ui/pattern/usefulness/empty-match.exhaustive_patterns.stderr Outdated Show resolved Hide resolved

estebank commented Feb 22, 2025

View reviewed changes

tests/ui/pattern/usefulness/empty-types.never_pats.stderr Show resolved Hide resolved

This comment has been minimized.

Sign in to view

estebank force-pushed the match-arm branch from d112fca to 15b16fe Compare February 22, 2025 02:33

This comment has been minimized.

Sign in to view

estebank force-pushed the match-arm branch from 15b16fe to 522a9f8 Compare February 22, 2025 05:00

estebank force-pushed the match-arm branch from 522a9f8 to af9fe58 Compare February 25, 2025 21:54

This comment has been minimized.

Sign in to view

estebank force-pushed the match-arm branch from c35c3bb to b2b3e2b Compare February 26, 2025 17:22

compiler-errors reviewed Feb 28, 2025

View reviewed changes

estebank force-pushed the match-arm branch from b2b3e2b to 88398eb Compare February 28, 2025 17:02

compiler-errors changed the title ~~Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion~~ Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose Feb 28, 2025

estebank commented Feb 28, 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 Feb 28, 2025

matthiaskrgr mentioned this pull request Mar 1, 2025

Rollup of 9 pull requests #137839

Closed

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 1, 2025

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 1, 2025

estebank added 7 commits March 1, 2025 17:30

Tweak "add match arm" suggestion

5d45d1e

Better track trailing commas in match arms. Do not suggest adding trailing comma to match arm with block body. Better heuristic for "is this match in one line".

fix clippy tests

049e437

fix rebase

3c26d34

estebank force-pushed the match-arm branch from 88398eb to 3c26d34 Compare March 1, 2025 18:03

compiler-errors reviewed Mar 1, 2025

View reviewed changes

Make never pattern suggestions hidden

7e1f8f2

estebank commented Mar 1, 2025

View reviewed changes

estebank mentioned this pull request Mar 1, 2025

Include whitespace in "remove |" suggestion and make it hidden #137872

Open

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 3, 2025

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose #137409

Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose #137409

estebank commented Feb 22, 2025

rustbot commented Feb 22, 2025

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 22, 2025

This comment has been minimized.

This comment has been minimized.

compiler-errors Feb 28, 2025

compiler-errors Feb 28, 2025

estebank Feb 28, 2025

compiler-errors commented Feb 28, 2025

compiler-errors commented Feb 28, 2025

estebank Feb 28, 2025

estebank Feb 28, 2025

estebank commented Feb 28, 2025

bors commented Feb 28, 2025

matthiaskrgr commented Mar 1, 2025

compiler-errors Mar 1, 2025

estebank Mar 1, 2025

estebank Mar 1, 2025 •

edited

Loading

compiler-errors commented Mar 2, 2025

estebank commented Mar 3, 2025

bors commented Mar 3, 2025

bors commented Mar 3, 2025

compiler-errors commented Mar 3, 2025

Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose #137409

Are you sure you want to change the base?

Tweak comma handling of "missing match arm" suggestion and fix "remove this arm" suggestion, and make suggestion verbose #137409

Conversation

estebank commented Feb 22, 2025

rustbot commented Feb 22, 2025

This comment has been minimized.

This comment has been minimized.

rustbot commented Feb 22, 2025

This comment has been minimized.

This comment has been minimized.

compiler-errors Feb 28, 2025

Choose a reason for hiding this comment

compiler-errors Feb 28, 2025

Choose a reason for hiding this comment

estebank Feb 28, 2025

Choose a reason for hiding this comment

compiler-errors commented Feb 28, 2025

compiler-errors commented Feb 28, 2025

estebank Feb 28, 2025

Choose a reason for hiding this comment

estebank Feb 28, 2025

Choose a reason for hiding this comment

estebank commented Feb 28, 2025

bors commented Feb 28, 2025

matthiaskrgr commented Mar 1, 2025

compiler-errors Mar 1, 2025

Choose a reason for hiding this comment

estebank Mar 1, 2025

Choose a reason for hiding this comment

estebank Mar 1, 2025 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Mar 2, 2025

estebank commented Mar 3, 2025

bors commented Mar 3, 2025

bors commented Mar 3, 2025

compiler-errors commented Mar 3, 2025

estebank Mar 1, 2025 •

edited

Loading