Skip to content
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.

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

update indirect_structural_match and pointer_structural_match lints to match RFC and to show up for dependencies #120423

Merged
merged 4 commits into from
Feb 7, 2024
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
29 changes: 1 addition & 28 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use std::mem;
use std::ops::{ControlFlow, Deref};

use super::ops::{self, NonConstOp, Status};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::qualifs::{self, HasMutInterior, NeedsDrop, NeedsNonConstDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{ConstCx, Qualif};
use crate::const_eval::is_unstable_const_fn;
Expand Down Expand Up @@ -149,37 +149,10 @@ impl<'mir, 'tcx> Qualifs<'mir, 'tcx> {

let return_loc = ccx.body.terminator_loc(return_block);

let custom_eq = match ccx.const_kind() {
// We don't care whether a `const fn` returns a value that is not structurally
// matchable. Functions calls are opaque and always use type-based qualification, so
// this value should never be used.
hir::ConstContext::ConstFn => true,

// If we know that all values of the return type are structurally matchable, there's no
// need to run dataflow.
// Opaque types do not participate in const generics or pattern matching, so we can safely count them out.
_ if ccx.body.return_ty().has_opaque_types()
|| !CustomEq::in_any_value_of_ty(ccx, ccx.body.return_ty()) =>
{
false
}

hir::ConstContext::Const { .. } | hir::ConstContext::Static(_) => {
let mut cursor = FlowSensitiveAnalysis::new(CustomEq, ccx)
.into_engine(ccx.tcx, ccx.body)
.iterate_to_fixpoint()
.into_results_cursor(ccx.body);

cursor.seek_after_primary_effect(return_loc);
cursor.get().contains(RETURN_PLACE)
}
};

ConstQualifs {
needs_drop: self.needs_drop(ccx, RETURN_PLACE, return_loc),
needs_non_const_drop: self.needs_non_const_drop(ccx, RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior(ccx, RETURN_PLACE, return_loc),
custom_eq,
tainted_by_errors,
}
}
Expand Down
32 changes: 1 addition & 31 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_middle::mir::*;
use rustc_middle::traits::BuiltinImplSource;
use rustc_middle::ty::{self, AdtDef, GenericArgsRef, Ty};
use rustc_trait_selection::traits::{
self, ImplSource, Obligation, ObligationCause, ObligationCtxt, SelectionContext,
ImplSource, Obligation, ObligationCause, ObligationCtxt, SelectionContext,
};

use super::ConstCx;
Expand All @@ -24,7 +24,6 @@ pub fn in_any_value_of_ty<'tcx>(
has_mut_interior: HasMutInterior::in_any_value_of_ty(cx, ty),
needs_drop: NeedsDrop::in_any_value_of_ty(cx, ty),
needs_non_const_drop: NeedsNonConstDrop::in_any_value_of_ty(cx, ty),
custom_eq: CustomEq::in_any_value_of_ty(cx, ty),
tainted_by_errors,
}
}
Expand Down Expand Up @@ -213,35 +212,6 @@ impl Qualif for NeedsNonConstDrop {
}
}

/// A constant that cannot be used as part of a pattern in a `match` expression.
pub struct CustomEq;

impl Qualif for CustomEq {
const ANALYSIS_NAME: &'static str = "flow_custom_eq";

fn in_qualifs(qualifs: &ConstQualifs) -> bool {
qualifs.custom_eq
}

fn in_any_value_of_ty<'tcx>(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
// If *any* component of a composite data type does not implement `Structural{Partial,}Eq`,
// we know that at least some values of that type are not structural-match. I say "some"
// because that component may be part of an enum variant (e.g.,
// `Option::<NonStructuralMatchTy>::Some`), in which case some values of this type may be
// structural-match (`Option::None`).
traits::search_for_structural_match_violation(cx.body.span, cx.tcx, ty).is_some()
}

fn in_adt_inherently<'tcx>(
cx: &ConstCx<'_, 'tcx>,
def: AdtDef<'tcx>,
args: GenericArgsRef<'tcx>,
) -> bool {
let ty = Ty::new_adt(cx.tcx, def, args);
!ty.is_structural_eq_shallow(cx.tcx)
}
}

// FIXME: Use `mir::visit::Visitor` for the `in_*` functions if/when it supports early return.

/// Returns `true` if this `Rvalue` contains qualif `Q`.
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,11 @@ fn register_builtins(store: &mut LintStore) {
"illegal_floating_point_literal_pattern",
"no longer a warning, float patterns behave the same as `==`",
);
store.register_removed(
"nontrivial_structural_match",
"no longer needed, 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
51 changes: 7 additions & 44 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
//! These are the built-in lints that are emitted direct in the main
//! compiler code, rather than using their own custom pass. Those
//! lints are all available in `rustc_lint::builtin`.
//!
//! When removing a lint, make sure to also add a call to `register_removed` in
//! compiler/rustc_lint/src/lib.rs.

use crate::{declare_lint, declare_lint_pass, FutureIncompatibilityReason};
use rustc_span::edition::Edition;
Expand Down Expand Up @@ -66,7 +69,6 @@ declare_lint_pass! {
MUST_NOT_SUSPEND,
NAMED_ARGUMENTS_USED_POSITIONALLY,
NON_EXHAUSTIVE_OMITTED_PATTERNS,
NONTRIVIAL_STRUCTURAL_MATCH,
ORDER_DEPENDENT_TRAIT_OBJECTS,
OVERLAPPING_RANGE_ENDPOINTS,
PATTERNS_IN_FNS_WITHOUT_BODY,
Expand Down Expand Up @@ -2280,8 +2282,8 @@ declare_lint! {
Warn,
"constant used in pattern contains value of non-structural-match type in a field or a variant",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #62411 <https:/rust-lang/rust/issues/62411>",
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #120362 <https:/rust-lang/rust/issues/120362>",
};
}

Expand Down Expand Up @@ -2336,47 +2338,8 @@ declare_lint! {
Warn,
"pointers are not structural-match",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #62411 <https:/rust-lang/rust/issues/70861>",
};
}

declare_lint! {
/// The `nontrivial_structural_match` lint detects constants that are used in patterns,
/// whose type is not structural-match and whose initializer body actually uses values
/// that are not structural-match. So `Option<NotStructuralMatch>` is ok if the constant
/// is just `None`.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(nontrivial_structural_match)]
///
/// #[derive(Copy, Clone, Debug)]
/// struct NoDerive(u32);
/// impl PartialEq for NoDerive { fn eq(&self, _: &Self) -> bool { false } }
/// impl Eq for NoDerive { }
/// fn main() {
/// const INDEX: Option<NoDerive> = [None, Some(NoDerive(10))][0];
/// match None { Some(_) => panic!("whoops"), INDEX => dbg!(INDEX), };
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Previous versions of Rust accepted constants in patterns, even if those constants' types
/// did not have `PartialEq` derived. Thus the compiler falls back to runtime execution of
/// `PartialEq`, which can report that two constants are not equal even if they are
/// bit-equivalent.
pub NONTRIVIAL_STRUCTURAL_MATCH,
Warn,
"constant used in pattern of non-structural-match type and the constant's initializer \
expression contains values of non-structural-match types",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #73448 <https:/rust-lang/rust/issues/73448>",
reason: FutureIncompatibilityReason::FutureReleaseErrorReportInDeps,
reference: "issue #120362 <https:/rust-lang/rust/issues/120362>",
};
}

Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_middle/src/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,14 @@ pub struct BorrowCheckResult<'tcx> {

/// The result of the `mir_const_qualif` query.
///
/// Each field (except `error_occurred`) corresponds to an implementer of the `Qualif` trait in
/// Each field (except `tainted_by_errors`) corresponds to an implementer of the `Qualif` trait in
/// `rustc_const_eval/src/transform/check_consts/qualifs.rs`. See that file for more information on each
/// `Qualif`.
#[derive(Clone, Copy, Debug, Default, TyEncodable, TyDecodable, HashStable)]
pub struct ConstQualifs {
pub has_mut_interior: bool,
pub needs_drop: bool,
pub needs_non_const_drop: bool,
pub custom_eq: bool,
pub tainted_by_errors: Option<ErrorGuaranteed>,
}

Expand Down
Loading
Loading