Auto merge of #115893 - RalfJung:match-require-partial-eq, r=oli-obk

lint towards rejecting consts in patterns that do not implement PartialEq

I think we definitely don't want to allow such consts, so even while the general plan around structural matching is up in the air, we can start the process of getting non-PartialEq matches out of the ecosystem.
This commit is contained in:
bors 2023-09-26 13:38:28 +00:00
commit 1f2bacf677
8 changed files with 139 additions and 11 deletions

View File

@ -2311,6 +2311,57 @@ declare_lint! {
};
}
declare_lint! {
/// The `const_patterns_without_partial_eq` lint detects constants that are used in patterns,
/// whose type does not implement `PartialEq`.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(const_patterns_without_partial_eq)]
///
/// trait EnumSetType {
/// type Repr;
/// }
///
/// enum Enum8 { }
/// impl EnumSetType for Enum8 {
/// type Repr = u8;
/// }
///
/// #[derive(PartialEq, Eq)]
/// struct EnumSet<T: EnumSetType> {
/// __enumset_underlying: T::Repr,
/// }
///
/// const CONST_SET: EnumSet<Enum8> = EnumSet { __enumset_underlying: 3 };
///
/// fn main() {
/// match CONST_SET {
/// CONST_SET => { /* ok */ }
/// _ => panic!("match fell through?"),
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Previous versions of Rust accepted constants in patterns, even if those constants' types
/// did not have `PartialEq` implemented. The compiler falls back to comparing the value
/// field-by-field. In the future we'd like to ensure that pattern matching always
/// follows `PartialEq` semantics, so that trait bound will become a requirement for
/// matching on constants.
pub CONST_PATTERNS_WITHOUT_PARTIAL_EQ,
Warn,
"constant in pattern does not implement `PartialEq`",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #116122 <https://github.com/rust-lang/rust/issues/116122>",
};
}
declare_lint! {
/// The `ambiguous_associated_items` lint detects ambiguity between
/// [associated items] and [enum variants].
@ -3357,6 +3408,7 @@ declare_lint_pass! {
CONFLICTING_REPR_HINTS,
CONST_EVALUATABLE_UNCHECKED,
CONST_ITEM_MUTATION,
CONST_PATTERNS_WITHOUT_PARTIAL_EQ,
DEAD_CODE,
DEPRECATED,
DEPRECATED_CFG_ATTR_CRATE_TYPE_NAME,

View File

@ -229,6 +229,9 @@ mir_build_non_exhaustive_patterns_type_not_empty = non-exhaustive patterns: type
.suggestion = ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
.help = ensure that all possible cases are being handled by adding a match arm with a wildcard pattern
mir_build_non_partial_eq_match =
to use a constant of type `{$non_peq_ty}` in a pattern, the type must implement `PartialEq`
mir_build_nontrivial_structural_match =
to use a constant of type `{$non_sm_ty}` in a pattern, the constant's initializer must be trivial or `{$non_sm_ty}` must be annotated with `#[derive(PartialEq, Eq)]`

View File

@ -748,6 +748,12 @@ pub struct NontrivialStructuralMatch<'tcx> {
pub non_sm_ty: Ty<'tcx>,
}
#[derive(LintDiagnostic)]
#[diag(mir_build_non_partial_eq_match)]
pub struct NonPartialEqMatch<'tcx> {
pub non_peq_ty: Ty<'tcx>,
}
#[derive(LintDiagnostic)]
#[diag(mir_build_overlapping_range_endpoints)]
#[note]

View File

@ -16,8 +16,8 @@ use std::cell::Cell;
use super::PatCtxt;
use crate::errors::{
FloatPattern, IndirectStructuralMatch, InvalidPattern, NontrivialStructuralMatch,
PointerPattern, TypeNotStructural, UnionPattern, UnsizedPattern,
FloatPattern, IndirectStructuralMatch, InvalidPattern, NonPartialEqMatch,
NontrivialStructuralMatch, PointerPattern, TypeNotStructural, UnionPattern, UnsizedPattern,
};
impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
@ -155,8 +155,9 @@ impl<'tcx> ConstToPat<'tcx> {
};
if !self.saw_const_match_error.get() {
// If we were able to successfully convert the const to some pat,
// double-check that all types in the const implement `Structural`.
// If we were able to successfully convert the const to some pat (possibly with some
// lints, but no errors), double-check that all types in the const implement
// `Structural` and `PartialEq`.
let structural =
traits::search_for_structural_match_violation(self.span, self.tcx(), cv.ty());
@ -178,7 +179,7 @@ impl<'tcx> ConstToPat<'tcx> {
}
if let Some(non_sm_ty) = structural {
if !self.type_may_have_partial_eq_impl(cv.ty()) {
if !self.type_has_partial_eq_impl(cv.ty()) {
if let ty::Adt(def, ..) = non_sm_ty.kind() {
if def.is_union() {
let err = UnionPattern { span: self.span };
@ -192,8 +193,10 @@ impl<'tcx> ConstToPat<'tcx> {
} else {
let err = InvalidPattern { span: self.span, non_sm_ty };
self.tcx().sess.emit_err(err);
return Box::new(Pat { span: self.span, ty: cv.ty(), kind: PatKind::Wild });
}
// All branches above emitted an error. Don't print any more lints.
// The pattern we return is irrelevant since we errored.
return Box::new(Pat { span: self.span, ty: cv.ty(), kind: PatKind::Wild });
} else if !self.saw_const_match_lint.get() {
if let Some(mir_structural_match_violation) = mir_structural_match_violation {
match non_sm_ty.kind() {
@ -238,13 +241,24 @@ impl<'tcx> ConstToPat<'tcx> {
_ => {}
}
}
// Always check for `PartialEq`, even if we emitted other lints. (But not if there were
// any errors.) This ensures it shows up in cargo's future-compat reports as well.
if !self.type_has_partial_eq_impl(cv.ty()) {
self.tcx().emit_spanned_lint(
lint::builtin::CONST_PATTERNS_WITHOUT_PARTIAL_EQ,
self.id,
self.span,
NonPartialEqMatch { non_peq_ty: cv.ty() },
);
}
}
inlined_const_as_pat
}
#[instrument(level = "trace", skip(self), ret)]
fn type_may_have_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool {
fn type_has_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool {
// double-check there even *is* a semantic `PartialEq` to dispatch to.
//
// (If there isn't, then we can safely issue a hard
@ -259,8 +273,13 @@ impl<'tcx> ConstToPat<'tcx> {
ty::TraitRef::new(self.tcx(), partial_eq_trait_id, [ty, ty]),
);
// FIXME: should this call a `predicate_must_hold` variant instead?
self.infcx.predicate_may_hold(&partial_eq_obligation)
// This *could* accept a type that isn't actually `PartialEq`, because region bounds get
// ignored. However that should be pretty much impossible since consts that do not depend on
// generics can only mention the `'static` lifetime, and how would one have a type that's
// `PartialEq` for some lifetime but *not* for `'static`? If this ever becomes a problem
// we'll need to leave some sort of trace of this requirement in the MIR so that borrowck
// can ensure that the type really implements `PartialEq`.
self.infcx.predicate_must_hold_modulo_regions(&partial_eq_obligation)
}
fn field_pats(

View File

@ -15,7 +15,8 @@ const C: &[O<B>] = &[O::None];
fn main() {
let x = O::None;
match &[x][..] {
C => (),
C => (), //~WARN: the type must implement `PartialEq`
//~| previously accepted
_ => (),
}
}

View File

@ -0,0 +1,23 @@
warning: to use a constant of type `&[O<B>]` in a pattern, the type must implement `PartialEq`
--> $DIR/issue-65466.rs:18:9
|
LL | C => (),
| ^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default
warning: 1 warning emitted
Future incompatibility report: Future breakage diagnostic:
warning: to use a constant of type `&[O<B>]` in a pattern, the type must implement `PartialEq`
--> $DIR/issue-65466.rs:18:9
|
LL | C => (),
| ^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default

View File

@ -17,7 +17,8 @@ const CONST_SET: EnumSet<Enum8> = EnumSet { __enumset_underlying: 3 };
fn main() {
match CONST_SET {
CONST_SET => { /* ok */ }
CONST_SET => { /* ok */ } //~WARN: must implement `PartialEq`
//~| previously accepted
_ => panic!("match fell through?"),
}
}

View File

@ -0,0 +1,23 @@
warning: to use a constant of type `EnumSet<Enum8>` in a pattern, the type must implement `PartialEq`
--> $DIR/issue-72896-non-partial-eq-const.rs:20:9
|
LL | CONST_SET => { /* ok */ }
| ^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default
warning: 1 warning emitted
Future incompatibility report: Future breakage diagnostic:
warning: to use a constant of type `EnumSet<Enum8>` in a pattern, the type must implement `PartialEq`
--> $DIR/issue-72896-non-partial-eq-const.rs:20:9
|
LL | CONST_SET => { /* ok */ }
| ^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default