Skip to content

Commit

Permalink
float_cmp: Allow named constants by path.
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed Jul 18, 2024
1 parent 63d5498 commit e1550af
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 134 deletions.
2 changes: 1 addition & 1 deletion clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ define_Conf! {
/// x == VALUE
/// }
/// ```
(float_cmp_allowed_constants: Vec<String> = true),
(float_cmp_allowed_constants: Vec<String> = Vec::new()),
/// Lint: FLOAT_CMP
///
/// Whether to ignore comparisons which have a constant result.
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(manual_rem_euclid::ManualRemEuclid::new(conf)));
store.register_late_pass(move |_| Box::new(manual_retain::ManualRetain::new(conf)));
store.register_late_pass(move |_| Box::new(manual_rotate::ManualRotate));
store.register_late_pass(move |_| Box::new(operators::Operators::new(conf)));
store.register_late_pass(move |tcx| Box::new(operators::Operators::new(tcx, conf)));
store.register_late_pass(|_| Box::<std_instead_of_core::StdReexports>::default());
store.register_late_pass(move |_| Box::new(instant_subtraction::InstantSubtraction::new(conf)));
store.register_late_pass(|_| Box::new(partialeq_to_none::PartialeqToNone));
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/operators/float_cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::sugg::Sugg;
use clippy_utils::visitors::{for_each_expr_without_closures, is_const_evaluatable};
use clippy_utils::{get_item_name, is_expr_named_const, path_res, peel_hir_expr_while, SpanlessEq};
use clippy_utils::{get_item_name, get_named_const_def_id, path_res, peel_hir_expr_while, SpanlessEq};
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
Expand All @@ -14,7 +14,7 @@ use super::{FloatCmpConfig, FLOAT_CMP};

pub(crate) fn check<'tcx>(
cx: &LateContext<'tcx>,
config: FloatCmpConfig,
config: &FloatCmpConfig,
expr: &'tcx Expr<'_>,
op: BinOpKind,
left: &'tcx Expr<'_>,
Expand Down Expand Up @@ -56,8 +56,8 @@ pub(crate) fn check<'tcx>(
return;
}

if config.ignore_named_constants
&& (is_expr_named_const(cx, left_reduced) || is_expr_named_const(cx, right_reduced))
if get_named_const_def_id(cx, left_reduced).is_some_and(|id| config.allowed_constants.contains(&id))
|| get_named_const_def_id(cx, right_reduced).is_some_and(|id| config.allowed_constants.contains(&id))
{
return;
}
Expand Down
5 changes: 3 additions & 2 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use clippy_utils::def_path_def_ids;
use rustc_hir::def_id::DefIdSet;
use rustc_hir::{Body, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass;

declare_clippy_lint! {
Expand Down Expand Up @@ -789,7 +790,7 @@ pub struct Operators {
float_cmp_config: FloatCmpConfig,
}
impl Operators {
pub fn new(conf: &'static Conf) -> Self {
pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self {
Self {
arithmetic_context: numeric_arithmetic::Context::default(),
verbose_bit_mask_threshold: conf.verbose_bit_mask_threshold,
Expand Down Expand Up @@ -859,7 +860,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators {
float_equality_without_abs::check(cx, e, op.node, lhs, rhs);
integer_division::check(cx, e, op.node, lhs, rhs);
cmp_owned::check(cx, op.node, lhs, rhs);
float_cmp::check(cx, self.float_cmp_config, e, op.node, lhs, rhs);
float_cmp::check(cx, &self.float_cmp_config, e, op.node, lhs, rhs);
modulo_one::check(cx, e, op.node, rhs);
modulo_arithmetic::check(
cx,
Expand Down
17 changes: 9 additions & 8 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,15 @@ pub fn is_inside_always_const_context(tcx: TyCtxt<'_>, hir_id: HirId) -> bool {
}
}

/// Checks if the expression is path to either a constant or an associated constant.
pub fn is_expr_named_const<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> bool {
matches!(&e.kind, ExprKind::Path(p)
if matches!(
cx.qpath_res(p, e.hir_id),
Res::Def(DefKind::Const | DefKind::AssocConst, _)
)
)
/// If the expression is path to either a constant or an associated constant get the `DefId`.
pub fn get_named_const_def_id<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> Option<DefId> {
if let ExprKind::Path(p) = &e.kind
&& let Res::Def(DefKind::Const | DefKind::AssocConst, id) = cx.qpath_res(p, e.hir_id)
{
Some(id)
} else {
None
}
}

/// Checks if a `Res` refers to a constructor of a `LangItem`
Expand Down
5 changes: 5 additions & 0 deletions tests/ui-toml/float_cmp/change_detect/clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
float-cmp-ignore-change-detection = false
float-cmp-allowed-constants = [
"core::f32::EPSILON",
"f32::EPSILON",
"test::F32_ARRAY",
]
5 changes: 5 additions & 0 deletions tests/ui-toml/float_cmp/const_cmp/clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
float-cmp-ignore-constant-comparisons = false
float-cmp-allowed-constants = [
"core::f32::EPSILON",
"f32::EPSILON",
"test::F32_ARRAY",
]
1 change: 0 additions & 1 deletion tests/ui-toml/float_cmp/named_const/clippy.toml
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
float-cmp-ignore-named-constants = false
52 changes: 26 additions & 26 deletions tests/ui-toml/float_cmp/test.change_detect.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:15:21
--> tests/ui-toml/float_cmp/test.rs:17:21
|
LL | let _ = x == y;
| ^^^^^^ help: consider comparing them within some margin of error: `(x - y).abs() < error_margin`
Expand All @@ -11,130 +11,130 @@ LL | #![deny(clippy::float_cmp)]
| ^^^^^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:16:21
--> tests/ui-toml/float_cmp/test.rs:18:21
|
LL | let _ = x != y;
| ^^^^^^ help: consider comparing them within some margin of error: `(x - y).abs() > error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:17:21
--> tests/ui-toml/float_cmp/test.rs:19:21
|
LL | let _ = x == 5.5;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(x - 5.5).abs() < error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:18:21
--> tests/ui-toml/float_cmp/test.rs:20:21
|
LL | let _ = 5.5 == x;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(5.5 - x).abs() < error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:41:21
--> tests/ui-toml/float_cmp/test.rs:43:21
|
LL | let _ = x == y;
| ^^^^^^ help: consider comparing them within some margin of error: `(x - y).abs() < error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:42:21
--> tests/ui-toml/float_cmp/test.rs:44:21
|
LL | let _ = x != y;
| ^^^^^^ help: consider comparing them within some margin of error: `(x - y).abs() > error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:43:21
--> tests/ui-toml/float_cmp/test.rs:45:21
|
LL | let _ = x == 5.5;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(x - 5.5).abs() < error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:44:21
--> tests/ui-toml/float_cmp/test.rs:46:21
|
LL | let _ = 5.5 == x;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(5.5 - x).abs() < error_margin`

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:67:21
--> tests/ui-toml/float_cmp/test.rs:69:21
|
LL | let _ = x == y;
| ^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:68:21
--> tests/ui-toml/float_cmp/test.rs:70:21
|
LL | let _ = x == [5.5; 4];
| ^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:69:21
--> tests/ui-toml/float_cmp/test.rs:71:21
|
LL | let _ = [5.5; 4] == x;
| ^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:70:21
--> tests/ui-toml/float_cmp/test.rs:72:21
|
LL | let _ = [0.0, 0.0, 0.0, 5.5] == x;
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:71:21
--> tests/ui-toml/float_cmp/test.rs:73:21
|
LL | let _ = x == [0.0, 0.0, 0.0, 5.5];
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:87:21
--> tests/ui-toml/float_cmp/test.rs:89:21
|
LL | let _ = x == y;
| ^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:88:21
--> tests/ui-toml/float_cmp/test.rs:90:21
|
LL | let _ = x == [5.5; 4];
| ^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:89:21
--> tests/ui-toml/float_cmp/test.rs:91:21
|
LL | let _ = [5.5; 4] == x;
| ^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:90:21
--> tests/ui-toml/float_cmp/test.rs:92:21
|
LL | let _ = [0.0, 0.0, 0.0, 5.5] == x;
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:91:21
--> tests/ui-toml/float_cmp/test.rs:93:21
|
LL | let _ = x == [0.0, 0.0, 0.0, 5.5];
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:109:21
--> tests/ui-toml/float_cmp/test.rs:111:21
|
LL | let _ = x == y;
| ^^^^^^ help: consider comparing them within some margin of error: `(x - y).abs() < error_margin`

error: strict comparison of `f32` or `f64` arrays
--> tests/ui-toml/float_cmp/test.rs:115:21
--> tests/ui-toml/float_cmp/test.rs:117:21
|
LL | let _ = x == y;
| ^^^^^^

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:131:21
--> tests/ui-toml/float_cmp/test.rs:132:21
|
LL | let _ = C * x == x * x;
| ^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(C * x - x * x).abs() < error_margin`
LL | let _ = f32::EPSILON * x == x * x;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(f32::EPSILON * x - x * x).abs() < error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:132:21
--> tests/ui-toml/float_cmp/test.rs:133:21
|
LL | let _ = x * x == C * x;
| ^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(x * x - C * x).abs() < error_margin`
LL | let _ = x * x == f32::EPSILON * x;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(x * x - f32::EPSILON * x).abs() < error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui-toml/float_cmp/test.rs:158:17
Expand Down
Loading

0 comments on commit e1550af

Please sign in to comment.