Skip to content

Commit 3ea2fbc

Browse files
borsbors
committed
Auto merge of #154010 - estebank:issue-42753, r=nnethercote
Suggest using equality comparison instead of pattern matching on non-structural constant in pattern When encountering a pattern containing a non-structural constant (not marked as `#[derive(PartialEq)]` to make it suitable for pattern matching, `C` in the examples below), we would previously not provide additional guidance. With this PR, the `help` in the following examples are added: ``` error: constant of non-structural type `partial_eq::S` in a pattern --> $DIR/suggest_equality_comparison_instead_of_pattern_matching.rs:16:18 | LL | struct S; | -------- `partial_eq::S` must be annotated with `#[derive(PartialEq)]` to be usable in patterns ... LL | const C: S = S; | ---------- constant defined here ... LL | Some(C) => {} | ^ constant of non-structural type | note: the `PartialEq` trait must be derived, manual `impl`s are not sufficient; see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details --> $DIR/suggest_equality_comparison_instead_of_pattern_matching.rs:5:5 | LL | impl PartialEq<S> for S { | ^^^^^^^^^^^^^^^^^^^^^^^ help: add a condition to the match arm checking for equality | LL - Some(C) => {} LL + Some(binding) if binding == C => {} | ``` ``` error: constant of non-structural type `partial_eq::S` in a pattern --> $DIR/suggest_equality_comparison_instead_of_pattern_matching.rs:22:18 | LL | struct S; | -------- `partial_eq::S` must be annotated with `#[derive(PartialEq)]` to be usable in patterns ... LL | const C: S = S; | ---------- constant defined here ... LL | let Some(C) = Some(S) else { return; }; | ^ constant of non-structural type | note: the `PartialEq` trait must be derived, manual `impl`s are not sufficient; see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details --> $DIR/suggest_equality_comparison_instead_of_pattern_matching.rs:5:5 | LL | impl PartialEq<S> for S { | ^^^^^^^^^^^^^^^^^^^^^^^ help: check for equality instead of pattern matching | LL - let Some(C) = Some(S) else { return; }; LL + if Some(C) == Some(S) { return; }; | ``` The suggestion accounts for a few conditions: - if the type is not from the local crate and has no `PartialEq` impl, the user can't make it structural, so we don't provide the suggestion - regardless of whether the type is local or remote, if it has a manual `PartialEq`, explain that with a derived `PartialEq` you could use equality - if the type is local and has no impl, suggest adding a derived `PartialEq` and use equality check instead of pattern matching - when suggesting equality, account for `if-let` to suggest chaining (edition dependent), `match` arm with a present `if` check, `match` arm without an existing `if` check - when encountering `let-else`, we suggest turning it into an `if` expression instead (this doesn't check for additional bindings beyond the constant, which would suggest incorrect code in some more complex cases). Fix #42753.
2 parents 6d8bc65 + 801eafd commit 3ea2fbc

22 files changed

Lines changed: 658 additions & 13 deletions

‎compiler/rustc_mir_build/src/errors.rs‎

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,17 +1053,115 @@ pub(crate) struct TypeNotStructural<'tcx> {
10531053
#[primary_span]
10541054
#[label("constant of non-structural type")]
10551055
pub(crate) span: Span,
1056-
#[label("`{$ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns")]
1056+
#[label(
1057+
"{$is_local ->
1058+
*[true] `{$ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
1059+
[false] `{$ty}` is not usable in patterns
1060+
}"
1061+
)]
10571062
pub(crate) ty_def_span: Span,
10581063
pub(crate) ty: Ty<'tcx>,
10591064
#[note(
1060-
"the `PartialEq` trait must be derived, manual `impl`s are not sufficient; see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details"
1065+
"the `PartialEq` trait must be derived, manual `impl`s are not sufficient; see \
1066+
https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details"
10611067
)]
10621068
pub(crate) manual_partialeq_impl_span: Option<Span>,
10631069
#[note(
10641070
"see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details"
10651071
)]
10661072
pub(crate) manual_partialeq_impl_note: bool,
1073+
#[subdiagnostic]
1074+
pub(crate) suggestion: Option<SuggestEq<'tcx>>,
1075+
pub(crate) is_local: bool,
1076+
}
1077+
1078+
#[derive(Subdiagnostic)]
1079+
pub(crate) enum SuggestEq<'tcx> {
1080+
#[multipart_suggestion(
1081+
"{$manual_partialeq_impl ->
1082+
[false] if `{$ty}` manually implemented `PartialEq`, you could add
1083+
*[true] add
1084+
} a condition to the match arm checking for equality",
1085+
applicability = "maybe-incorrect",
1086+
style = "verbose"
1087+
)]
1088+
AddIf {
1089+
#[suggestion_part(code = "binding")]
1090+
pat_span: Span,
1091+
#[suggestion_part(code = " if binding == {name}")]
1092+
if_span: Span,
1093+
name: String,
1094+
ty: Ty<'tcx>,
1095+
manual_partialeq_impl: bool,
1096+
},
1097+
#[multipart_suggestion(
1098+
"{$manual_partialeq_impl ->
1099+
[false] if `{$ty}` manually implemented `PartialEq`, you could add
1100+
*[true] add
1101+
} a check for equality to the condition of the match arm",
1102+
applicability = "maybe-incorrect",
1103+
style = "verbose"
1104+
)]
1105+
AddToIf {
1106+
#[suggestion_part(code = "binding")]
1107+
pat_span: Span,
1108+
#[suggestion_part(code = " && binding == {name}")]
1109+
span: Span,
1110+
name: String,
1111+
ty: Ty<'tcx>,
1112+
manual_partialeq_impl: bool,
1113+
},
1114+
#[multipart_suggestion(
1115+
"{$manual_partialeq_impl ->
1116+
[false] if `{$ty}` manually implemented `PartialEq`, you could check
1117+
*[true] check
1118+
} for equality instead of pattern matching",
1119+
applicability = "maybe-incorrect",
1120+
style = "verbose"
1121+
)]
1122+
AddToLetChain {
1123+
#[suggestion_part(code = "binding")]
1124+
pat_span: Span,
1125+
#[suggestion_part(code = " && binding == {name}")]
1126+
span: Span,
1127+
name: String,
1128+
ty: Ty<'tcx>,
1129+
manual_partialeq_impl: bool,
1130+
},
1131+
#[multipart_suggestion(
1132+
"{$manual_partialeq_impl ->
1133+
[false] if `{$ty}` manually implemented `PartialEq`, you could check
1134+
*[true] check
1135+
} for equality instead of pattern matching",
1136+
applicability = "maybe-incorrect",
1137+
style = "verbose"
1138+
)]
1139+
ReplaceWithEq {
1140+
#[suggestion_part(code = "")]
1141+
removal: Span,
1142+
#[suggestion_part(code = " == ")]
1143+
eq: Span,
1144+
ty: Ty<'tcx>,
1145+
manual_partialeq_impl: bool,
1146+
},
1147+
#[multipart_suggestion(
1148+
"{$manual_partialeq_impl ->
1149+
[false] if `{$ty}` manually implemented `PartialEq`, you could check
1150+
*[true] check
1151+
} for equality instead of pattern matching",
1152+
applicability = "maybe-incorrect",
1153+
style = "verbose"
1154+
)]
1155+
ReplaceLetElseWithIf {
1156+
#[suggestion_part(code = "if ")]
1157+
if_span: Span,
1158+
#[suggestion_part(code = " == ")]
1159+
eq: Span,
1160+
#[suggestion_part(code = " ")]
1161+
else_span: Span,
1162+
ty: Ty<'tcx>,
1163+
manual_partialeq_impl: bool,
1164+
},
10671165
}
10681166

10691167
#[derive(Diagnostic)]

‎compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use tracing::{debug, instrument, trace};
2222
use super::PatCtxt;
2323
use crate::errors::{
2424
ConstPatternDependsOnGenericParameter, CouldNotEvalConstPattern, InvalidPattern, NaNPattern,
25-
PointerPattern, TypeNotPartialEq, TypeNotStructural, UnionPattern, UnsizedPattern,
25+
PointerPattern, SuggestEq, TypeNotPartialEq, TypeNotStructural, UnionPattern, UnsizedPattern,
2626
};
2727

2828
impl<'tcx, 'ptcx> PatCtxt<'tcx, 'ptcx> {
@@ -224,13 +224,93 @@ impl<'tcx> ConstToPat<'tcx> {
224224
}
225225
_ => (None, true),
226226
};
227+
let manual_partialeq_impl =
228+
manual_partialeq_impl_note || manual_partialeq_impl_span.is_some();
229+
let is_local = adt_def.did().is_local();
227230
let ty_def_span = tcx.def_span(adt_def.did());
231+
let suggestion = if let Ok(name) = tcx.sess.source_map().span_to_snippet(self.span)
232+
&& (is_local || manual_partialeq_impl)
233+
{
234+
let mut hir_id = self.id;
235+
while let hir::Node::Pat(pat) = tcx.parent_hir_node(hir_id) {
236+
hir_id = pat.hir_id;
237+
}
238+
match tcx.parent_hir_node(hir_id) {
239+
hir::Node::Arm(hir::Arm { pat, guard: None, .. }) => {
240+
// Add an if condition to the match arm.
241+
Some(SuggestEq::AddIf {
242+
if_span: pat.span.shrink_to_hi(),
243+
pat_span: self.span,
244+
name,
245+
ty,
246+
manual_partialeq_impl,
247+
})
248+
}
249+
hir::Node::Arm(hir::Arm { guard: Some(guard), .. }) => {
250+
// Modify the the match arm if condition and add a check for equality.
251+
Some(SuggestEq::AddToIf {
252+
span: guard.span.shrink_to_hi(),
253+
pat_span: self.span,
254+
name,
255+
ty,
256+
manual_partialeq_impl,
257+
})
258+
}
259+
hir::Node::Expr(hir::Expr {
260+
kind: hir::ExprKind::Let(let_expr),
261+
span,
262+
..
263+
}) => {
264+
if let_expr.pat.span == self.span {
265+
// `if let CONST = expr` -> `if CONST == expr`.
266+
Some(SuggestEq::ReplaceWithEq {
267+
removal: span.until(self.span),
268+
eq: self.span.between(let_expr.init.span),
269+
ty,
270+
manual_partialeq_impl,
271+
})
272+
} else if tcx.sess.edition().at_least_rust_2024() {
273+
// `if let Some(CONST) = expr` ->
274+
// `if let Some(binding) = expr && binding == CONST`.
275+
Some(SuggestEq::AddToLetChain {
276+
span: span.shrink_to_hi(),
277+
pat_span: self.span,
278+
name,
279+
ty,
280+
manual_partialeq_impl,
281+
})
282+
} else {
283+
None
284+
}
285+
}
286+
hir::Node::LetStmt(let_stmt)
287+
if let Some(init) = let_stmt.init
288+
&& let Some(els) = let_stmt.els
289+
&& init.span.ctxt().is_root()
290+
&& els.span.ctxt().is_root() =>
291+
{
292+
// `let PAT = expr else {` -> `if PAT == expr {`.
293+
Some(SuggestEq::ReplaceLetElseWithIf {
294+
if_span: let_stmt.span.until(let_stmt.pat.span),
295+
eq: let_stmt.pat.span.between(init.span),
296+
else_span: init.span.between(els.span),
297+
ty,
298+
manual_partialeq_impl,
299+
})
300+
}
301+
_ => None,
302+
}
303+
} else {
304+
None
305+
};
228306
let err = TypeNotStructural {
229307
span,
230308
ty,
231309
ty_def_span,
232310
manual_partialeq_impl_span,
233311
manual_partialeq_impl_note,
312+
is_local,
313+
suggestion,
234314
};
235315
return self.mk_err(tcx.dcx().create_err(err), ty);
236316
}

‎tests/ui/consts/const_in_pattern/cross-crate-fail.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn main() {
1414
}
1515

1616
match None {
17-
<Defaulted as consts::AssocConst>::SOME => panic!(),
17+
<Defaulted as consts::AssocConst>::SOME => panic!(),
1818
//~^ ERROR constant of non-structural type `CustomEq` in a pattern
1919
_ => {}
2020
}

‎tests/ui/consts/const_in_pattern/cross-crate-fail.stderr‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,38 @@ LL | consts::SOME => panic!(),
77
::: $DIR/auxiliary/consts.rs:1:1
88
|
99
LL | pub struct CustomEq;
10-
| ------------------- `CustomEq` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
10+
| ------------------- `CustomEq` is not usable in patterns
1111
...
1212
LL | pub const SOME: Option<CustomEq> = Some(CustomEq);
1313
| -------------------------------- constant defined here
1414
|
1515
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
16+
help: add a condition to the match arm checking for equality
17+
|
18+
LL - consts::SOME => panic!(),
19+
LL + binding if binding == consts::SOME => panic!(),
20+
|
1621

1722
error: constant of non-structural type `CustomEq` in a pattern
1823
--> $DIR/cross-crate-fail.rs:17:9
1924
|
20-
LL | <Defaulted as consts::AssocConst>::SOME => panic!(),
25+
LL | <Defaulted as consts::AssocConst>::SOME => panic!(),
2126
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constant of non-structural type
2227
|
2328
::: $DIR/auxiliary/consts.rs:1:1
2429
|
2530
LL | pub struct CustomEq;
26-
| ------------------- `CustomEq` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
31+
| ------------------- `CustomEq` is not usable in patterns
2732
...
2833
LL | const SOME: Option<CustomEq> = Some(CustomEq);
2934
| ---------------------------- constant defined here
3035
|
3136
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
37+
help: add a condition to the match arm checking for equality
38+
|
39+
LL - <Defaulted as consts::AssocConst>::SOME => panic!(),
40+
LL + binding if binding == <Defaulted as consts::AssocConst>::SOME => panic!(),
41+
|
3242

3343
error: aborting due to 2 previous errors
3444

‎tests/ui/consts/const_in_pattern/no-eq-branch-fail.stderr‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ LL | BAR_BAZ => panic!(),
1111
| ^^^^^^^ constant of non-structural type
1212
|
1313
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
14+
help: add a condition to the match arm checking for equality
15+
|
16+
LL - BAR_BAZ => panic!(),
17+
LL + binding if binding == BAR_BAZ => panic!(),
18+
|
1419

1520
error: aborting due to 1 previous error
1621

0 commit comments

Comments
 (0)