Skip to content

Commit 3e91349

Browse files
committed
Uplift improved version of clippy::cmp_nan to rustc
1 parent 3681285 commit 3e91349

9 files changed

+530
-7
lines changed

Diff for: compiler/rustc_lint/messages.ftl

+5
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,11 @@ lint_invalid_from_utf8_checked = calls to `{$method}` with a invalid literal alw
314314
lint_invalid_from_utf8_unchecked = calls to `{$method}` with a invalid literal are undefined behavior
315315
.label = the literal was valid UTF-8 up to the {$valid_up_to} bytes
316316
317+
lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be directly compared to itself
318+
.suggestion = use `f32::is_nan()` or `f64::is_nan()` instead
319+
320+
lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable
321+
317322
lint_lintpass_by_hand = implementing `LintPass` by hand
318323
.help = try using `declare_lint_pass!` or `impl_lint_pass!` instead
319324

Diff for: compiler/rustc_lint/src/lints.rs

+30
Original file line numberDiff line numberDiff line change
@@ -1434,6 +1434,36 @@ pub struct OverflowingLiteral<'a> {
14341434
#[diag(lint_unused_comparisons)]
14351435
pub struct UnusedComparisons;
14361436

1437+
#[derive(LintDiagnostic)]
1438+
pub enum InvalidNanComparisons {
1439+
#[diag(lint_invalid_nan_comparisons_eq_ne)]
1440+
EqNe {
1441+
#[subdiagnostic]
1442+
suggestion: InvalidNanComparisonsSuggestion,
1443+
},
1444+
#[diag(lint_invalid_nan_comparisons_lt_le_gt_ge)]
1445+
LtLeGtGe,
1446+
}
1447+
1448+
#[derive(Subdiagnostic)]
1449+
pub enum InvalidNanComparisonsSuggestion {
1450+
#[multipart_suggestion(
1451+
lint_suggestion,
1452+
style = "verbose",
1453+
applicability = "machine-applicable"
1454+
)]
1455+
Spanful {
1456+
#[suggestion_part(code = "!")]
1457+
neg: Option<Span>,
1458+
#[suggestion_part(code = ".is_nan()")]
1459+
float: Span,
1460+
#[suggestion_part(code = "")]
1461+
nan_plus_binop: Span,
1462+
},
1463+
#[help(lint_suggestion)]
1464+
Spanless,
1465+
}
1466+
14371467
pub struct ImproperCTypes<'a> {
14381468
pub ty: Ty<'a>,
14391469
pub desc: &'a str,

Diff for: compiler/rustc_lint/src/types.rs

+95-7
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ use crate::{
22
fluent_generated as fluent,
33
lints::{
44
AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes,
5-
InvalidAtomicOrderingDiag, OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign,
6-
OverflowingBinHexSub, OverflowingInt, OverflowingIntHelp, OverflowingLiteral,
7-
OverflowingUInt, RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange,
8-
VariantSizeDifferencesDiag,
5+
InvalidAtomicOrderingDiag, InvalidNanComparisons, InvalidNanComparisonsSuggestion,
6+
OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, OverflowingBinHexSub,
7+
OverflowingInt, OverflowingIntHelp, OverflowingLiteral, OverflowingUInt,
8+
RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange, VariantSizeDifferencesDiag,
99
},
1010
};
1111
use crate::{LateContext, LateLintPass, LintContext};
@@ -113,13 +113,35 @@ declare_lint! {
113113
"detects enums with widely varying variant sizes"
114114
}
115115

116+
declare_lint! {
117+
/// The `invalid_nan_comparisons` lint checks comparison with `f32::NAN` or `f64::NAN`
118+
/// as one of the operand.
119+
///
120+
/// ### Example
121+
///
122+
/// ```rust
123+
/// let a = 2.3f32;
124+
/// if a == f32::NAN {}
125+
/// ```
126+
///
127+
/// {{produces}}
128+
///
129+
/// ### Explanation
130+
///
131+
/// NaN does not compare meaningfully to anything – not
132+
/// even itself – so those comparisons are always false.
133+
INVALID_NAN_COMPARISONS,
134+
Warn,
135+
"detects invalid floating point NaN comparisons"
136+
}
137+
116138
#[derive(Copy, Clone)]
117139
pub struct TypeLimits {
118140
/// Id of the last visited negated expression
119141
negated_expr_id: Option<hir::HirId>,
120142
}
121143

122-
impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS]);
144+
impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS, INVALID_NAN_COMPARISONS]);
123145

124146
impl TypeLimits {
125147
pub fn new() -> TypeLimits {
@@ -486,6 +508,68 @@ fn lint_literal<'tcx>(
486508
}
487509
}
488510

511+
fn lint_nan<'tcx>(
512+
cx: &LateContext<'tcx>,
513+
e: &'tcx hir::Expr<'tcx>,
514+
binop: hir::BinOp,
515+
l: &'tcx hir::Expr<'tcx>,
516+
r: &'tcx hir::Expr<'tcx>,
517+
) {
518+
fn is_nan(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
519+
let expr = expr.peel_blocks().peel_borrows();
520+
match expr.kind {
521+
ExprKind::Path(qpath) => {
522+
let Some(def_id) = cx.typeck_results().qpath_res(&qpath, expr.hir_id).opt_def_id() else { return false; };
523+
524+
matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::f32_nan | sym::f64_nan))
525+
}
526+
_ => false,
527+
}
528+
}
529+
530+
fn eq_ne(
531+
e: &hir::Expr<'_>,
532+
l: &hir::Expr<'_>,
533+
r: &hir::Expr<'_>,
534+
f: impl FnOnce(Span, Span) -> InvalidNanComparisonsSuggestion,
535+
) -> InvalidNanComparisons {
536+
let suggestion =
537+
if let Some(l_span) = l.span.find_ancestor_inside(e.span) &&
538+
let Some(r_span) = r.span.find_ancestor_inside(e.span) {
539+
f(l_span, r_span)
540+
} else {
541+
InvalidNanComparisonsSuggestion::Spanless
542+
};
543+
544+
InvalidNanComparisons::EqNe { suggestion }
545+
}
546+
547+
let lint = match binop.node {
548+
hir::BinOpKind::Eq | hir::BinOpKind::Ne if is_nan(cx, l) => {
549+
eq_ne(e, l, r, |l_span, r_span| InvalidNanComparisonsSuggestion::Spanful {
550+
nan_plus_binop: l_span.until(r_span),
551+
float: r_span.shrink_to_hi(),
552+
neg: (binop.node == hir::BinOpKind::Ne).then(|| r_span.shrink_to_lo()),
553+
})
554+
}
555+
hir::BinOpKind::Eq | hir::BinOpKind::Ne if is_nan(cx, r) => {
556+
eq_ne(e, l, r, |l_span, r_span| InvalidNanComparisonsSuggestion::Spanful {
557+
nan_plus_binop: l_span.shrink_to_hi().to(r_span),
558+
float: l_span.shrink_to_hi(),
559+
neg: (binop.node == hir::BinOpKind::Ne).then(|| l_span.shrink_to_lo()),
560+
})
561+
}
562+
hir::BinOpKind::Lt | hir::BinOpKind::Le | hir::BinOpKind::Gt | hir::BinOpKind::Ge
563+
if is_nan(cx, l) || is_nan(cx, r) =>
564+
{
565+
InvalidNanComparisons::LtLeGtGe
566+
}
567+
_ => return,
568+
};
569+
570+
cx.emit_spanned_lint(INVALID_NAN_COMPARISONS, e.span, lint);
571+
}
572+
489573
impl<'tcx> LateLintPass<'tcx> for TypeLimits {
490574
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) {
491575
match e.kind {
@@ -496,8 +580,12 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits {
496580
}
497581
}
498582
hir::ExprKind::Binary(binop, ref l, ref r) => {
499-
if is_comparison(binop) && !check_limits(cx, binop, &l, &r) {
500-
cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons);
583+
if is_comparison(binop) {
584+
if !check_limits(cx, binop, &l, &r) {
585+
cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons);
586+
} else {
587+
lint_nan(cx, e, binop, l, r);
588+
}
501589
}
502590
}
503591
hir::ExprKind::Lit(ref lit) => lint_literal(cx, self, e, lit),

Diff for: tests/ui/issues/issue-50811.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// run-pass
22
#![feature(test)]
3+
#![allow(invalid_nan_comparisons)]
34

45
extern crate test;
56

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// check-pass
2+
// run-rustfix
3+
4+
fn main() {
5+
let x = 5f32;
6+
let _ = x.is_nan();
7+
//~^ WARN incorrect NaN comparison
8+
let _ = !x.is_nan();
9+
//~^ WARN incorrect NaN comparison
10+
11+
let x = 5f64;
12+
let _ = x.is_nan();
13+
//~^ WARN incorrect NaN comparison
14+
let _ = !x.is_nan();
15+
//~^ WARN incorrect NaN comparison
16+
17+
let b = &2.3f32;
18+
if !b.is_nan() {}
19+
//~^ WARN incorrect NaN comparison
20+
21+
let b = &2.3f32;
22+
if !b.is_nan() {}
23+
//~^ WARN incorrect NaN comparison
24+
25+
let _ =
26+
!b.is_nan();
27+
28+
#[allow(unused_macros)]
29+
macro_rules! nan { () => { f32::NAN }; }
30+
macro_rules! number { () => { 5f32 }; }
31+
32+
let _ = number!().is_nan();
33+
//~^ WARN incorrect NaN comparison
34+
let _ = !number!().is_nan();
35+
//~^ WARN incorrect NaN comparison
36+
}

Diff for: tests/ui/lint/invalid-nan-comparison-suggestion.rs

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// check-pass
2+
// run-rustfix
3+
4+
fn main() {
5+
let x = 5f32;
6+
let _ = x == f32::NAN;
7+
//~^ WARN incorrect NaN comparison
8+
let _ = x != f32::NAN;
9+
//~^ WARN incorrect NaN comparison
10+
11+
let x = 5f64;
12+
let _ = x == f64::NAN;
13+
//~^ WARN incorrect NaN comparison
14+
let _ = x != f64::NAN;
15+
//~^ WARN incorrect NaN comparison
16+
17+
let b = &2.3f32;
18+
if b != &f32::NAN {}
19+
//~^ WARN incorrect NaN comparison
20+
21+
let b = &2.3f32;
22+
if b != { &f32::NAN } {}
23+
//~^ WARN incorrect NaN comparison
24+
25+
let _ =
26+
b != {
27+
//~^ WARN incorrect NaN comparison
28+
&f32::NAN
29+
};
30+
31+
#[allow(unused_macros)]
32+
macro_rules! nan { () => { f32::NAN }; }
33+
macro_rules! number { () => { 5f32 }; }
34+
35+
let _ = nan!() == number!();
36+
//~^ WARN incorrect NaN comparison
37+
let _ = number!() != nan!();
38+
//~^ WARN incorrect NaN comparison
39+
}
+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
2+
--> $DIR/invalid-nan-comparison-suggestion.rs:6:13
3+
|
4+
LL | let _ = x == f32::NAN;
5+
| ^^^^^^^^^^^^^
6+
|
7+
= note: `#[warn(invalid_nan_comparisons)]` on by default
8+
help: use `f32::is_nan()` or `f64::is_nan()` instead
9+
|
10+
LL - let _ = x == f32::NAN;
11+
LL + let _ = x.is_nan();
12+
|
13+
14+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
15+
--> $DIR/invalid-nan-comparison-suggestion.rs:8:13
16+
|
17+
LL | let _ = x != f32::NAN;
18+
| ^^^^^^^^^^^^^
19+
|
20+
help: use `f32::is_nan()` or `f64::is_nan()` instead
21+
|
22+
LL - let _ = x != f32::NAN;
23+
LL + let _ = !x.is_nan();
24+
|
25+
26+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
27+
--> $DIR/invalid-nan-comparison-suggestion.rs:12:13
28+
|
29+
LL | let _ = x == f64::NAN;
30+
| ^^^^^^^^^^^^^
31+
|
32+
help: use `f32::is_nan()` or `f64::is_nan()` instead
33+
|
34+
LL - let _ = x == f64::NAN;
35+
LL + let _ = x.is_nan();
36+
|
37+
38+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
39+
--> $DIR/invalid-nan-comparison-suggestion.rs:14:13
40+
|
41+
LL | let _ = x != f64::NAN;
42+
| ^^^^^^^^^^^^^
43+
|
44+
help: use `f32::is_nan()` or `f64::is_nan()` instead
45+
|
46+
LL - let _ = x != f64::NAN;
47+
LL + let _ = !x.is_nan();
48+
|
49+
50+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
51+
--> $DIR/invalid-nan-comparison-suggestion.rs:18:8
52+
|
53+
LL | if b != &f32::NAN {}
54+
| ^^^^^^^^^^^^^^
55+
|
56+
help: use `f32::is_nan()` or `f64::is_nan()` instead
57+
|
58+
LL - if b != &f32::NAN {}
59+
LL + if !b.is_nan() {}
60+
|
61+
62+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
63+
--> $DIR/invalid-nan-comparison-suggestion.rs:22:8
64+
|
65+
LL | if b != { &f32::NAN } {}
66+
| ^^^^^^^^^^^^^^^^^^
67+
|
68+
help: use `f32::is_nan()` or `f64::is_nan()` instead
69+
|
70+
LL - if b != { &f32::NAN } {}
71+
LL + if !b.is_nan() {}
72+
|
73+
74+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
75+
--> $DIR/invalid-nan-comparison-suggestion.rs:26:9
76+
|
77+
LL | / b != {
78+
LL | |
79+
LL | | &f32::NAN
80+
LL | | };
81+
| |_________^
82+
|
83+
help: use `f32::is_nan()` or `f64::is_nan()` instead
84+
|
85+
LL - b != {
86+
LL + !b.is_nan();
87+
|
88+
89+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
90+
--> $DIR/invalid-nan-comparison-suggestion.rs:35:13
91+
|
92+
LL | let _ = nan!() == number!();
93+
| ^^^^^^^^^^^^^^^^^^^
94+
|
95+
help: use `f32::is_nan()` or `f64::is_nan()` instead
96+
|
97+
LL - let _ = nan!() == number!();
98+
LL + let _ = number!().is_nan();
99+
|
100+
101+
warning: incorrect NaN comparison, NaN cannot be directly compared to itself
102+
--> $DIR/invalid-nan-comparison-suggestion.rs:37:13
103+
|
104+
LL | let _ = number!() != nan!();
105+
| ^^^^^^^^^^^^^^^^^^^
106+
|
107+
help: use `f32::is_nan()` or `f64::is_nan()` instead
108+
|
109+
LL - let _ = number!() != nan!();
110+
LL + let _ = !number!().is_nan();
111+
|
112+
113+
warning: 9 warnings emitted
114+

0 commit comments

Comments
 (0)