Skip to content

Turn remaining non-structural-const-in-pattern lints into hard errors #124661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 26, 2024
Merged

Turn remaining non-structural-const-in-pattern lints into hard errors #124661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
179a6a0
remove IndirectStructuralMatch lint, emit the usual hard error instead
RalfJung May 3, 2024
cbd682b
turn pointer_structural_match into a hard error
RalfJung May 3, 2024
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
10 changes: 10 additions & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,16 @@ fn register_builtins(store: &mut LintStore) {
"converted into hard error, see RFC #3535 \
<https://rust-lang.github.io/rfcs/3535-constants-in-patterns.html> for more information",
);
store.register_removed(
"indirect_structural_match",
"converted into hard error, see RFC #3535 \
<https://rust-lang.github.io/rfcs/3535-constants-in-patterns.html> for more information",
);
store.register_removed(
"pointer_structural_match",
"converted into hard error, see RFC #3535 \
<https://rust-lang.github.io/rfcs/3535-constants-in-patterns.html> for more information",
);
}

fn register_internals(store: &mut LintStore) {
Expand Down
87 changes: 0 additions & 87 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ declare_lint_pass! {
HIDDEN_GLOB_REEXPORTS,
ILL_FORMED_ATTRIBUTE_INPUT,
INCOMPLETE_INCLUDE,
INDIRECT_STRUCTURAL_MATCH,
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
INLINE_NO_SANITIZE,
INVALID_DOC_ATTRIBUTES,
Expand All @@ -75,7 +74,6 @@ declare_lint_pass! {
ORDER_DEPENDENT_TRAIT_OBJECTS,
OVERLAPPING_RANGE_ENDPOINTS,
PATTERNS_IN_FNS_WITHOUT_BODY,
POINTER_STRUCTURAL_MATCH,
PRIVATE_BOUNDS,
PRIVATE_INTERFACES,
PROC_MACRO_BACK_COMPAT,
Expand Down Expand Up @@ -2355,52 +2353,6 @@ declare_lint! {
"outlives requirements can be inferred"
}

declare_lint! {
/// The `indirect_structural_match` lint detects a `const` in a pattern
/// that manually implements [`PartialEq`] and [`Eq`].
///
/// [`PartialEq`]: https://doc.rust-lang.org/std/cmp/trait.PartialEq.html
/// [`Eq`]: https://doc.rust-lang.org/std/cmp/trait.Eq.html
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(indirect_structural_match)]
///
/// struct NoDerive(i32);
/// impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } }
/// impl Eq for NoDerive { }
/// #[derive(PartialEq, Eq)]
/// struct WrapParam<T>(T);
/// const WRAP_INDIRECT_PARAM: & &WrapParam<NoDerive> = & &WrapParam(NoDerive(0));
/// fn main() {
/// match WRAP_INDIRECT_PARAM {
/// WRAP_INDIRECT_PARAM => { }
/// _ => { }
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// The compiler unintentionally accepted this form in the past. This is a
/// [future-incompatible] lint to transition this to a hard error in the
/// future. See [issue #62411] for a complete description of the problem,
/// and some possible solutions.
///
/// [issue #62411]: https://github.com/rust-lang/rust/issues/62411
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub INDIRECT_STRUCTURAL_MATCH,
Warn,
"constant used in pattern contains value of non-structural-match type in a field or a variant",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #120362 <https://github.com/rust-lang/rust/issues/120362>",
};
}

declare_lint! {
/// The `deprecated_in_future` lint is internal to rustc and should not be
/// used by user code.
Expand All @@ -2418,45 +2370,6 @@ declare_lint! {
report_in_external_macro
}

declare_lint! {
/// The `pointer_structural_match` lint detects pointers used in patterns whose behaviour
/// cannot be relied upon across compiler versions and optimization levels.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(pointer_structural_match)]
/// fn foo(a: usize, b: usize) -> usize { a + b }
/// const FOO: fn(usize, usize) -> usize = foo;
/// fn main() {
/// match FOO {
/// FOO => {},
/// _ => {},
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Previous versions of Rust allowed function pointers and all raw pointers in patterns.
/// While these work in many cases as expected by users, it is possible that due to
/// optimizations pointers are "not equal to themselves" or pointers to different functions
/// compare as equal during runtime. This is because LLVM optimizations can deduplicate
/// functions if their bodies are the same, thus also making pointers to these functions point
/// to the same location. Additionally functions may get duplicated if they are instantiated
/// in different crates and not deduplicated again via LTO. Pointer identity for memory
/// created by `const` is similarly unreliable.
pub POINTER_STRUCTURAL_MATCH,
Warn,
"pointers are not structural-match",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #120362 <https://github.com/rust-lang/rust/issues/120362>",
};
}

declare_lint! {
/// The `ambiguous_associated_items` lint detects ambiguity between
/// [associated items] and [enum variants].
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_mir_build/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ mir_build_extern_static_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
.note = extern statics are not controlled by the Rust type system: invalid data, aliasing violations or data races will cause undefined behavior
.label = use of extern static

mir_build_indirect_structural_match =
to use a constant of type `{$non_sm_ty}` in a pattern, `{$non_sm_ty}` must be annotated with `#[derive(PartialEq)]`

mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant

mir_build_initializing_type_with_requires_unsafe =
Expand Down Expand Up @@ -257,9 +254,6 @@ mir_build_non_exhaustive_patterns_type_not_empty = non-exhaustive patterns: type
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)]`

mir_build_pattern_not_covered = refutable pattern in {$origin}
.pattern_ty = the matched value is of type `{$pattern_ty}`

Expand Down
23 changes: 5 additions & 18 deletions compiler/rustc_mir_build/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,9 +788,12 @@ pub struct NaNPattern {
pub span: Span,
}

#[derive(LintDiagnostic)]
#[derive(Diagnostic)]
#[diag(mir_build_pointer_pattern)]
pub struct PointerPattern;
pub struct PointerPattern {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(mir_build_non_empty_never_pattern)]
Expand All @@ -802,22 +805,6 @@ pub struct NonEmptyNeverPattern<'tcx> {
pub ty: Ty<'tcx>,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_indirect_structural_match)]
#[note(mir_build_type_not_structural_tip)]
#[note(mir_build_type_not_structural_more_info)]
pub struct IndirectStructuralMatch<'tcx> {
pub non_sm_ty: Ty<'tcx>,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_nontrivial_structural_match)]
#[note(mir_build_type_not_structural_tip)]
#[note(mir_build_type_not_structural_more_info)]
pub struct NontrivialStructuralMatch<'tcx> {
pub non_sm_ty: Ty<'tcx>,
}

#[derive(Diagnostic)]
#[diag(mir_build_exceeds_mcdc_condition_num_limit)]
pub(crate) struct MCDCExceedsConditionNumLimit {
Expand Down
86 changes: 6 additions & 80 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use rustc_infer::traits::Obligation;
use rustc_middle::mir;
use rustc_middle::thir::{FieldPat, Pat, PatKind};
use rustc_middle::ty::{self, Ty, TyCtxt, ValTree};
use rustc_session::lint;
use rustc_span::{ErrorGuaranteed, Span};
use rustc_target::abi::{FieldIdx, VariantIdx};
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
Expand All @@ -16,8 +15,8 @@ use std::cell::Cell;

use super::PatCtxt;
use crate::errors::{
IndirectStructuralMatch, InvalidPattern, NaNPattern, PointerPattern, TypeNotPartialEq,
TypeNotStructural, UnionPattern, UnsizedPattern,
InvalidPattern, NaNPattern, PointerPattern, TypeNotPartialEq, TypeNotStructural, UnionPattern,
UnsizedPattern,
};

impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -49,15 +48,6 @@ struct ConstToPat<'tcx> {
// value.
saw_const_match_error: Cell<Option<ErrorGuaranteed>>,

// This tracks if we emitted some diagnostic for a given const value, so that
// we will not subsequently issue an irrelevant lint for the same const
// value.
saw_const_match_lint: Cell<bool>,

// For backcompat we need to keep allowing non-structurally-eq types behind references.
// See also all the `cant-hide-behind` tests.
behind_reference: Cell<bool>,

// inference context used for checking `T: Structural` bounds.
infcx: InferCtxt<'tcx>,

Expand All @@ -84,8 +74,6 @@ impl<'tcx> ConstToPat<'tcx> {
infcx,
param_env: pat_ctxt.param_env,
saw_const_match_error: Cell::new(None),
saw_const_match_lint: Cell::new(false),
behind_reference: Cell::new(false),
treat_byte_string_as_slice: pat_ctxt
.typeck_results
.treat_byte_string_as_slice
Expand Down Expand Up @@ -197,15 +185,12 @@ impl<'tcx> ConstToPat<'tcx> {
// complained about structural match violations there, so we don't
// have to check anything any more.
}
} else if !have_valtree && !self.saw_const_match_lint.get() {
} else if !have_valtree {
// The only way valtree construction can fail without the structural match
// checker finding a violation is if there is a pointer somewhere.
self.tcx().emit_node_span_lint(
lint::builtin::POINTER_STRUCTURAL_MATCH,
self.id,
self.span,
PointerPattern,
);
let e = self.tcx().dcx().emit_err(PointerPattern { span: self.span });
let kind = PatKind::Error(e);
return Box::new(Pat { span: self.span, ty: cv.ty(), kind });
}

// Always check for `PartialEq` if we had no other errors yet.
Expand Down Expand Up @@ -274,36 +259,11 @@ impl<'tcx> ConstToPat<'tcx> {
cv: ValTree<'tcx>,
ty: Ty<'tcx>,
) -> Result<Box<Pat<'tcx>>, FallbackToOpaqueConst> {
let id = self.id;
let span = self.span;
let tcx = self.tcx();
let param_env = self.param_env;

let kind = match ty.kind() {
// If the type is not structurally comparable, just emit the constant directly,
// causing the pattern match code to treat it opaquely.
// FIXME: This code doesn't emit errors itself, the caller emits the errors.
// So instead of specific errors, you just get blanket errors about the whole
// const type. See
// https://github.com/rust-lang/rust/pull/70743#discussion_r404701963 for
// details.
// Backwards compatibility hack because we can't cause hard errors on these
// types, so we compare them via `PartialEq::eq` at runtime.
ty::Adt(..) if !self.type_marked_structural(ty) && self.behind_reference.get() => {
if self.saw_const_match_error.get().is_none() && !self.saw_const_match_lint.get() {
self.saw_const_match_lint.set(true);
tcx.emit_node_span_lint(
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
id,
span,
IndirectStructuralMatch { non_sm_ty: ty },
);
}
// Since we are behind a reference, we can just bubble the error up so we get a
// constant at reference type, making it easy to let the fallback call
// `PartialEq::eq` on it.
return Err(FallbackToOpaqueConst);
}
ty::FnDef(..) => {
let e = tcx.dcx().emit_err(InvalidPattern { span, non_sm_ty: ty });
self.saw_const_match_error.set(Some(e));
Expand Down Expand Up @@ -377,38 +337,6 @@ impl<'tcx> ConstToPat<'tcx> {
ty::Str => {
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_value(tcx, cv, ty)) }
}
// Backwards compatibility hack: support references to non-structural types,
// but hard error if we aren't behind a double reference. We could just use
// the fallback code path below, but that would allow *more* of this fishy
// code to compile, as then it only goes through the future incompat lint
// instead of a hard error.
ty::Adt(_, _) if !self.type_marked_structural(*pointee_ty) => {
if self.behind_reference.get() {
if self.saw_const_match_error.get().is_none()
&& !self.saw_const_match_lint.get()
{
self.saw_const_match_lint.set(true);
tcx.emit_node_span_lint(
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
self.id,
span,
IndirectStructuralMatch { non_sm_ty: *pointee_ty },
);
}
return Err(FallbackToOpaqueConst);
} else {
if let Some(e) = self.saw_const_match_error.get() {
// We already errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Error(e)
} else {
let err = TypeNotStructural { span, non_sm_ty: *pointee_ty };
let e = tcx.dcx().emit_err(err);
self.saw_const_match_error.set(Some(e));
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Error(e)
}
}
}
// All other references are converted into deref patterns and then recursively
// convert the dereferenced constant to a pattern that is the sub-pattern of the
// deref pattern.
Expand All @@ -419,7 +347,6 @@ impl<'tcx> ConstToPat<'tcx> {
// We errored. Signal that in the pattern, so that follow up errors can be silenced.
PatKind::Error(e)
} else {
let old = self.behind_reference.replace(true);
// `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when
// matching against references, you can only use byte string literals.
// The typechecker has a special case for byte string literals, by treating them
Expand All @@ -434,7 +361,6 @@ impl<'tcx> ConstToPat<'tcx> {
};
// References have the same valtree representation as their pointee.
let subpattern = self.recur(cv, pointee_ty)?;
self.behind_reference.set(old);
PatKind::Deref { subpattern }
}
}
Expand Down
6 changes: 2 additions & 4 deletions tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
//@ edition:2021

const PATTERN_REF: &str = "Hello World";
const NUMBER: i32 = 30;
const NUMBER_POINTER: *const i32 = &NUMBER;
const NUMBER_POINTER: *const i32 = 30 as *const i32;

pub fn edge_case_ref(event: &str) {
let _ = || {
Expand All @@ -26,8 +25,7 @@ pub fn edge_case_str(event: String) {
pub fn edge_case_raw_ptr(event: *const i32) {
let _ = || {
match event {
NUMBER_POINTER => (), //~WARN behave unpredictably
//~| previously accepted
NUMBER_POINTER => (),
_ => (),
};
};
Expand Down
23 changes: 0 additions & 23 deletions tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.stderr
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion tests/ui/consts/const_in_pattern/accept_structural.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//@ run-pass

#![allow(non_local_definitions)]
#![warn(indirect_structural_match)]

// This test is checking our logic for structural match checking by enumerating
// the different kinds of const expressions. This test is collecting cases where
Expand Down
Loading
Loading