Skip to content

Commit d136b31

Browse files
estebankestebank
committed
Add more context to fall-through "const pattern of non-structural type" error
Point at types that need to be marked with `#[derive(PartialEq)]`. We use a visitor to look at a type that isn't structural, looking for all ADTs that don't derive `PartialEq`. These can either be manual `impl PartialEq`s or no `impl` at all, so we differentiate between those two cases to provide more context to the user. We also only point at types and impls from the local crate, otherwise show only a note. ``` error: constant of non-structural type `&[B]` in a pattern --> $DIR/issue-61188-match-slice-forbidden-without-eq.rs:15:9 | LL | struct B(i32); | -------- must be annotated with `#[derive(PartialEq)]` to be usable in patterns LL | LL | const A: &[B] = &[]; | ------------- constant defined here ... LL | A => (), | ^ constant of non-structural type | = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details ```
1 parent 27a1880 commit d136b31

File tree

6 files changed

+169
-46
lines changed

6 files changed

+169
-46
lines changed

‎compiler/rustc_mir_build/src/errors.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -897,7 +897,6 @@ pub(crate) struct TypeNotStructural<'tcx> {
897897

898898
#[derive(Diagnostic)]
899899
#[diag(mir_build_non_partial_eq_match)]
900-
#[note(mir_build_type_not_structural_def)]
901900
#[note(mir_build_type_not_structural_more_info)]
902901
pub(crate) struct TypeNotPartialEq<'tcx> {
903902
#[primary_span]

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

Lines changed: 157 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
use rustc_abi::{FieldIdx, VariantIdx};
22
use rustc_apfloat::Float;
3+
use rustc_data_structures::fx::FxHashSet;
34
use rustc_errors::{Diag, PResult};
45
use rustc_hir as hir;
56
use rustc_index::Idx;
67
use rustc_infer::infer::TyCtxtInferExt;
78
use rustc_infer::traits::Obligation;
89
use rustc_middle::mir::interpret::ErrorHandled;
910
use rustc_middle::thir::{FieldPat, Pat, PatKind};
10-
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt, ValTree};
11+
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt, TypeVisitor, ValTree};
1112
use rustc_middle::{mir, span_bug};
12-
use rustc_span::Span;
13+
use rustc_span::def_id::DefId;
14+
use rustc_span::{Span, sym};
1315
use rustc_trait_selection::traits::ObligationCause;
1416
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
1517
use tracing::{debug, instrument, trace};
@@ -189,42 +191,16 @@ impl<'tcx> ConstToPat<'tcx> {
189191

190192
if !inlined_const_as_pat.references_error() {
191193
// Always check for `PartialEq` if we had no other errors yet.
192-
if !self.type_has_partial_eq_impl(ty) {
193-
let err = TypeNotPartialEq { span: self.span, ty };
194-
// FIXME: visit every type in `ty` and if it doesn't derive `PartialEq`, mention it.
195-
return self.mk_err(self.tcx.dcx().create_err(err), ty);
194+
if !type_has_partial_eq_impl(self.tcx, typing_env, ty).0 {
195+
let mut err = self.tcx.dcx().create_err(TypeNotPartialEq { span: self.span, ty });
196+
extend_type_not_partial_eq(self.tcx, typing_env, ty, &mut err);
197+
return self.mk_err(err, ty);
196198
}
197199
}
198200

199201
inlined_const_as_pat
200202
}
201203

202-
#[instrument(level = "trace", skip(self), ret)]
203-
fn type_has_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool {
204-
let (infcx, param_env) = self.tcx.infer_ctxt().build_with_typing_env(self.typing_env);
205-
// double-check there even *is* a semantic `PartialEq` to dispatch to.
206-
//
207-
// (If there isn't, then we can safely issue a hard
208-
// error, because that's never worked, due to compiler
209-
// using `PartialEq::eq` in this scenario in the past.)
210-
let partial_eq_trait_id =
211-
self.tcx.require_lang_item(hir::LangItem::PartialEq, Some(self.span));
212-
let partial_eq_obligation = Obligation::new(
213-
self.tcx,
214-
ObligationCause::dummy(),
215-
param_env,
216-
ty::TraitRef::new(self.tcx, partial_eq_trait_id, [ty, ty]),
217-
);
218-
219-
// This *could* accept a type that isn't actually `PartialEq`, because region bounds get
220-
// ignored. However that should be pretty much impossible since consts that do not depend on
221-
// generics can only mention the `'static` lifetime, and how would one have a type that's
222-
// `PartialEq` for some lifetime but *not* for `'static`? If this ever becomes a problem
223-
// we'll need to leave some sort of trace of this requirement in the MIR so that borrowck
224-
// can ensure that the type really implements `PartialEq`.
225-
infcx.predicate_must_hold_modulo_regions(&partial_eq_obligation)
226-
}
227-
228204
fn field_pats(
229205
&self,
230206
vals: impl Iterator<Item = (ValTree<'tcx>, Ty<'tcx>)>,
@@ -255,21 +231,23 @@ impl<'tcx> ConstToPat<'tcx> {
255231
// Extremely important check for all ADTs! Make sure they opted-in to be used in
256232
// patterns.
257233
debug!("adt_def {:?} has !type_marked_structural for cv.ty: {:?}", adt_def, ty);
234+
let (impls_partial_eq, derived, structural, impl_def_id) =
235+
type_has_partial_eq_impl(self.tcx, self.typing_env, ty);
236+
let (manual_partialeq_impl_span, manual_partialeq_impl_note) =
237+
match (impls_partial_eq, derived, structural, impl_def_id) {
238+
(_, _, true, _) => (None, false),
239+
(_, false, _, Some(def_id)) if def_id.is_local() => {
240+
(Some(tcx.def_span(def_id)), false)
241+
}
242+
_ => (None, true),
243+
};
258244
let ty_def_span = tcx.def_span(adt_def.did());
259-
let mut manual_partialeq_impl_span = None;
260-
let partial_eq_trait_id =
261-
tcx.require_lang_item(hir::LangItem::PartialEq, Some(self.span));
262-
tcx.for_each_relevant_impl(partial_eq_trait_id, ty, |def_id| {
263-
if def_id.is_local() {
264-
manual_partialeq_impl_span = Some(tcx.def_span(def_id));
265-
}
266-
});
267245
let err = TypeNotStructural {
268246
span,
269247
ty,
270248
ty_def_span,
271249
manual_partialeq_impl_span,
272-
manual_partialeq_impl_note: manual_partialeq_impl_span.is_none(),
250+
manual_partialeq_impl_note,
273251
};
274252
return Err(tcx.dcx().create_err(err));
275253
}
@@ -402,3 +380,141 @@ impl<'tcx> ConstToPat<'tcx> {
402380
Ok(Box::new(Pat { span, ty, kind }))
403381
}
404382
}
383+
384+
/// Given a type with type parameters, visit every ADT looking for types that need to
385+
/// `#[derive(PartialEq)]` to be a structural type.
386+
fn extend_type_not_partial_eq<'tcx>(
387+
tcx: TyCtxt<'tcx>,
388+
typing_env: ty::TypingEnv<'tcx>,
389+
ty: Ty<'tcx>,
390+
err: &mut Diag<'_>,
391+
) {
392+
/// Collect all types that need to be `StructuralPartialEq`.
393+
struct UsedParamsNeedInstantiationVisitor<'tcx> {
394+
tcx: TyCtxt<'tcx>,
395+
typing_env: ty::TypingEnv<'tcx>,
396+
/// The user has written `impl PartialEq for Ty` which means it's non-structual.
397+
adts_with_manual_partialeq: FxHashSet<Span>,
398+
/// The type has no `PartialEq` implementation, neither manual or derived.
399+
adts_without_partialeq: FxHashSet<Span>,
400+
/// The user has written `impl PartialEq for Ty` which means it's non-structual,
401+
/// but we don't have a span to point at, so we'll just add them as a `note`.
402+
manual: Vec<Ty<'tcx>>,
403+
/// The type has no `PartialEq` implementation, neither manual or derived, but
404+
/// we don't have a span to point at, so we'll just add them as a `note`.
405+
without: Vec<Ty<'tcx>>,
406+
}
407+
408+
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for UsedParamsNeedInstantiationVisitor<'tcx> {
409+
fn visit_ty(&mut self, ty: Ty<'tcx>) -> Self::Result {
410+
if let ty::Adt(def, _args) = ty.kind() {
411+
let ty_def_id = def.did();
412+
let ty_def_span = self.tcx.def_span(ty_def_id);
413+
let (impls_partial_eq, derived, structural, impl_def_id) =
414+
type_has_partial_eq_impl(self.tcx, self.typing_env, ty);
415+
match (impls_partial_eq, derived, structural, impl_def_id) {
416+
(_, _, true, _) => {}
417+
(true, false, _, Some(def_id)) if def_id.is_local() => {
418+
self.adts_with_manual_partialeq.insert(self.tcx.def_span(def_id));
419+
}
420+
(true, false, _, _) if ty_def_id.is_local() => {
421+
self.adts_with_manual_partialeq.insert(ty_def_span);
422+
}
423+
(false, _, _, _) if ty_def_id.is_local() => {
424+
self.adts_without_partialeq.insert(ty_def_span);
425+
}
426+
(true, false, _, _) => {
427+
self.manual.push(ty);
428+
}
429+
(false, _, _, _) => {
430+
self.without.push(ty);
431+
}
432+
_ => {}
433+
};
434+
}
435+
use rustc_middle::ty::TypeSuperVisitable;
436+
ty.super_visit_with(self)
437+
}
438+
}
439+
let mut v = UsedParamsNeedInstantiationVisitor {
440+
tcx,
441+
typing_env,
442+
adts_with_manual_partialeq: FxHashSet::default(),
443+
adts_without_partialeq: FxHashSet::default(),
444+
manual: vec![],
445+
without: vec![],
446+
};
447+
v.visit_ty(ty);
448+
#[allow(rustc::potential_query_instability)] // Span labels will be sorted by the rendering
449+
for span in v.adts_with_manual_partialeq {
450+
err.span_note(span, "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");
451+
}
452+
#[allow(rustc::potential_query_instability)] // Span labels will be sorted by the rendering
453+
for span in v.adts_without_partialeq {
454+
err.span_label(
455+
span,
456+
"must be annotated with `#[derive(PartialEq)]` to be usable in patterns",
457+
);
458+
}
459+
for ty in v.manual {
460+
err.note(format!(
461+
"`{ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns, manual `impl`s are not sufficient; see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details"
462+
));
463+
}
464+
for ty in v.without {
465+
err.note(format!(
466+
"`{ty}` must be annotated with `#[derive(PartialEq)]` to be usable in patterns"
467+
));
468+
}
469+
}
470+
471+
#[instrument(level = "trace", skip(tcx), ret)]
472+
fn type_has_partial_eq_impl<'tcx>(
473+
tcx: TyCtxt<'tcx>,
474+
typing_env: ty::TypingEnv<'tcx>,
475+
ty: Ty<'tcx>,
476+
) -> (
477+
/* has impl */ bool,
478+
/* is derived */ bool,
479+
/* structural partial eq */ bool,
480+
/* non-blanket impl */ Option<DefId>,
481+
) {
482+
let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
483+
// double-check there even *is* a semantic `PartialEq` to dispatch to.
484+
//
485+
// (If there isn't, then we can safely issue a hard
486+
// error, because that's never worked, due to compiler
487+
// using `PartialEq::eq` in this scenario in the past.)
488+
let partial_eq_trait_id = tcx.require_lang_item(hir::LangItem::PartialEq, None);
489+
let structural_partial_eq_trait_id = tcx.require_lang_item(hir::LangItem::StructuralPeq, None);
490+
491+
let partial_eq_obligation = Obligation::new(
492+
tcx,
493+
ObligationCause::dummy(),
494+
param_env,
495+
ty::TraitRef::new(tcx, partial_eq_trait_id, [ty, ty]),
496+
);
497+
498+
let mut automatically_derived = false;
499+
let mut structural_peq = false;
500+
let mut impl_def_id = None;
501+
for def_id in tcx.non_blanket_impls_for_ty(partial_eq_trait_id, ty) {
502+
automatically_derived = tcx.has_attr(def_id, sym::automatically_derived);
503+
impl_def_id = Some(def_id);
504+
}
505+
for _ in tcx.non_blanket_impls_for_ty(structural_partial_eq_trait_id, ty) {
506+
structural_peq = true;
507+
}
508+
// This *could* accept a type that isn't actually `PartialEq`, because region bounds get
509+
// ignored. However that should be pretty much impossible since consts that do not depend on
510+
// generics can only mention the `'static` lifetime, and how would one have a type that's
511+
// `PartialEq` for some lifetime but *not* for `'static`? If this ever becomes a problem
512+
// we'll need to leave some sort of trace of this requirement in the MIR so that borrowck
513+
// can ensure that the type really implements `PartialEq`.
514+
(
515+
infcx.predicate_must_hold_modulo_regions(&partial_eq_obligation),
516+
automatically_derived,
517+
structural_peq,
518+
impl_def_id,
519+
)
520+
}

‎tests/ui/consts/const_in_pattern/issue-65466.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
error: constant of non-structural type `&[O<B>]` in a pattern
22
--> $DIR/issue-65466.rs:14:9
33
|
4+
LL | struct B;
5+
| -------- must be annotated with `#[derive(PartialEq)]` to be usable in patterns
6+
LL |
47
LL | const C: &[O<B>] = &[O::None];
58
| ---------------- constant defined here
69
...
710
LL | C => (),
811
| ^ constant of non-structural type
912
|
10-
= note: `&[O<B>]` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
1113
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
1214

1315
error: aborting due to 1 previous error

‎tests/ui/consts/const_in_pattern/reject_non_partial_eq.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
error: constant of non-structural type `Option<NoPartialEq>` in a pattern
22
--> $DIR/reject_non_partial_eq.rs:28:9
33
|
4+
LL | struct NoPartialEq(u32);
5+
| ------------------ must be annotated with `#[derive(PartialEq)]` to be usable in patterns
6+
...
47
LL | const NO_PARTIAL_EQ_NONE: Option<NoPartialEq> = None;
58
| --------------------------------------------- constant defined here
69
...
710
LL | NO_PARTIAL_EQ_NONE => println!("NO_PARTIAL_EQ_NONE"),
811
| ^^^^^^^^^^^^^^^^^^ constant of non-structural type
912
|
10-
= note: `Option<NoPartialEq>` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
1113
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
1214

1315
error: aborting due to 1 previous error

‎tests/ui/match/issue-72896-non-partial-eq-const.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
error: constant of non-structural type `EnumSet<Enum8>` in a pattern
22
--> $DIR/issue-72896-non-partial-eq-const.rs:19:9
33
|
4+
LL | enum Enum8 { }
5+
| ---------- must be annotated with `#[derive(PartialEq)]` to be usable in patterns
6+
...
47
LL | const CONST_SET: EnumSet<Enum8> = EnumSet { __enumset_underlying: 3 };
58
| ------------------------------- constant defined here
69
...
710
LL | CONST_SET => { /* ok */ }
811
| ^^^^^^^^^ constant of non-structural type
912
|
10-
= note: `EnumSet<Enum8>` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
1113
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
1214

1315
error: aborting due to 1 previous error

‎tests/ui/rfcs/rfc-1445-restrict-constants-in-patterns/issue-61188-match-slice-forbidden-without-eq.stderr

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
error: constant of non-structural type `&[B]` in a pattern
22
--> $DIR/issue-61188-match-slice-forbidden-without-eq.rs:15:9
33
|
4+
LL | struct B(i32);
5+
| -------- must be annotated with `#[derive(PartialEq)]` to be usable in patterns
6+
LL |
47
LL | const A: &[B] = &[];
58
| ------------- constant defined here
69
...
710
LL | A => (),
811
| ^ constant of non-structural type
912
|
10-
= note: `&[B]` must be annotated with `#[derive(PartialEq)]` to be usable in patterns
1113
= note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralPartialEq.html for details
1214

1315
error: aborting due to 1 previous error

0 commit comments

Comments
 (0)