Skip to content

Commit

Permalink
Auto merge of #115893 - RalfJung:match-require-partial-eq, r=oli-obk
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Sep 26, 2023
2 parents 8bf0dec + a1d6fc4 commit 1f2bacf
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 11 deletions.
52 changes: 52 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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)]`
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
37 changes: 28 additions & 9 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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());
Expand All @@ -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 };
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/consts/const_in_pattern/issue-65466.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
_ => (),
}
}
23 changes: 23 additions & 0 deletions tests/ui/consts/const_in_pattern/issue-65466.stderr
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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?"),
}
}
23 changes: 23 additions & 0 deletions tests/ui/match/issue-72896-non-partial-eq-const.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 1f2bacf

Please sign in to comment.