Skip to content

Commit 0c0a4d5

Browse files
odysaodysa
committed
fix closure and refactor tooling code
1 parent 1eb2920 commit 0c0a4d5

File tree

7 files changed

+112
-87
lines changed

7 files changed

+112
-87
lines changed

‎clippy_lints/src/floating_point_arithmetic.rs

Lines changed: 6 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@ use clippy_utils::consts::Constant::{F32, F64, Int};
22
use clippy_utils::consts::{ConstEvalCtxt, Constant};
33
use clippy_utils::diagnostics::span_lint_and_sugg;
44
use clippy_utils::{
5-
eq_expr_value, get_parent_expr, higher, is_in_const_context, is_inherent_method_call, is_no_std_crate,
6-
numeric_literal, peel_blocks, sugg, sym,
5+
eq_expr_value, get_parent_expr, has_ambiguous_literal_in_expr, higher, is_in_const_context,
6+
is_inherent_method_call, is_no_std_crate, numeric_literal, peel_blocks, sugg, sym,
77
};
88
use rustc_ast::ast;
99
use rustc_errors::Applicability;
10-
use rustc_hir::def::Res;
11-
use rustc_hir::{BinOpKind, Expr, ExprKind, Node, PathSegment, UnOp};
10+
use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp};
1211
use rustc_lint::{LateContext, LateLintPass};
1312
use rustc_middle::ty;
1413
use rustc_session::declare_lint_pass;
@@ -456,61 +455,6 @@ fn is_float_mul_expr<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(&'
456455
None
457456
}
458457

459-
// Check if any variable in an expression has an ambiguous type (could be f32 or f64)
460-
fn has_ambiguous_float_type(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
461-
match &expr.kind {
462-
ExprKind::Path(qpath) => {
463-
if let Res::Local(hir_id) = cx.qpath_res(qpath, expr.hir_id) {
464-
if let Node::LetStmt(local) = cx.tcx.parent_hir_node(hir_id) {
465-
// If the local has no type annotation, check if the initializer has ambiguous float literals
466-
if local.ty.is_none() {
467-
if let Some(init) = local.init {
468-
return has_ambiguous_float_literal_in_expr(cx, init);
469-
}
470-
}
471-
}
472-
}
473-
false
474-
},
475-
_ => false, // only check path
476-
}
477-
}
478-
479-
// Recursively check if an expression contains any unsuffixed float literals
480-
fn has_ambiguous_float_literal_in_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
481-
match &expr.kind {
482-
ExprKind::Lit(lit) => {
483-
if let ast::LitKind::Float(_, ast::LitFloatType::Unsuffixed) = lit.node {
484-
return true;
485-
}
486-
false
487-
},
488-
ExprKind::Binary(_, lhs, rhs) => {
489-
has_ambiguous_float_literal_in_expr(cx, lhs) || has_ambiguous_float_literal_in_expr(cx, rhs)
490-
},
491-
ExprKind::Unary(_, expr) => has_ambiguous_float_literal_in_expr(cx, expr),
492-
ExprKind::If(_, then, else_) => {
493-
has_ambiguous_float_literal_in_expr(cx, then)
494-
|| else_
495-
.as_ref()
496-
.map_or(false, |else_expr| has_ambiguous_float_literal_in_expr(cx, else_expr))
497-
},
498-
ExprKind::Block(block, _) => block
499-
.expr
500-
.as_ref()
501-
.map_or(false, |expr| has_ambiguous_float_literal_in_expr(cx, expr)),
502-
ExprKind::MethodCall(_, receiver, args, _) => {
503-
has_ambiguous_float_literal_in_expr(cx, receiver)
504-
|| args.iter().any(|arg| has_ambiguous_float_literal_in_expr(cx, arg))
505-
},
506-
ExprKind::Call(func, args) => {
507-
has_ambiguous_float_literal_in_expr(cx, func)
508-
|| args.iter().any(|arg| has_ambiguous_float_literal_in_expr(cx, arg))
509-
},
510-
_ => false,
511-
}
512-
}
513-
514458
fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
515459
if let ExprKind::Binary(
516460
Spanned {
@@ -548,7 +492,9 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) {
548492

549493
// Check if any variable in the expression has an ambiguous type (could be f32 or f64)
550494
// see: https://github.com/rust-lang/rust-clippy/issues/14897
551-
if has_ambiguous_float_type(cx, recv) {
495+
if (matches!(recv.kind, ExprKind::Path(_)) || matches!(recv.kind, ExprKind::Call(_, _)))
496+
&& has_ambiguous_literal_in_expr(cx, recv)
497+
{
552498
return;
553499
}
554500

‎clippy_utils/src/hir_utils.rs

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@ use crate::consts::ConstEvalCtxt;
22
use crate::macros::macro_backtrace;
33
use crate::source::{SpanRange, SpanRangeExt, walk_span_to_context};
44
use crate::tokenize_with_text;
5+
use rustc_ast::ast;
56
use rustc_ast::ast::InlineAsmTemplatePiece;
67
use rustc_data_structures::fx::FxHasher;
78
use rustc_hir::MatchSource::TryDesugar;
89
use rustc_hir::def::{DefKind, Res};
910
use rustc_hir::{
1011
AssocItemConstraint, BinOpKind, BindingMode, Block, BodyId, Closure, ConstArg, ConstArgKind, Expr, ExprField,
1112
ExprKind, FnRetTy, GenericArg, GenericArgs, HirId, HirIdMap, InlineAsmOperand, LetExpr, Lifetime, LifetimeKind,
12-
Pat, PatExpr, PatExprKind, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, StructTailExpr,
13-
TraitBoundModifiers, Ty, TyKind, TyPat, TyPatKind,
13+
Node, Pat, PatExpr, PatExprKind, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind,
14+
StructTailExpr, TraitBoundModifiers, Ty, TyKind, TyPat, TyPatKind,
1415
};
1516
use rustc_lexer::{TokenKind, tokenize};
1617
use rustc_lint::LateContext;
@@ -1004,8 +1005,8 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
10041005
self.hash_expr(e);
10051006
}
10061007
},
1007-
ExprKind::Match(e, arms, s) => {
1008-
self.hash_expr(e);
1008+
ExprKind::Match(scrutinee, arms, _) => {
1009+
self.hash_expr(scrutinee);
10091010

10101011
for arm in *arms {
10111012
self.hash_pat(arm.pat);
@@ -1014,8 +1015,6 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
10141015
}
10151016
self.hash_expr(arm.body);
10161017
}
1017-
1018-
s.hash(&mut self.s);
10191018
},
10201019
ExprKind::MethodCall(path, receiver, args, _fn_span) => {
10211020
self.hash_name(path.ident.name);
@@ -1058,8 +1057,8 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> {
10581057
ExprKind::Use(expr, _) => {
10591058
self.hash_expr(expr);
10601059
},
1061-
ExprKind::Unary(lop, le) => {
1062-
std::mem::discriminant(lop).hash(&mut self.s);
1060+
ExprKind::Unary(l_op, le) => {
1061+
std::mem::discriminant(l_op).hash(&mut self.s);
10631062
self.hash_expr(le);
10641063
},
10651064
ExprKind::UnsafeBinderCast(kind, expr, ty) => {
@@ -1394,3 +1393,70 @@ fn eq_span_tokens(
13941393
}
13951394
f(cx, left.into_range(), right.into_range(), pred)
13961395
}
1396+
1397+
/// Returns true if the expression contains ambiguous literals (unsuffixed float or int literals)
1398+
/// that could be interpreted as either f32/f64 or i32/i64 depending on context.
1399+
pub fn has_ambiguous_literal_in_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
1400+
match expr.kind {
1401+
ExprKind::Path(ref qpath) => {
1402+
if let Res::Local(hir_id) = cx.qpath_res(qpath, expr.hir_id)
1403+
&& let Node::LetStmt(local) = cx.tcx.parent_hir_node(hir_id)
1404+
&& local.ty.is_none()
1405+
&& let Some(init) = local.init
1406+
{
1407+
return has_ambiguous_literal_in_expr(cx, init);
1408+
}
1409+
false
1410+
},
1411+
ExprKind::Lit(lit) => matches!(
1412+
lit.node,
1413+
ast::LitKind::Float(_, ast::LitFloatType::Unsuffixed) | ast::LitKind::Int(_, ast::LitIntType::Unsuffixed)
1414+
),
1415+
1416+
ExprKind::Array(exprs) | ExprKind::Tup(exprs) => exprs.iter().any(|e| has_ambiguous_literal_in_expr(cx, e)),
1417+
1418+
ExprKind::Assign(lhs, rhs, _) | ExprKind::AssignOp(_, lhs, rhs) | ExprKind::Binary(_, lhs, rhs) => {
1419+
has_ambiguous_literal_in_expr(cx, lhs) || has_ambiguous_literal_in_expr(cx, rhs)
1420+
},
1421+
1422+
ExprKind::Unary(_, e)
1423+
| ExprKind::Cast(e, _)
1424+
| ExprKind::Type(e, _)
1425+
| ExprKind::DropTemps(e)
1426+
| ExprKind::AddrOf(_, _, e)
1427+
| ExprKind::Field(e, _)
1428+
| ExprKind::Index(e, _, _)
1429+
| ExprKind::Yield(e, _) => has_ambiguous_literal_in_expr(cx, e),
1430+
1431+
ExprKind::MethodCall(_, receiver, args, _) | ExprKind::Call(receiver, args) => {
1432+
has_ambiguous_literal_in_expr(cx, receiver) || args.iter().any(|e| has_ambiguous_literal_in_expr(cx, e))
1433+
},
1434+
1435+
ExprKind::Closure(Closure { body, .. }) => {
1436+
let body = cx.tcx.hir_body(*body);
1437+
let closure_expr = crate::peel_blocks(body.value);
1438+
has_ambiguous_literal_in_expr(cx, closure_expr)
1439+
},
1440+
1441+
ExprKind::Block(blk, _) => blk.expr.as_ref().is_some_and(|e| has_ambiguous_literal_in_expr(cx, e)),
1442+
1443+
ExprKind::If(cond, then_expr, else_expr) => {
1444+
has_ambiguous_literal_in_expr(cx, cond)
1445+
|| has_ambiguous_literal_in_expr(cx, then_expr)
1446+
|| else_expr.as_ref().is_some_and(|e| has_ambiguous_literal_in_expr(cx, e))
1447+
},
1448+
1449+
ExprKind::Match(scrutinee, arms, _) => {
1450+
has_ambiguous_literal_in_expr(cx, scrutinee)
1451+
|| arms.iter().any(|arm| has_ambiguous_literal_in_expr(cx, arm.body))
1452+
},
1453+
1454+
ExprKind::Loop(body, ..) => body.expr.is_some_and(|e| has_ambiguous_literal_in_expr(cx, e)),
1455+
1456+
ExprKind::Ret(opt_expr) | ExprKind::Break(_, opt_expr) => {
1457+
opt_expr.as_ref().is_some_and(|e| has_ambiguous_literal_in_expr(cx, e))
1458+
},
1459+
1460+
_ => false,
1461+
}
1462+
}

‎clippy_utils/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ pub mod visitors;
7777
pub use self::attrs::*;
7878
pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match};
7979
pub use self::hir_utils::{
80-
HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over,
80+
HirEqInterExpr, SpanlessEq, SpanlessHash, both, count_eq, eq_expr_value, has_ambiguous_literal_in_expr, hash_expr,
81+
hash_stmt, is_bool, over,
8182
};
8283

8384
use core::mem;

‎clippy_utils/src/visitors.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ pub fn is_const_evaluatable<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) ->
353353
ExprKind::Binary(_, lhs, rhs)
354354
if self.cx.typeck_results().expr_ty(lhs).peel_refs().is_primitive_ty()
355355
&& self.cx.typeck_results().expr_ty(rhs).peel_refs().is_primitive_ty() => {},
356-
ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_ref() => (),
356+
ExprKind::Unary(UnOp::Deref, e) if self.cx.typeck_results().expr_ty(e).is_raw_ptr() => (),
357357
ExprKind::Unary(_, e) if self.cx.typeck_results().expr_ty(e).peel_refs().is_primitive_ty() => (),
358358
ExprKind::Index(base, _, _)
359359
if matches!(
@@ -388,7 +388,8 @@ pub fn is_const_evaluatable<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) ->
388388
| ExprKind::Repeat(..)
389389
| ExprKind::Struct(..)
390390
| ExprKind::Tup(_)
391-
| ExprKind::Type(..) => (),
391+
| ExprKind::Type(..)
392+
| ExprKind::UnsafeBinderCast(..) => (),
392393

393394
_ => {
394395
return ControlFlow::Break(());
@@ -676,10 +677,7 @@ pub fn for_each_unconsumed_temporary<'tcx, B>(
676677
helper(typeck, true, else_expr, f)?;
677678
}
678679
},
679-
ExprKind::Type(e, _) => {
680-
helper(typeck, consume, e, f)?;
681-
},
682-
ExprKind::UnsafeBinderCast(_, e, _) => {
680+
ExprKind::Type(e, _) | ExprKind::UnsafeBinderCast(_, e, _) => {
683681
helper(typeck, consume, e, f)?;
684682
},
685683

‎tests/ui/floating_point_mul_add.fixed

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ fn _issue14897() {
8989
let x = 2.4 + 1.2;
9090
let _ = 0.5 + x * 1.2; // should not suggest mul_add
9191

92+
let f = || 4.0;
93+
let x = f();
94+
let _ = 0.5 + f() * 1.2; // should not suggest mul_add
95+
let _ = 0.5 + x * 1.2; // should not suggest mul_add
96+
97+
let x = 0.1;
98+
let y = x;
99+
let z = y;
100+
let _ = 0.5 + z * 1.2; // should not suggest mul_add
101+
92102
let _ = 2.0f64.mul_add(x, 0.5);
93103
//~^ suboptimal_flops
94104
let _ = 2.0f64.mul_add(x, 0.5);

‎tests/ui/floating_point_mul_add.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,16 @@ fn _issue14897() {
8989
let x = 2.4 + 1.2;
9090
let _ = 0.5 + x * 1.2; // should not suggest mul_add
9191

92+
let f = || 4.0;
93+
let x = f();
94+
let _ = 0.5 + f() * 1.2; // should not suggest mul_add
95+
let _ = 0.5 + x * 1.2; // should not suggest mul_add
96+
97+
let x = 0.1;
98+
let y = x;
99+
let z = y;
100+
let _ = 0.5 + z * 1.2; // should not suggest mul_add
101+
92102
let _ = 0.5 + 2.0 * x;
93103
//~^ suboptimal_flops
94104
let _ = 2.0 * x + 0.5;

‎tests/ui/floating_point_mul_add.stderr

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,40 +80,34 @@ LL | let _ = a - (b * u as f64);
8080
| ^^^^^^^^^^^^^^^^^^ help: consider using: `b.mul_add(-(u as f64), a)`
8181

8282
error: multiply and add expressions can be calculated more efficiently and accurately
83-
--> tests/ui/floating_point_mul_add.rs:85:13
84-
|
85-
LL | let _ = 0.5 + x * 1.2; // should not suggest mul_add
86-
| ^^^^^^^^^^^^^ help: consider using: `x.mul_add(1.2, 0.5)`
87-
88-
error: multiply and add expressions can be calculated more efficiently and accurately
89-
--> tests/ui/floating_point_mul_add.rs:96:13
83+
--> tests/ui/floating_point_mul_add.rs:102:13
9084
|
9185
LL | let _ = 0.5 + 2.0 * x;
9286
| ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(x, 0.5)`
9387

9488
error: multiply and add expressions can be calculated more efficiently and accurately
95-
--> tests/ui/floating_point_mul_add.rs:98:13
89+
--> tests/ui/floating_point_mul_add.rs:104:13
9690
|
9791
LL | let _ = 2.0 * x + 0.5;
9892
| ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(x, 0.5)`
9993

10094
error: multiply and add expressions can be calculated more efficiently and accurately
101-
--> tests/ui/floating_point_mul_add.rs:101:13
95+
--> tests/ui/floating_point_mul_add.rs:107:13
10296
|
10397
LL | let _ = x + 2.0 * 4.0;
10498
| ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(4.0, x)`
10599

106100
error: multiply and add expressions can be calculated more efficiently and accurately
107-
--> tests/ui/floating_point_mul_add.rs:105:13
101+
--> tests/ui/floating_point_mul_add.rs:111:13
108102
|
109103
LL | let _ = y * 2.0 + 0.5;
110104
| ^^^^^^^^^^^^^ help: consider using: `y.mul_add(2.0, 0.5)`
111105

112106
error: multiply and add expressions can be calculated more efficiently and accurately
113-
--> tests/ui/floating_point_mul_add.rs:107:13
107+
--> tests/ui/floating_point_mul_add.rs:113:13
114108
|
115109
LL | let _ = 1.0 * 2.0 + 0.5;
116110
| ^^^^^^^^^^^^^^^ help: consider using: `1.0f64.mul_add(2.0, 0.5)`
117111

118-
error: aborting due to 19 previous errors
112+
error: aborting due to 18 previous errors
119113

0 commit comments

Comments
 (0)