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

make matching on NaN a hard error, and remove the rest of illegal_floating_point_literal_pattern #116284

Merged
merged 3 commits into from
Feb 5, 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
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,10 @@ fn register_builtins(store: &mut LintStore) {
"converted into hard error, see PR #118649 \
<https://github.com/rust-lang/rust/pull/118649> for more information",
);
store.register_removed(
"illegal_floating_point_literal_pattern",
"no longer a warning, float patterns behave the same as `==`",
);
}

fn register_internals(store: &mut LintStore) {
Expand Down
50 changes: 0 additions & 50 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ declare_lint_pass! {
FUZZY_PROVENANCE_CASTS,
HIDDEN_GLOB_REEXPORTS,
ILL_FORMED_ATTRIBUTE_INPUT,
ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
INCOMPLETE_INCLUDE,
INDIRECT_STRUCTURAL_MATCH,
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
Expand Down Expand Up @@ -1873,55 +1872,6 @@ declare_lint! {
};
}

declare_lint! {
/// The `illegal_floating_point_literal_pattern` lint detects
/// floating-point literals used in patterns.
///
/// ### Example
///
/// ```rust
/// let x = 42.0;
///
/// match x {
/// 5.0 => {}
/// _ => {}
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Previous versions of the compiler accepted floating-point literals in
/// patterns, but it was later determined this was a mistake. The
/// semantics of comparing floating-point values may not be clear in a
/// pattern when contrasted with "structural equality". Typically you can
/// work around this by using a [match guard], such as:
///
/// ```rust
/// # let x = 42.0;
///
/// match x {
/// y if y == 5.0 => {}
/// _ => {}
/// }
/// ```
///
/// This is a [future-incompatible] lint to transition this to a hard
/// error in the future. See [issue #41620] for more details.
///
/// [issue #41620]: https://github.com/rust-lang/rust/issues/41620
/// [match guard]: https://doc.rust-lang.org/reference/expressions/match-expr.html#match-guards
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
Warn,
"floating-point literals cannot be used in patterns",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #41620 <https://github.com/rust-lang/rust/issues/41620>",
};
}

declare_lint! {
/// The `unstable_name_collisions` lint detects that you have used a name
/// that the standard library plans to add in the future.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ impl<'tcx, Prov: Provenance> Scalar<Prov> {

#[inline]
pub fn to_float<F: Float>(self) -> InterpResult<'tcx, F> {
// Going through `to_uint` to check size and truncation.
Ok(F::from_bits(self.to_uint(Size::from_bits(F::BITS))?))
// Going through `to_bits` to check size and truncation.
Ok(F::from_bits(self.to_bits(Size::from_bits(F::BITS))?))
}

#[inline]
Expand Down
71 changes: 44 additions & 27 deletions compiler/rustc_middle/src/ty/consts/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,6 @@ impl ScalarInt {
}
}

#[inline]
pub fn try_to_target_usize(&self, tcx: TyCtxt<'_>) -> Result<u64, Size> {
Ok(self.to_bits(tcx.data_layout.pointer_size)? as u64)
}

/// Tries to convert the `ScalarInt` to an unsigned integer of the given size.
/// Fails if the size of the `ScalarInt` is not equal to `size` and returns the
/// `ScalarInt`s size in that case.
Expand All @@ -262,56 +257,61 @@ impl ScalarInt {
self.to_bits(size)
}

// Tries to convert the `ScalarInt` to `bool`. Fails if the `size` of the `ScalarInt`
// in not equal to `Size { raw: 1 }` or if the value is not 0 or 1 and returns the `size`
// value of the `ScalarInt` in that case.
#[inline]
pub fn try_to_bool(self) -> Result<bool, Size> {
match self.try_to_u8()? {
0 => Ok(false),
1 => Ok(true),
_ => Err(self.size()),
}
}

// Tries to convert the `ScalarInt` to `u8`. Fails if the `size` of the `ScalarInt`
// in not equal to `Size { raw: 1 }` and returns the `size` value of the `ScalarInt` in
// that case.
#[inline]
pub fn try_to_u8(self) -> Result<u8, Size> {
self.to_bits(Size::from_bits(8)).map(|v| u8::try_from(v).unwrap())
self.try_to_uint(Size::from_bits(8)).map(|v| u8::try_from(v).unwrap())
}

/// Tries to convert the `ScalarInt` to `u16`. Fails if the size of the `ScalarInt`
/// in not equal to `Size { raw: 2 }` and returns the `size` value of the `ScalarInt` in
/// that case.
#[inline]
pub fn try_to_u16(self) -> Result<u16, Size> {
self.to_bits(Size::from_bits(16)).map(|v| u16::try_from(v).unwrap())
self.try_to_uint(Size::from_bits(16)).map(|v| u16::try_from(v).unwrap())
}

/// Tries to convert the `ScalarInt` to `u32`. Fails if the `size` of the `ScalarInt`
/// in not equal to `Size { raw: 4 }` and returns the `size` value of the `ScalarInt` in
/// that case.
#[inline]
pub fn try_to_u32(self) -> Result<u32, Size> {
self.to_bits(Size::from_bits(32)).map(|v| u32::try_from(v).unwrap())
self.try_to_uint(Size::from_bits(32)).map(|v| u32::try_from(v).unwrap())
}

/// Tries to convert the `ScalarInt` to `u64`. Fails if the `size` of the `ScalarInt`
/// in not equal to `Size { raw: 8 }` and returns the `size` value of the `ScalarInt` in
/// that case.
#[inline]
pub fn try_to_u64(self) -> Result<u64, Size> {
self.to_bits(Size::from_bits(64)).map(|v| u64::try_from(v).unwrap())
self.try_to_uint(Size::from_bits(64)).map(|v| u64::try_from(v).unwrap())
}

/// Tries to convert the `ScalarInt` to `u128`. Fails if the `size` of the `ScalarInt`
/// in not equal to `Size { raw: 16 }` and returns the `size` value of the `ScalarInt` in
/// that case.
#[inline]
pub fn try_to_u128(self) -> Result<u128, Size> {
self.to_bits(Size::from_bits(128))
self.try_to_uint(Size::from_bits(128))
}

#[inline]
pub fn try_to_target_usize(&self, tcx: TyCtxt<'_>) -> Result<u64, Size> {
self.try_to_uint(tcx.data_layout.pointer_size).map(|v| u64::try_from(v).unwrap())
}

// Tries to convert the `ScalarInt` to `bool`. Fails if the `size` of the `ScalarInt`
// in not equal to `Size { raw: 1 }` or if the value is not 0 or 1 and returns the `size`
// value of the `ScalarInt` in that case.
#[inline]
pub fn try_to_bool(self) -> Result<bool, Size> {
match self.try_to_u8()? {
0 => Ok(false),
1 => Ok(true),
_ => Err(self.size()),
}
}

/// Tries to convert the `ScalarInt` to a signed integer of the given size.
Expand Down Expand Up @@ -357,6 +357,27 @@ impl ScalarInt {
pub fn try_to_i128(self) -> Result<i128, Size> {
self.try_to_int(Size::from_bits(128))
}

#[inline]
pub fn try_to_target_isize(&self, tcx: TyCtxt<'_>) -> Result<i64, Size> {
self.try_to_int(tcx.data_layout.pointer_size).map(|v| i64::try_from(v).unwrap())
}

#[inline]
pub fn try_to_float<F: Float>(self) -> Result<F, Size> {
// Going through `to_uint` to check size and truncation.
Ok(F::from_bits(self.to_bits(Size::from_bits(F::BITS))?))
}

#[inline]
pub fn try_to_f32(self) -> Result<Single, Size> {
self.try_to_float()
}

#[inline]
pub fn try_to_f64(self) -> Result<Double, Size> {
self.try_to_float()
}
}

macro_rules! from {
Expand Down Expand Up @@ -399,11 +420,7 @@ impl TryFrom<ScalarInt> for bool {
type Error = Size;
#[inline]
fn try_from(int: ScalarInt) -> Result<Self, Size> {
int.to_bits(Size::from_bytes(1)).and_then(|u| match u {
0 => Ok(false),
1 => Ok(true),
_ => Err(Size::from_bytes(1)),
})
int.try_to_bool()
}
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,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_float_pattern = floating-point types cannot be used in patterns

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)]`

Expand Down Expand Up @@ -232,6 +230,10 @@ mir_build_mutation_of_layout_constrained_field_requires_unsafe_unsafe_op_in_unsa
.note = mutating layout constrained fields cannot statically be checked for valid values
.label = mutation of layout constrained field

mir_build_nan_pattern = cannot use NaN in patterns
.note = NaNs compare inequal to everything, even themselves, so this pattern would never match
.help = try using the `is_nan` method instead

mir_build_non_const_path = runtime values cannot be referenced in patterns

mir_build_non_empty_never_pattern =
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,9 +780,14 @@ pub struct UnsizedPattern<'tcx> {
pub non_sm_ty: Ty<'tcx>,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_float_pattern)]
pub struct FloatPattern;
#[derive(Diagnostic)]
#[diag(mir_build_nan_pattern)]
#[note]
#[help]
pub struct NaNPattern {
#[primary_span]
pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag(mir_build_pointer_pattern)]
Expand Down
29 changes: 18 additions & 11 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rustc_apfloat::Float;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_index::Idx;
Expand All @@ -16,7 +17,7 @@ use std::cell::Cell;

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

Expand Down Expand Up @@ -317,16 +318,6 @@ impl<'tcx> ConstToPat<'tcx> {
let param_env = self.param_env;

let kind = match ty.kind() {
ty::Float(_) => {
self.saw_const_match_lint.set(true);
tcx.emit_node_span_lint(
lint::builtin::ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
id,
span,
FloatPattern,
);
return Err(FallbackToOpaqueConst);
}
// 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.
Expand Down Expand Up @@ -486,6 +477,22 @@ impl<'tcx> ConstToPat<'tcx> {
}
}
},
ty::Float(flt) => {
let v = cv.unwrap_leaf();
let is_nan = match flt {
ty::FloatTy::F32 => v.try_to_f32().unwrap().is_nan(),
ty::FloatTy::F64 => v.try_to_f64().unwrap().is_nan(),
};
if is_nan {
// NaNs are not ever equal to anything so they make no sense as patterns.
// Also see <https://github.com/rust-lang/rfcs/pull/3535>.
let e = tcx.dcx().emit_err(NaNPattern { span });
self.saw_const_match_error.set(Some(e));
return Err(FallbackToOpaqueConst);
} else {
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_value(tcx, cv, ty)) }
}
}
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => {
// The raw pointers we see here have been "vetted" by valtree construction to be
// just integers, so we simply allow them.
Expand Down
16 changes: 4 additions & 12 deletions src/tools/clippy/tests/ui/expect_tool_lint_rfc_2383.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,8 @@ mod rustc_ok {
pub fn rustc_lints() {
let x = 42.0;

#[expect(illegal_floating_point_literal_pattern)]
match x {
5.0 => {}
6.0 => {}
_ => {}
}
#[expect(invalid_nan_comparisons)]
let _b = x == f32::NAN;
}
}

Expand All @@ -38,13 +34,9 @@ mod rustc_warn {
pub fn rustc_lints() {
let x = 42;

#[expect(illegal_floating_point_literal_pattern)]
#[expect(invalid_nan_comparisons)]
//~^ ERROR: this lint expectation is unfulfilled
match x {
5 => {}
6 => {}
_ => {}
}
let _b = x == 5;
}
}

Expand Down
16 changes: 8 additions & 8 deletions src/tools/clippy/tests/ui/expect_tool_lint_rfc_2383.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:35:14
--> $DIR/expect_tool_lint_rfc_2383.rs:31:14
|
LL | #[expect(dead_code)]
| ^^^^^^^^^
Expand All @@ -8,31 +8,31 @@ LL | #[expect(dead_code)]
= help: to override `-D warnings` add `#[allow(unfulfilled_lint_expectations)]`

error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:41:18
--> $DIR/expect_tool_lint_rfc_2383.rs:37:18
|
LL | #[expect(illegal_floating_point_literal_pattern)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | #[expect(invalid_nan_comparisons)]
| ^^^^^^^^^^^^^^^^^^^^^^^

error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:116:14
--> $DIR/expect_tool_lint_rfc_2383.rs:108:14
|
LL | #[expect(clippy::almost_swapped)]
| ^^^^^^^^^^^^^^^^^^^^^^

error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:124:14
--> $DIR/expect_tool_lint_rfc_2383.rs:116:14
|
LL | #[expect(clippy::bytes_nth)]
| ^^^^^^^^^^^^^^^^^

error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:130:14
--> $DIR/expect_tool_lint_rfc_2383.rs:122:14
|
LL | #[expect(clippy::if_same_then_else)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: this lint expectation is unfulfilled
--> $DIR/expect_tool_lint_rfc_2383.rs:136:14
--> $DIR/expect_tool_lint_rfc_2383.rs:128:14
|
LL | #[expect(clippy::overly_complex_bool_expr)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
Loading
Loading