diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 51eb8210d46..5e13ee7b8a4 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -314,6 +314,11 @@ lint_invalid_from_utf8_checked = calls to `{$method}` with a invalid literal alw lint_invalid_from_utf8_unchecked = calls to `{$method}` with a invalid literal are undefined behavior .label = the literal was valid UTF-8 up to the {$valid_up_to} bytes +lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be directly compared to itself + .suggestion = use `f32::is_nan()` or `f64::is_nan()` instead + +lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable + lint_lintpass_by_hand = implementing `LintPass` by hand .help = try using `declare_lint_pass!` or `impl_lint_pass!` instead diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 03dbffcc2c4..e990c771bdf 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1434,6 +1434,36 @@ pub struct OverflowingLiteral<'a> { #[diag(lint_unused_comparisons)] pub struct UnusedComparisons; +#[derive(LintDiagnostic)] +pub enum InvalidNanComparisons { + #[diag(lint_invalid_nan_comparisons_eq_ne)] + EqNe { + #[subdiagnostic] + suggestion: InvalidNanComparisonsSuggestion, + }, + #[diag(lint_invalid_nan_comparisons_lt_le_gt_ge)] + LtLeGtGe, +} + +#[derive(Subdiagnostic)] +pub enum InvalidNanComparisonsSuggestion { + #[multipart_suggestion( + lint_suggestion, + style = "verbose", + applicability = "machine-applicable" + )] + Spanful { + #[suggestion_part(code = "!")] + neg: Option<Span>, + #[suggestion_part(code = ".is_nan()")] + float: Span, + #[suggestion_part(code = "")] + nan_plus_binop: Span, + }, + #[help(lint_suggestion)] + Spanless, +} + pub struct ImproperCTypes<'a> { pub ty: Ty<'a>, pub desc: &'a str, diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 4bf4fda8292..264a59c5585 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -2,10 +2,10 @@ use crate::{ fluent_generated as fluent, lints::{ AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes, - InvalidAtomicOrderingDiag, OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, - OverflowingBinHexSub, OverflowingInt, OverflowingIntHelp, OverflowingLiteral, - OverflowingUInt, RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange, - VariantSizeDifferencesDiag, + InvalidAtomicOrderingDiag, InvalidNanComparisons, InvalidNanComparisonsSuggestion, + OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, OverflowingBinHexSub, + OverflowingInt, OverflowingIntHelp, OverflowingLiteral, OverflowingUInt, + RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange, VariantSizeDifferencesDiag, }, }; use crate::{LateContext, LateLintPass, LintContext}; @@ -113,13 +113,35 @@ declare_lint! { "detects enums with widely varying variant sizes" } +declare_lint! { + /// The `invalid_nan_comparisons` lint checks comparison with `f32::NAN` or `f64::NAN` + /// as one of the operand. + /// + /// ### Example + /// + /// ```rust + /// let a = 2.3f32; + /// if a == f32::NAN {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// NaN does not compare meaningfully to anything – not + /// even itself – so those comparisons are always false. + INVALID_NAN_COMPARISONS, + Warn, + "detects invalid floating point NaN comparisons" +} + #[derive(Copy, Clone)] pub struct TypeLimits { /// Id of the last visited negated expression negated_expr_id: Option<hir::HirId>, } -impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS]); +impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS, INVALID_NAN_COMPARISONS]); impl TypeLimits { pub fn new() -> TypeLimits { @@ -486,6 +508,68 @@ fn lint_literal<'tcx>( } } +fn lint_nan<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx hir::Expr<'tcx>, + binop: hir::BinOp, + l: &'tcx hir::Expr<'tcx>, + r: &'tcx hir::Expr<'tcx>, +) { + fn is_nan(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + let expr = expr.peel_blocks().peel_borrows(); + match expr.kind { + ExprKind::Path(qpath) => { + let Some(def_id) = cx.typeck_results().qpath_res(&qpath, expr.hir_id).opt_def_id() else { return false; }; + + matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::f32_nan | sym::f64_nan)) + } + _ => false, + } + } + + fn eq_ne( + e: &hir::Expr<'_>, + l: &hir::Expr<'_>, + r: &hir::Expr<'_>, + f: impl FnOnce(Span, Span) -> InvalidNanComparisonsSuggestion, + ) -> InvalidNanComparisons { + let suggestion = + if let Some(l_span) = l.span.find_ancestor_inside(e.span) && + let Some(r_span) = r.span.find_ancestor_inside(e.span) { + f(l_span, r_span) + } else { + InvalidNanComparisonsSuggestion::Spanless + }; + + InvalidNanComparisons::EqNe { suggestion } + } + + let lint = match binop.node { + hir::BinOpKind::Eq | hir::BinOpKind::Ne if is_nan(cx, l) => { + eq_ne(e, l, r, |l_span, r_span| InvalidNanComparisonsSuggestion::Spanful { + nan_plus_binop: l_span.until(r_span), + float: r_span.shrink_to_hi(), + neg: (binop.node == hir::BinOpKind::Ne).then(|| r_span.shrink_to_lo()), + }) + } + hir::BinOpKind::Eq | hir::BinOpKind::Ne if is_nan(cx, r) => { + eq_ne(e, l, r, |l_span, r_span| InvalidNanComparisonsSuggestion::Spanful { + nan_plus_binop: l_span.shrink_to_hi().to(r_span), + float: l_span.shrink_to_hi(), + neg: (binop.node == hir::BinOpKind::Ne).then(|| l_span.shrink_to_lo()), + }) + } + hir::BinOpKind::Lt | hir::BinOpKind::Le | hir::BinOpKind::Gt | hir::BinOpKind::Ge + if is_nan(cx, l) || is_nan(cx, r) => + { + InvalidNanComparisons::LtLeGtGe + } + _ => return, + }; + + cx.emit_spanned_lint(INVALID_NAN_COMPARISONS, e.span, lint); +} + impl<'tcx> LateLintPass<'tcx> for TypeLimits { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) { match e.kind { @@ -496,8 +580,12 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { } } hir::ExprKind::Binary(binop, ref l, ref r) => { - if is_comparison(binop) && !check_limits(cx, binop, &l, &r) { - cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons); + if is_comparison(binop) { + if !check_limits(cx, binop, &l, &r) { + cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons); + } else { + lint_nan(cx, e, binop, l, r); + } } } hir::ExprKind::Lit(ref lit) => lint_literal(cx, self, e, lit), diff --git a/tests/ui/issues/issue-50811.rs b/tests/ui/issues/issue-50811.rs index 683c856049f..2a20e50fa45 100644 --- a/tests/ui/issues/issue-50811.rs +++ b/tests/ui/issues/issue-50811.rs @@ -1,5 +1,6 @@ // run-pass #![feature(test)] +#![allow(invalid_nan_comparisons)] extern crate test; diff --git a/tests/ui/lint/invalid-nan-comparison-suggestion.fixed b/tests/ui/lint/invalid-nan-comparison-suggestion.fixed new file mode 100644 index 00000000000..feafc6c1b8c --- /dev/null +++ b/tests/ui/lint/invalid-nan-comparison-suggestion.fixed @@ -0,0 +1,36 @@ +// check-pass +// run-rustfix + +fn main() { + let x = 5f32; + let _ = x.is_nan(); + //~^ WARN incorrect NaN comparison + let _ = !x.is_nan(); + //~^ WARN incorrect NaN comparison + + let x = 5f64; + let _ = x.is_nan(); + //~^ WARN incorrect NaN comparison + let _ = !x.is_nan(); + //~^ WARN incorrect NaN comparison + + let b = &2.3f32; + if !b.is_nan() {} + //~^ WARN incorrect NaN comparison + + let b = &2.3f32; + if !b.is_nan() {} + //~^ WARN incorrect NaN comparison + + let _ = + !b.is_nan(); + + #[allow(unused_macros)] + macro_rules! nan { () => { f32::NAN }; } + macro_rules! number { () => { 5f32 }; } + + let _ = number!().is_nan(); + //~^ WARN incorrect NaN comparison + let _ = !number!().is_nan(); + //~^ WARN incorrect NaN comparison +} diff --git a/tests/ui/lint/invalid-nan-comparison-suggestion.rs b/tests/ui/lint/invalid-nan-comparison-suggestion.rs new file mode 100644 index 00000000000..ad5eb66e5f1 --- /dev/null +++ b/tests/ui/lint/invalid-nan-comparison-suggestion.rs @@ -0,0 +1,39 @@ +// check-pass +// run-rustfix + +fn main() { + let x = 5f32; + let _ = x == f32::NAN; + //~^ WARN incorrect NaN comparison + let _ = x != f32::NAN; + //~^ WARN incorrect NaN comparison + + let x = 5f64; + let _ = x == f64::NAN; + //~^ WARN incorrect NaN comparison + let _ = x != f64::NAN; + //~^ WARN incorrect NaN comparison + + let b = &2.3f32; + if b != &f32::NAN {} + //~^ WARN incorrect NaN comparison + + let b = &2.3f32; + if b != { &f32::NAN } {} + //~^ WARN incorrect NaN comparison + + let _ = + b != { + //~^ WARN incorrect NaN comparison + &f32::NAN + }; + + #[allow(unused_macros)] + macro_rules! nan { () => { f32::NAN }; } + macro_rules! number { () => { 5f32 }; } + + let _ = nan!() == number!(); + //~^ WARN incorrect NaN comparison + let _ = number!() != nan!(); + //~^ WARN incorrect NaN comparison +} diff --git a/tests/ui/lint/invalid-nan-comparison-suggestion.stderr b/tests/ui/lint/invalid-nan-comparison-suggestion.stderr new file mode 100644 index 00000000000..c310341de07 --- /dev/null +++ b/tests/ui/lint/invalid-nan-comparison-suggestion.stderr @@ -0,0 +1,114 @@ +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:6:13 + | +LL | let _ = x == f32::NAN; + | ^^^^^^^^^^^^^ + | + = note: `#[warn(invalid_nan_comparisons)]` on by default +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - let _ = x == f32::NAN; +LL + let _ = x.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:8:13 + | +LL | let _ = x != f32::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - let _ = x != f32::NAN; +LL + let _ = !x.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:12:13 + | +LL | let _ = x == f64::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - let _ = x == f64::NAN; +LL + let _ = x.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:14:13 + | +LL | let _ = x != f64::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - let _ = x != f64::NAN; +LL + let _ = !x.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:18:8 + | +LL | if b != &f32::NAN {} + | ^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - if b != &f32::NAN {} +LL + if !b.is_nan() {} + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:22:8 + | +LL | if b != { &f32::NAN } {} + | ^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - if b != { &f32::NAN } {} +LL + if !b.is_nan() {} + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:26:9 + | +LL | / b != { +LL | | +LL | | &f32::NAN +LL | | }; + | |_________^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - b != { +LL + !b.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:35:13 + | +LL | let _ = nan!() == number!(); + | ^^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - let _ = nan!() == number!(); +LL + let _ = number!().is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison-suggestion.rs:37:13 + | +LL | let _ = number!() != nan!(); + | ^^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - let _ = number!() != nan!(); +LL + let _ = !number!().is_nan(); + | + +warning: 9 warnings emitted + diff --git a/tests/ui/lint/invalid-nan-comparison.rs b/tests/ui/lint/invalid-nan-comparison.rs new file mode 100644 index 00000000000..d7e793ca583 --- /dev/null +++ b/tests/ui/lint/invalid-nan-comparison.rs @@ -0,0 +1,51 @@ +// check-pass + +fn main() { + f32(); + f64(); +} + +const TEST: bool = 5f32 == f32::NAN; +//~^ WARN incorrect NaN comparison + +fn f32() { + macro_rules! number { () => { 5f32 }; } + let x = number!(); + x == f32::NAN; + //~^ WARN incorrect NaN comparison + x != f32::NAN; + //~^ WARN incorrect NaN comparison + x < f32::NAN; + //~^ WARN incorrect NaN comparison + x > f32::NAN; + //~^ WARN incorrect NaN comparison + x <= f32::NAN; + //~^ WARN incorrect NaN comparison + x >= f32::NAN; + //~^ WARN incorrect NaN comparison + number!() == f32::NAN; + //~^ WARN incorrect NaN comparison + f32::NAN != number!(); + //~^ WARN incorrect NaN comparison +} + +fn f64() { + macro_rules! number { () => { 5f64 }; } + let x = number!(); + x == f64::NAN; + //~^ WARN incorrect NaN comparison + x != f64::NAN; + //~^ WARN incorrect NaN comparison + x < f64::NAN; + //~^ WARN incorrect NaN comparison + x > f64::NAN; + //~^ WARN incorrect NaN comparison + x <= f64::NAN; + //~^ WARN incorrect NaN comparison + x >= f64::NAN; + //~^ WARN incorrect NaN comparison + number!() == f64::NAN; + //~^ WARN incorrect NaN comparison + f64::NAN != number!(); + //~^ WARN incorrect NaN comparison +} diff --git a/tests/ui/lint/invalid-nan-comparison.stderr b/tests/ui/lint/invalid-nan-comparison.stderr new file mode 100644 index 00000000000..054c06d38b3 --- /dev/null +++ b/tests/ui/lint/invalid-nan-comparison.stderr @@ -0,0 +1,159 @@ +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:8:20 + | +LL | const TEST: bool = 5f32 == f32::NAN; + | ^^^^^^^^^^^^^^^^ + | + = note: `#[warn(invalid_nan_comparisons)]` on by default +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - const TEST: bool = 5f32 == f32::NAN; +LL + const TEST: bool = 5f32.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:14:5 + | +LL | x == f32::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - x == f32::NAN; +LL + x.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:16:5 + | +LL | x != f32::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - x != f32::NAN; +LL + !x.is_nan(); + | + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:18:5 + | +LL | x < f32::NAN; + | ^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:20:5 + | +LL | x > f32::NAN; + | ^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:22:5 + | +LL | x <= f32::NAN; + | ^^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:24:5 + | +LL | x >= f32::NAN; + | ^^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:26:5 + | +LL | number!() == f32::NAN; + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - number!() == f32::NAN; +LL + number!().is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:28:5 + | +LL | f32::NAN != number!(); + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - f32::NAN != number!(); +LL + !number!().is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:35:5 + | +LL | x == f64::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - x == f64::NAN; +LL + x.is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:37:5 + | +LL | x != f64::NAN; + | ^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - x != f64::NAN; +LL + !x.is_nan(); + | + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:39:5 + | +LL | x < f64::NAN; + | ^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:41:5 + | +LL | x > f64::NAN; + | ^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:43:5 + | +LL | x <= f64::NAN; + | ^^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN is not orderable + --> $DIR/invalid-nan-comparison.rs:45:5 + | +LL | x >= f64::NAN; + | ^^^^^^^^^^^^^ + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:47:5 + | +LL | number!() == f64::NAN; + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - number!() == f64::NAN; +LL + number!().is_nan(); + | + +warning: incorrect NaN comparison, NaN cannot be directly compared to itself + --> $DIR/invalid-nan-comparison.rs:49:5 + | +LL | f64::NAN != number!(); + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: use `f32::is_nan()` or `f64::is_nan()` instead + | +LL - f64::NAN != number!(); +LL + !number!().is_nan(); + | + +warning: 17 warnings emitted +