Skip to content

Commit 15b16fe

Browse files
committed
Fix incorrect suggestion for never patterns with bodies or guard
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).
1 parent 0ad983b commit 15b16fe

29 files changed

+213
-84
lines changed

compiler/rustc_ast_lowering/messages.ftl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,11 @@ ast_lowering_misplaced_relax_trait_bound =
133133
ast_lowering_never_pattern_with_body =
134134
a never pattern is always unreachable
135135
.label = this will never be executed
136-
.suggestion = remove this expression
136+
.suggestion = remove the match arm expression
137137
138138
ast_lowering_never_pattern_with_guard =
139139
a guard on a never pattern will never be run
140-
.suggestion = remove this guard
140+
.suggestion = remove the match arm guard
141141
142142
ast_lowering_no_precise_captures_on_apit = `use<...>` precise capturing syntax not allowed in argument-position `impl Trait`
143143

compiler/rustc_ast_lowering/src/errors.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ pub(crate) struct MisplacedRelaxTraitBound {
335335
pub(crate) struct MatchArmWithNoBody {
336336
#[primary_span]
337337
pub span: Span,
338-
#[suggestion(code = " => todo!(),", applicability = "has-placeholders")]
338+
#[suggestion(code = " => todo!(),", applicability = "has-placeholders", style = "verbose")]
339339
pub suggestion: Span,
340340
}
341341

@@ -344,16 +344,18 @@ pub(crate) struct MatchArmWithNoBody {
344344
pub(crate) struct NeverPatternWithBody {
345345
#[primary_span]
346346
#[label]
347-
#[suggestion(code = "", applicability = "maybe-incorrect")]
348347
pub span: Span,
348+
#[suggestion(code = ",", applicability = "maybe-incorrect", style = "verbose")]
349+
pub removal_span: Span,
349350
}
350351

351352
#[derive(Diagnostic)]
352353
#[diag(ast_lowering_never_pattern_with_guard)]
353354
pub(crate) struct NeverPatternWithGuard {
354355
#[primary_span]
355-
#[suggestion(code = "", applicability = "maybe-incorrect")]
356356
pub span: Span,
357+
#[suggestion(code = ",", applicability = "maybe-incorrect", style = "verbose")]
358+
pub removal_span: Span,
357359
}
358360

359361
#[derive(Diagnostic)]

compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,9 +685,33 @@ impl<'hir> LoweringContext<'_, 'hir> {
685685
self.dcx().emit_err(MatchArmWithNoBody { span, suggestion });
686686
}
687687
} else if let Some(body) = &arm.body {
688-
self.dcx().emit_err(NeverPatternWithBody { span: body.span });
688+
self.dcx().emit_err(NeverPatternWithBody {
689+
span: body.span,
690+
removal_span: if pat.span.eq_ctxt(arm.span) {
691+
// - ! => {}
692+
// + !,
693+
// FIXME: account for `(!|!)`, as pat.span ends *within* the parentheses.
694+
pat.span.shrink_to_hi().with_hi(arm.span.hi())
695+
} else {
696+
// Subtly incorrect, but close enough if macros are involved.
697+
// - ! => {}
698+
// + ! => ,
699+
body.span
700+
},
701+
});
689702
} else if let Some(g) = &arm.guard {
690-
self.dcx().emit_err(NeverPatternWithGuard { span: g.span });
703+
self.dcx().emit_err(NeverPatternWithGuard {
704+
span: g.span,
705+
removal_span: if pat.span.eq_ctxt(arm.span) {
706+
// - ! if cond,
707+
// + !,
708+
pat.span.shrink_to_hi().with_hi(arm.span.hi())
709+
} else {
710+
// We have something like `never!() if cond =>`
711+
// We just remove ^^^^ which isn't entirely correct.
712+
g.span
713+
},
714+
});
691715
}
692716

693717
// We add a fake `loop {}` arm body so that it typecks to `!`. The mir lowering of never

compiler/rustc_parse/src/parser/expr.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3175,13 +3175,13 @@ impl<'a> Parser<'a> {
31753175
arm_body = Some(expr);
31763176
// Eat a comma if it exists, though.
31773177
let _ = this.eat(exp!(Comma));
3178-
comma = Some(this.prev_token.span);
31793178
Ok(Recovered::No)
31803179
} else if let Some((span, guar)) =
31813180
this.parse_arm_body_missing_braces(&expr, arrow_span)
31823181
{
31833182
let body = this.mk_expr_err(span, guar);
31843183
arm_body = Some(body);
3184+
let _ = this.eat(exp!(Comma));
31853185
Ok(Recovered::Yes(guar))
31863186
} else {
31873187
let expr_span = expr.span;
@@ -3222,6 +3222,9 @@ impl<'a> Parser<'a> {
32223222
})
32233223
}
32243224
};
3225+
if let TokenKind::Comma = this.prev_token.kind {
3226+
comma = Some(this.prev_token.span);
3227+
}
32253228

32263229
let hi_span =
32273230
comma.unwrap_or(arm_body.as_ref().map_or(span_before_body, |body| body.span));

tests/ui/attributes/collapse-debuginfo-invalid.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ LL | #[collapse_debuginfo(yes)]
7676
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
7777
LL |
7878
LL | _ => (),
79-
| ------- not a macro definition
79+
| -------- not a macro definition
8080

8181
error: `collapse_debuginfo` attribute should be applied to macro definitions
8282
--> $DIR/collapse-debuginfo-invalid.rs:40:1

tests/ui/closures/2229_closure_analysis/match/issue-88331.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ LL | pub struct Opcode(pub u8);
1313
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
1414
|
1515
LL ~ Opcode::OP1 => unimplemented!(),
16-
LL ~ Opcode(0_u8) | Opcode(2_u8..=u8::MAX) => todo!(),
16+
LL + Opcode(0_u8) | Opcode(2_u8..=u8::MAX) => todo!()
1717
|
1818

1919
error[E0004]: non-exhaustive patterns: `Opcode2(Opcode(0_u8))` and `Opcode2(Opcode(2_u8..=u8::MAX))` not covered
@@ -31,7 +31,7 @@ LL | pub struct Opcode2(Opcode);
3131
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
3232
|
3333
LL ~ Opcode2::OP2=> unimplemented!(),
34-
LL ~ Opcode2(Opcode(0_u8)) | Opcode2(Opcode(2_u8..=u8::MAX)) => todo!(),
34+
LL + Opcode2(Opcode(0_u8)) | Opcode2(Opcode(2_u8..=u8::MAX)) => todo!()
3535
|
3636

3737
error: aborting due to 2 previous errors

tests/ui/coverage-attr/allowed-positions.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ error[E0788]: coverage attribute not allowed here
8282
LL | #[coverage(off)]
8383
| ^^^^^^^^^^^^^^^^
8484
LL | () => (),
85-
| -------- not a function, impl block, or module
85+
| --------- not a function, impl block, or module
8686
|
8787
= help: coverage attribute can be applied to a function (with body), impl block, or module
8888

tests/ui/feature-gates/feature-gate-never_patterns.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,13 @@ error: a guard on a never pattern will never be run
108108
--> $DIR/feature-gate-never_patterns.rs:54:19
109109
|
110110
LL | Err(!) if false,
111-
| ^^^^^ help: remove this guard
111+
| ^^^^^
112+
|
113+
help: remove the match arm guard
114+
|
115+
LL - Err(!) if false,
116+
LL + Err(!),
117+
|
112118

113119
error: aborting due to 13 previous errors
114120

tests/ui/force-inlining/invalid.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ LL | #[rustc_force_inline]
316316
| ^^^^^^^^^^^^^^^^^^^^^
317317
LL |
318318
LL | 1 => (),
319-
| ------- not a function definition
319+
| -------- not a function definition
320320

321321
error: attribute should be applied to a function
322322
--> $DIR/invalid.rs:98:5

tests/ui/pattern/usefulness/issue-15129.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ LL | match (T::T1(()), V::V2(true)) {
88
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
99
|
1010
LL ~ (T::T2(()), V::V2(b)) => (),
11-
LL ~ (T::T1(()), V::V2(_)) | (T::T2(()), V::V1(_)) => todo!(),
11+
LL + (T::T1(()), V::V2(_)) | (T::T2(()), V::V1(_)) => todo!()
1212
|
1313

1414
error: aborting due to 1 previous error

0 commit comments

Comments
 (0)