fix: Improve floating point lint to handle ambiguous type (#15133)

Fixes: rust-lang/rust-clippy#14897, rust-lang/rust-clippy#10015

This is not a perfect solution, but it fixes the issue. Happy to discuss
further with the clippy team.

This PR refines check_mul_add to avoid false positives on ambiguous
float types (e.g., f32 vs f64). It improves type detection and ensures
mul_add is only suggested when the type is clear.

Changes:
- Improved detection of ambiguous float types
- Refined mul_add suggestion logic
- Added tests for ambiguous cases
## Example

Before this change, expressions like:
```rust
let x = 1.0; // ambiguous type
let _ = x * 2.0 + 0.5; // could incorrectly suggest mul_add
```

----

changelog: [`suboptimal_flops`]: improve handling of ambiguous float
types in mul_add suggestions

clippy_lints/src/floating_point_arithmetic.rs

@@ -2,8 +2,8 @@ use clippy_utils::consts::Constant::{F32, F64, Int};
 use clippy_utils::consts::{ConstEvalCtxt, Constant};
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::{
-    eq_expr_value, get_parent_expr, higher, is_in_const_context, is_inherent_method_call, is_no_std_crate,
-    numeric_literal, peel_blocks, sugg, sym,
+    eq_expr_value, get_parent_expr, has_ambiguous_literal_in_expr, higher, is_in_const_context,
+    is_inherent_method_call, is_no_std_crate, numeric_literal, peel_blocks, sugg, sym,
 };
 use rustc_ast::ast;
 use rustc_errors::Applicability;
@@ -455,7 +455,6 @@ fn is_float_mul_expr<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(&'
     None
 }
 
-// TODO: Fix rust-lang/rust-clippy#4735
 fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
     if let ExprKind::Binary(
         Spanned {
@@ -491,6 +490,14 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
             return;
         };
 
+        // Check if any variable in the expression has an ambiguous type (could be f32 or f64)
+        // see: https://github.com/rust-lang/rust-clippy/issues/14897
+        if (matches!(recv.kind, ExprKind::Path(_)) || matches!(recv.kind, ExprKind::Call(_, _)))
+            && has_ambiguous_literal_in_expr(cx, recv)
+        {
+            return;
+        }
+
         span_lint_and_sugg(
             cx,
             SUBOPTIMAL_FLOPS,

clippy_utils/src/hir_utils.rs

@@ -2,6 +2,7 @@ use crate::consts::ConstEvalCtxt;
 use crate::macros::macro_backtrace;
 use crate::source::{SpanRange, SpanRangeExt, walk_span_to_context};
 use crate::tokenize_with_text;
+use rustc_ast::ast;
 use rustc_ast::ast::InlineAsmTemplatePiece;
 use rustc_data_structures::fx::FxHasher;
 use rustc_hir::MatchSource::TryDesugar;
@@ -9,8 +10,8 @@ use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{
     AssocItemConstraint, BinOpKind, BindingMode, Block, BodyId, Closure, ConstArg, ConstArgKind, Expr, ExprField,
     ExprKind, FnRetTy, GenericArg, GenericArgs, HirId, HirIdMap, InlineAsmOperand, LetExpr, Lifetime, LifetimeKind,
-    Pat, PatExpr, PatExprKind, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, StructTailExpr,
-    TraitBoundModifiers, Ty, TyKind, TyPat, TyPatKind,
+    Node, Pat, PatExpr, PatExprKind, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind,
+    StructTailExpr, TraitBoundModifiers, Ty, TyKind, TyPat, TyPatKind,
 };
 use rustc_lexer::{TokenKind, tokenize};
 use rustc_lint::LateContext;
@@ -1004,8 +1005,8 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
                     self.hash_expr(e);
                 }
             },
-            ExprKind::Match(e, arms, s) => {
-                self.hash_expr(e);
+            ExprKind::Match(scrutinee, arms, _) => {
+                self.hash_expr(scrutinee);
 
                 for arm in *arms {
                     self.hash_pat(arm.pat);
@@ -1014,8 +1015,6 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
                     }
                     self.hash_expr(arm.body);
                 }
-
-                s.hash(&mut self.s);
             },
             ExprKind::MethodCall(path, receiver, args, _fn_span) => {
                 self.hash_name(path.ident.name);
@@ -1058,8 +1057,8 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
             ExprKind::Use(expr, _) => {
                 self.hash_expr(expr);
             },
-            ExprKind::Unary(lop, le) => {
-                std::mem::discriminant(lop).hash(&mut self.s);
+            ExprKind::Unary(l_op, le) => {
+                std::mem::discriminant(l_op).hash(&mut self.s);
                 self.hash_expr(le);
             },
             ExprKind::UnsafeBinderCast(kind, expr, ty) => {
@@ -1394,3 +1393,70 @@ fn eq_span_tokens(
     }
     f(cx, left.into_range(), right.into_range(), pred)
 }
+
+/// Returns true if the expression contains ambiguous literals (unsuffixed float or int literals)
+/// that could be interpreted as either f32/f64 or i32/i64 depending on context.
+pub fn has_ambiguous_literal_in_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    match expr.kind {
+        ExprKind::Path(ref qpath) => {
+            if let Res::Local(hir_id) = cx.qpath_res(qpath, expr.hir_id)
+                && let Node::LetStmt(local) = cx.tcx.parent_hir_node(hir_id)
+                && local.ty.is_none()
+                && let Some(init) = local.init
+            {
+                return has_ambiguous_literal_in_expr(cx, init);
+            }
+            false
+        },
+        ExprKind::Lit(lit) => matches!(
+            lit.node,
+            ast::LitKind::Float(_, ast::LitFloatType::Unsuffixed) | ast::LitKind::Int(_, ast::LitIntType::Unsuffixed)
+        ),
+
+        ExprKind::Array(exprs) | ExprKind::Tup(exprs) => exprs.iter().any(|e| has_ambiguous_literal_in_expr(cx, e)),
+
+        ExprKind::Assign(lhs, rhs, _) | ExprKind::AssignOp(_, lhs, rhs) | ExprKind::Binary(_, lhs, rhs) => {
+            has_ambiguous_literal_in_expr(cx, lhs) || has_ambiguous_literal_in_expr(cx, rhs)
+        },
+
+        ExprKind::Unary(_, e)
+        | ExprKind::Cast(e, _)
+        | ExprKind::Type(e, _)
+        | ExprKind::DropTemps(e)
+        | ExprKind::AddrOf(_, _, e)
+        | ExprKind::Field(e, _)
+        | ExprKind::Index(e, _, _)
+        | ExprKind::Yield(e, _) => has_ambiguous_literal_in_expr(cx, e),
+
+        ExprKind::MethodCall(_, receiver, args, _) | ExprKind::Call(receiver, args) => {
+            has_ambiguous_literal_in_expr(cx, receiver) || args.iter().any(|e| has_ambiguous_literal_in_expr(cx, e))
+        },
+
+        ExprKind::Closure(Closure { body, .. }) => {
+            let body = cx.tcx.hir_body(*body);
+            let closure_expr = crate::peel_blocks(body.value);
+            has_ambiguous_literal_in_expr(cx, closure_expr)
+        },
+
+        ExprKind::Block(blk, _) => blk.expr.as_ref().is_some_and(|e| has_ambiguous_literal_in_expr(cx, e)),
+
+        ExprKind::If(cond, then_expr, else_expr) => {
+            has_ambiguous_literal_in_expr(cx, cond)
+                || has_ambiguous_literal_in_expr(cx, then_expr)
+                || else_expr.as_ref().is_some_and(|e| has_ambiguous_literal_in_expr(cx, e))
+        },
+
+        ExprKind::Match(scrutinee, arms, _) => {
+            has_ambiguous_literal_in_expr(cx, scrutinee)
+                || arms.iter().any(|arm| has_ambiguous_literal_in_expr(cx, arm.body))
+        },
+
+        ExprKind::Loop(body, ..) => body.expr.is_some_and(|e| has_ambiguous_literal_in_expr(cx, e)),
+
+        ExprKind::Ret(opt_expr) | ExprKind::Break(_, opt_expr) => {
+            opt_expr.as_ref().is_some_and(|e| has_ambiguous_literal_in_expr(cx, e))
+        },
+
+        _ => false,
+    }
+}

clippy_utils/src/lib.rs

@@ -77,7 +77,8 @@ pub mod visitors;
 pub use self::attrs::*;
 pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match};
 pub use self::hir_utils::{
-    HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over,
+    HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, has_ambiguous_literal_in_expr, hash_expr,
+    hash_stmt, is_bool, over,
 };
 
 use core::mem;

clippy_utils/src/visitors.rs

@@ -353,7 +353,7 @@ pub fn is_const_evaluatable<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) ->
                 ExprKind::Binary(_, lhs, rhs)
                     if self.cx.typeck_results().expr_ty(lhs).peel_refs().is_primitive_ty()
                         && self.cx.typeck_results().expr_ty(rhs).peel_refs().is_primitive_ty() => {},
-                ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_ref() => (),
+                ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_raw_ptr() => (),
                 ExprKind::Unary(_, e) if self.cx.typeck_results().expr_ty(e).peel_refs().is_primitive_ty() => (),
                 ExprKind::Index(base, _, _)
                     if matches!(
@@ -388,7 +388,8 @@ pub fn is_const_evaluatable<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) ->
                 | ExprKind::Repeat(..)
                 | ExprKind::Struct(..)
                 | ExprKind::Tup(_)
-                | ExprKind::Type(..) => (),
+                | ExprKind::Type(..)
+                | ExprKind::UnsafeBinderCast(..) => (),
 
                 _ => {
                     return ControlFlow::Break(());
@@ -676,10 +677,7 @@ pub fn for_each_unconsumed_temporary<'tcx, B>(
                     helper(typeck, true, else_expr, f)?;
                 }
             },
-            ExprKind::Type(e, _) => {
-                helper(typeck, consume, e, f)?;
-            },
-            ExprKind::UnsafeBinderCast(_, e, _) => {
+            ExprKind::Type(e, _) | ExprKind::UnsafeBinderCast(_, e, _) => {
                 helper(typeck, consume, e, f)?;
             },
 

tests/ui/floating_point_mul_add.fixed

@@ -69,3 +69,47 @@ fn _issue11831() {
 
     let _ = a + b * c;
 }
+
+fn _issue14897() {
+    let x = 1.0;
+    let _ = x * 2.0 + 0.5; // should not suggest mul_add
+    let _ = 0.5 + x * 2.0; // should not suggest mul_add
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+    let _ = 1.2 + x * 1.2; // should not suggest mul_add
+
+    let x = -1.0;
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = { 4.0 };
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = if 1 > 2 { 1.0 } else { 2.0 };
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = 2.4 + 1.2;
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let f = || 4.0;
+    let x = f();
+    let _ = 0.5 + f() * 1.2; // should not suggest mul_add
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = 0.1;
+    let y = x;
+    let z = y;
+    let _ = 0.5 + z * 1.2; // should not suggest mul_add
+
+    let _ = 2.0f64.mul_add(x, 0.5);
+    //~^ suboptimal_flops
+    let _ = 2.0f64.mul_add(x, 0.5);
+    //~^ suboptimal_flops
+
+    let _ = 2.0f64.mul_add(4.0, x);
+    //~^ suboptimal_flops
+
+    let y: f64 = 1.0;
+    let _ = y.mul_add(2.0, 0.5);
+    //~^ suboptimal_flops
+    let _ = 1.0f64.mul_add(2.0, 0.5);
+    //~^ suboptimal_flops
+}

tests/ui/floating_point_mul_add.rs

@@ -69,3 +69,47 @@ fn _issue11831() {
 
     let _ = a + b * c;
 }
+
+fn _issue14897() {
+    let x = 1.0;
+    let _ = x * 2.0 + 0.5; // should not suggest mul_add
+    let _ = 0.5 + x * 2.0; // should not suggest mul_add
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+    let _ = 1.2 + x * 1.2; // should not suggest mul_add
+
+    let x = -1.0;
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = { 4.0 };
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = if 1 > 2 { 1.0 } else { 2.0 };
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = 2.4 + 1.2;
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let f = || 4.0;
+    let x = f();
+    let _ = 0.5 + f() * 1.2; // should not suggest mul_add
+    let _ = 0.5 + x * 1.2; // should not suggest mul_add
+
+    let x = 0.1;
+    let y = x;
+    let z = y;
+    let _ = 0.5 + z * 1.2; // should not suggest mul_add
+
+    let _ = 0.5 + 2.0 * x;
+    //~^ suboptimal_flops
+    let _ = 2.0 * x + 0.5;
+    //~^ suboptimal_flops
+
+    let _ = x + 2.0 * 4.0;
+    //~^ suboptimal_flops
+
+    let y: f64 = 1.0;
+    let _ = y * 2.0 + 0.5;
+    //~^ suboptimal_flops
+    let _ = 1.0 * 2.0 + 0.5;
+    //~^ suboptimal_flops
+}

tests/ui/floating_point_mul_add.stderr

@@ -79,5 +79,35 @@ error: multiply and add expressions can be calculated more efficiently and accur
 LL |     let _ = a - (b * u as f64);
    |             ^^^^^^^^^^^^^^^^^^ help: consider using: `b.mul_add(-(u as f64), a)`
 
-error: aborting due to 13 previous errors
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> tests/ui/floating_point_mul_add.rs:102:13
+   |
+LL |     let _ = 0.5 + 2.0 * x;
+   |             ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(x, 0.5)`
+
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> tests/ui/floating_point_mul_add.rs:104:13
+   |
+LL |     let _ = 2.0 * x + 0.5;
+   |             ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(x, 0.5)`
+
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> tests/ui/floating_point_mul_add.rs:107:13
+   |
+LL |     let _ = x + 2.0 * 4.0;
+   |             ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(4.0, x)`
+
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> tests/ui/floating_point_mul_add.rs:111:13
+   |
+LL |     let _ = y * 2.0 + 0.5;
+   |             ^^^^^^^^^^^^^ help: consider using: `y.mul_add(2.0, 0.5)`
+
+error: multiply and add expressions can be calculated more efficiently and accurately
+  --> tests/ui/floating_point_mul_add.rs:113:13
+   |
+LL |     let _ = 1.0 * 2.0 + 0.5;
+   |             ^^^^^^^^^^^^^^^ help: consider using: `1.0f64.mul_add(2.0, 0.5)`
+
+error: aborting due to 18 previous errors