Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 100 additions & 2 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,17 +1053,115 @@ pub(crate) struct TypeNotStructural<'tcx> {
#[primary_span]
#[label("constant of non-structural type")]
pub(crate) span: Span,
#[label("`{$ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns")]
#[label(
"{$is_local ->
*[true] `{$ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
[false] `{$ty}` is not usable in patterns
}"
)]
pub(crate) ty_def_span: Span,
pub(crate) ty: Ty<'tcx>,
#[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"
"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"
)]
pub(crate) manual_partialeq_impl_span: Option<Span>,
#[note(
"see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details"
)]
pub(crate) manual_partialeq_impl_note: bool,
#[subdiagnostic]
pub(crate) suggestion: Option<SuggestEq<'tcx>>,
pub(crate) is_local: bool,
}

#[derive(Subdiagnostic)]
pub(crate) enum SuggestEq<'tcx> {
#[multipart_suggestion(
"{$manual_partialeq_impl ->
[false] if `{$ty}` manually implemented `PartialEq`, you could add
*[true] add
} a condition to the match arm checking for equality",
applicability = "maybe-incorrect",
style = "verbose"
)]
AddIf {
#[suggestion_part(code = "binding")]
pat_span: Span,
#[suggestion_part(code = " if binding == {name}")]
if_span: Span,
name: String,
ty: Ty<'tcx>,
manual_partialeq_impl: bool,
},
#[multipart_suggestion(
"{$manual_partialeq_impl ->
[false] if `{$ty}` manually implemented `PartialEq`, you could add
*[true] add
} a check for equality to the condition of the match arm",
applicability = "maybe-incorrect",
style = "verbose"
)]
AddToIf {
#[suggestion_part(code = "binding")]
pat_span: Span,
#[suggestion_part(code = " && binding == {name}")]
span: Span,
name: String,
ty: Ty<'tcx>,
manual_partialeq_impl: bool,
},
#[multipart_suggestion(
"{$manual_partialeq_impl ->
[false] if `{$ty}` manually implemented `PartialEq`, you could check
*[true] check
} for equality instead of pattern matching",
applicability = "maybe-incorrect",
style = "verbose"
)]
AddToLetChain {
#[suggestion_part(code = "binding")]
pat_span: Span,
#[suggestion_part(code = " && binding == {name}")]
span: Span,
name: String,
ty: Ty<'tcx>,
manual_partialeq_impl: bool,
},
#[multipart_suggestion(
"{$manual_partialeq_impl ->
[false] if `{$ty}` manually implemented `PartialEq`, you could check
*[true] check
} for equality instead of pattern matching",
applicability = "maybe-incorrect",
style = "verbose"
)]
ReplaceWithEq {
#[suggestion_part(code = "")]
removal: Span,
#[suggestion_part(code = " == ")]
eq: Span,
ty: Ty<'tcx>,
manual_partialeq_impl: bool,
},
#[multipart_suggestion(
"{$manual_partialeq_impl ->
[false] if `{$ty}` manually implemented `PartialEq`, you could check
*[true] check
} for equality instead of pattern matching",
applicability = "maybe-incorrect",
style = "verbose"
)]
ReplaceLetElseWithIf {
#[suggestion_part(code = "if ")]
if_span: Span,
#[suggestion_part(code = " == ")]
eq: Span,
#[suggestion_part(code = " ")]
else_span: Span,
ty: Ty<'tcx>,
manual_partialeq_impl: bool,
},
}

#[derive(Diagnostic)]
Expand Down
82 changes: 81 additions & 1 deletion compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use tracing::{debug, instrument, trace};
use super::PatCtxt;
use crate::errors::{
ConstPatternDependsOnGenericParameter, CouldNotEvalConstPattern, InvalidPattern, NaNPattern,
PointerPattern, TypeNotPartialEq, TypeNotStructural, UnionPattern, UnsizedPattern,
PointerPattern, SuggestEq, TypeNotPartialEq, TypeNotStructural, UnionPattern, UnsizedPattern,
};

impl<'tcx> PatCtxt<'tcx> {
Expand Down Expand Up @@ -224,13 +224,93 @@ impl<'tcx> ConstToPat<'tcx> {
}
_ => (None, true),
};
let manual_partialeq_impl =
manual_partialeq_impl_note || manual_partialeq_impl_span.is_some();
let is_local = adt_def.did().is_local();
let ty_def_span = tcx.def_span(adt_def.did());
let suggestion = if let Ok(name) = tcx.sess.source_map().span_to_snippet(self.span)
&& (is_local || manual_partialeq_impl)
{
let mut hir_id = self.id;
while let hir::Node::Pat(pat) = tcx.parent_hir_node(hir_id) {
hir_id = pat.hir_id;
}
match tcx.parent_hir_node(hir_id) {
hir::Node::Arm(hir::Arm { pat, guard: None, .. }) => {
// Add an if condition to the match arm.
Some(SuggestEq::AddIf {
if_span: pat.span.shrink_to_hi(),
pat_span: self.span,
name,
ty,
manual_partialeq_impl,
})
}
hir::Node::Arm(hir::Arm { guard: Some(guard), .. }) => {
// Modify the the match arm if condition and add a check for equality.
Some(SuggestEq::AddToIf {
span: guard.span.shrink_to_hi(),
pat_span: self.span,
name,
ty,
manual_partialeq_impl,
})
}
hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Let(let_expr),
span,
..
}) => {
if let_expr.pat.span == self.span {
// `if let CONST = expr` -> `if CONST == expr`.
Some(SuggestEq::ReplaceWithEq {
removal: span.until(self.span),
eq: self.span.between(let_expr.init.span),
ty,
manual_partialeq_impl,
})
} else if tcx.sess.edition().at_least_rust_2024() {
// `if let Some(CONST) = expr` ->
// `if let Some(binding) = expr && binding == CONST`.
Some(SuggestEq::AddToLetChain {
span: span.shrink_to_hi(),
pat_span: self.span,
name,
ty,
manual_partialeq_impl,
})
} else {
None
}
}
hir::Node::LetStmt(let_stmt)
if let Some(init) = let_stmt.init
&& let Some(els) = let_stmt.els
&& init.span.ctxt().is_root()
&& els.span.ctxt().is_root() =>
{
// `let PAT = expr else {` -> `if PAT == expr {`.
Some(SuggestEq::ReplaceLetElseWithIf {
if_span: let_stmt.span.until(let_stmt.pat.span),
eq: let_stmt.pat.span.between(init.span),
else_span: init.span.between(els.span),
ty,
manual_partialeq_impl,
})
}
_ => None,
}
} else {
None
};
let err = TypeNotStructural {
span,
ty,
ty_def_span,
manual_partialeq_impl_span,
manual_partialeq_impl_note,
is_local,
suggestion,
};
return self.mk_err(tcx.dcx().create_err(err), ty);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const_in_pattern/cross-crate-fail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn main() {
}

match None {
<Defaulted as consts::AssocConst>::SOME => panic!(),
<Defaulted as consts::AssocConst>::SOME => panic!(),
//~^ ERROR constant of non-structural type `CustomEq` in a pattern
_ => {}
}
Expand Down
16 changes: 13 additions & 3 deletions tests/ui/consts/const_in_pattern/cross-crate-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,38 @@ LL | consts::SOME => panic!(),
::: $DIR/auxiliary/consts.rs:1:1
|
LL | pub struct CustomEq;
| ------------------- `CustomEq` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
| ------------------- `CustomEq` is not usable in patterns
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This non-local case is tricky, because the other crate might be under the author's control and able to be changed. I guess it's impossible to know for sure.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going back and forth on this one. I settled on "we're giving them a link to the docs". I was concerned with giving the impression to the user that they had to somehow fork their dep. We could instead say something like "CustomEq could be used in patterns if it was annotated with #[derive(PartialEq)]", but I fear that has the same problem.

...
LL | pub const SOME: Option<CustomEq> = Some(CustomEq);
| -------------------------------- constant defined here
|
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
help: add a condition to the match arm checking for equality
|
LL - consts::SOME => panic!(),
LL + binding if binding == consts::SOME => panic!(),
|

error: constant of non-structural type `CustomEq` in a pattern
--> $DIR/cross-crate-fail.rs:17:9
|
LL | <Defaulted as consts::AssocConst>::SOME => panic!(),
LL | <Defaulted as consts::AssocConst>::SOME => panic!(),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constant of non-structural type
|
::: $DIR/auxiliary/consts.rs:1:1
|
LL | pub struct CustomEq;
| ------------------- `CustomEq` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
| ------------------- `CustomEq` is not usable in patterns
...
LL | const SOME: Option<CustomEq> = Some(CustomEq);
| ---------------------------- constant defined here
|
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
help: add a condition to the match arm checking for equality
|
LL - <Defaulted as consts::AssocConst>::SOME => panic!(),
LL + binding if binding == <Defaulted as consts::AssocConst>::SOME => panic!(),
|

error: aborting due to 2 previous errors

5 changes: 5 additions & 0 deletions tests/ui/consts/const_in_pattern/no-eq-branch-fail.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ LL | BAR_BAZ => panic!(),
| ^^^^^^^ constant of non-structural type
|
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
help: add a condition to the match arm checking for equality
|
LL - BAR_BAZ => panic!(),
LL + binding if binding == BAR_BAZ => panic!(),
Copy link
Copy Markdown
Contributor

@nnethercote nnethercote Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit confusing that the error message suggests two separate ways of fixing this -- add derive(PartialEq) and use a match arm condition. Could the one-or-the-other nature be made clearer? Maybe by inserting "alternatively, " at the start of the help message?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was looking on how to address this. I'll follow up.

|

error: aborting due to 1 previous error

Loading
Loading