constant, Constant,
Constant::{F32, F64},
};
-use crate::utils::{span_lint_and_sugg, sugg};
+use crate::utils::{higher, span_lint_and_sugg, sugg, SpanlessEq};
use if_chain::if_chain;
use rustc::ty;
use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
+use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Lit, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Spanned;
+
+use rustc_ast::ast;
use std::f32::consts as f32_consts;
use std::f64::consts as f64_consts;
use sugg::{format_numeric_literal, Sugg};
-use syntax::ast;
+use syntax::ast::{self, FloatTy, LitFloatType, LitKind};
+
+declare_clippy_lint! {
+ /// **What it does:** Looks for floating-point expressions that
+ /// can be expressed using built-in methods to improve accuracy
+ /// at the cost of performance.
+ ///
+ /// **Why is this bad?** Negatively impacts accuracy.
+ ///
+ /// **Known problems:** None
+ ///
+ /// **Example:**
+ ///
+ /// ```rust
+ ///
+ /// let a = 3f32;
+ /// let _ = a.powf(1.0 / 3.0);
+ /// let _ = (1.0 + a).ln();
+ /// let _ = a.exp() - 1.0;
+ /// ```
+ ///
+ /// is better expressed as
+ ///
+ /// ```rust
+ ///
+ /// let a = 3f32;
+ /// let _ = a.cbrt();
+ /// let _ = a.ln_1p();
+ /// let _ = a.exp_m1();
+ /// ```
+ pub IMPRECISE_FLOPS,
+ nursery,
+ "usage of imprecise floating point operations"
+}
declare_clippy_lint! {
/// **What it does:** Looks for floating-point expressions that
/// let _ = (2f32).powf(a);
/// let _ = E.powf(a);
/// let _ = a.powf(1.0 / 2.0);
- /// let _ = a.powf(1.0 / 3.0);
/// let _ = a.log(2.0);
/// let _ = a.log(10.0);
/// let _ = a.log(E);
- /// let _ = (1.0 + a).ln();
- /// let _ = a.exp() - 1.0;
/// let _ = a.powf(2.0);
/// let _ = a * 2.0 + 4.0;
+ /// let _ = if a < 0.0 {
+ /// -a
+ /// } else {
+ /// a
+ /// }
+ /// let _ = if a < 0.0 {
+ /// a
+ /// } else {
+ /// -a
+ /// }
/// ```
///
/// is better expressed as
/// let _ = a.exp2();
/// let _ = a.exp();
/// let _ = a.sqrt();
- /// let _ = a.cbrt();
/// let _ = a.log2();
/// let _ = a.log10();
/// let _ = a.ln();
- /// let _ = a.ln_1p();
- /// let _ = a.exp_m1();
/// let _ = a.powi(2);
/// let _ = a.mul_add(2.0, 4.0);
+ /// let _ = a.abs();
+ /// let _ = -a.abs();
/// ```
pub SUBOPTIMAL_FLOPS,
nursery,
"usage of sub-optimal floating point operations"
}
-declare_lint_pass!(FloatingPointArithmetic => [SUBOPTIMAL_FLOPS]);
+declare_lint_pass!(FloatingPointArithmetic => [
+ IMPRECISE_FLOPS,
+ SUBOPTIMAL_FLOPS
+]);
// Returns the specialized log method for a given base if base is constant
// and is one of 2, 10 and e
// TODO: Lint expressions of the form `(x + y).ln()` where y > 1 and
// suggest usage of `(x + (y - 1)).ln_1p()` instead
fn check_ln1p(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
- if_chain! {
- if let ExprKind::Binary(op, ref lhs, ref rhs) = &args[0].kind;
- if op.node == BinOpKind::Add;
- then {
- let recv = match (constant(cx, cx.tables, lhs), constant(cx, cx.tables, rhs)) {
- (Some((value, _)), _) if F32(1.0) == value || F64(1.0) == value => rhs,
- (_, Some((value, _))) if F32(1.0) == value || F64(1.0) == value => lhs,
- _ => return,
- };
+ if let ExprKind::Binary(
+ Spanned {
+ node: BinOpKind::Add, ..
+ },
+ lhs,
+ rhs,
+ ) = &args[0].kind
+ {
+ let recv = match (constant(cx, cx.tables, lhs), constant(cx, cx.tables, rhs)) {
+ (Some((value, _)), _) if F32(1.0) == value || F64(1.0) == value => rhs,
+ (_, Some((value, _))) if F32(1.0) == value || F64(1.0) == value => lhs,
+ _ => return,
+ };
- span_lint_and_sugg(
- cx,
- SUBOPTIMAL_FLOPS,
- expr.span,
- "ln(1 + x) can be computed more accurately",
- "consider using",
- format!("{}.ln_1p()", prepare_receiver_sugg(cx, recv)),
- Applicability::MachineApplicable,
- );
- }
+ span_lint_and_sugg(
+ cx,
+ IMPRECISE_FLOPS,
+ expr.span,
+ "ln(1 + x) can be computed more accurately",
+ "consider using",
+ format!("{}.ln_1p()", prepare_receiver_sugg(cx, recv)),
+ Applicability::MachineApplicable,
+ );
}
}
// Check argument
if let Some((value, _)) = constant(cx, cx.tables, &args[1]) {
- let (help, suggestion) = if F32(1.0 / 2.0) == value || F64(1.0 / 2.0) == value {
+ let (lint, help, suggestion) = if F32(1.0 / 2.0) == value || F64(1.0 / 2.0) == value {
(
+ SUBOPTIMAL_FLOPS,
"square-root of a number can be computed more efficiently and accurately",
format!("{}.sqrt()", Sugg::hir(cx, &args[0], "..")),
)
} else if F32(1.0 / 3.0) == value || F64(1.0 / 3.0) == value {
(
+ IMPRECISE_FLOPS,
"cube-root of a number can be computed more accurately",
format!("{}.cbrt()", Sugg::hir(cx, &args[0], "..")),
)
} else if let Some(exponent) = get_integer_from_float_constant(&value) {
(
+ SUBOPTIMAL_FLOPS,
"exponentiation with integer powers can be computed more efficiently",
format!(
"{}.powi({})",
span_lint_and_sugg(
cx,
- SUBOPTIMAL_FLOPS,
+ lint,
expr.span,
help,
"consider using",
// and suggest usage of `x.exp_m1() - (y - 1)` instead
fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if_chain! {
- if let ExprKind::Binary(op, ref lhs, ref rhs) = expr.kind;
- if op.node == BinOpKind::Sub;
+ if let ExprKind::Binary(Spanned { node: BinOpKind::Sub, .. }, ref lhs, ref rhs) = expr.kind;
if cx.tables.expr_ty(lhs).is_floating_point();
if let Some((value, _)) = constant(cx, cx.tables, rhs);
if F32(1.0) == value || F64(1.0) == value;
then {
span_lint_and_sugg(
cx,
- SUBOPTIMAL_FLOPS,
+ IMPRECISE_FLOPS,
expr.span,
"(e.pow(x) - 1) can be computed more accurately",
"consider using",
fn is_float_mul_expr<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option<(&'a Expr<'a>, &'a Expr<'a>)> {
if_chain! {
- if let ExprKind::Binary(op, ref lhs, ref rhs) = &expr.kind;
- if let BinOpKind::Mul = op.node;
+ if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref lhs, ref rhs) = &expr.kind;
if cx.tables.expr_ty(lhs).is_floating_point();
if cx.tables.expr_ty(rhs).is_floating_point();
then {
}
// TODO: Fix rust-lang/rust-clippy#4735
-fn check_fma(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
- if_chain! {
- if let ExprKind::Binary(op, lhs, rhs) = &expr.kind;
- if let BinOpKind::Add = op.node;
- then {
- let (recv, arg1, arg2) = if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, lhs) {
- (inner_lhs, inner_rhs, rhs)
- } else if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, rhs) {
- (inner_lhs, inner_rhs, lhs)
- } else {
- return;
- };
+fn check_mul_add(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+ if let ExprKind::Binary(
+ Spanned {
+ node: BinOpKind::Add, ..
+ },
+ lhs,
+ rhs,
+ ) = &expr.kind
+ {
+ let (recv, arg1, arg2) = if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, lhs) {
+ (inner_lhs, inner_rhs, rhs)
+ } else if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, rhs) {
+ (inner_lhs, inner_rhs, lhs)
+ } else {
+ return;
+ };
- span_lint_and_sugg(
- cx,
- SUBOPTIMAL_FLOPS,
- expr.span,
- "multiply and add expressions can be calculated more efficiently and accurately",
- "consider using",
- format!(
- "{}.mul_add({}, {})",
- prepare_receiver_sugg(cx, recv),
- Sugg::hir(cx, arg1, ".."),
- Sugg::hir(cx, arg2, ".."),
- ),
- Applicability::MachineApplicable,
- );
+ span_lint_and_sugg(
+ cx,
+ SUBOPTIMAL_FLOPS,
+ expr.span,
+ "multiply and add expressions can be calculated more efficiently and accurately",
+ "consider using",
+ format!(
+ "{}.mul_add({}, {})",
+ prepare_receiver_sugg(cx, recv),
+ Sugg::hir(cx, arg1, ".."),
+ Sugg::hir(cx, arg2, ".."),
+ ),
+ Applicability::MachineApplicable,
+ );
+ }
+}
+
+/// Returns true iff expr is an expression which tests whether or not
+/// test is positive or an expression which tests whether or not test
+/// is nonnegative.
+/// Used for check-custom-abs function below
+fn is_testing_positive(cx: &LateContext<'_, '_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
+ if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
+ match op {
+ BinOpKind::Gt | BinOpKind::Ge => is_zero(right) && are_exprs_equal(cx, left, test),
+ BinOpKind::Lt | BinOpKind::Le => is_zero(left) && are_exprs_equal(cx, right, test),
+ _ => false,
+ }
+ } else {
+ false
+ }
+}
+
+fn is_testing_negative(cx: &LateContext<'_, '_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
+ if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
+ match op {
+ BinOpKind::Gt | BinOpKind::Ge => is_zero(left) && are_exprs_equal(cx, right, test),
+ BinOpKind::Lt | BinOpKind::Le => is_zero(right) && are_exprs_equal(cx, left, test),
+ _ => false,
+ }
+ } else {
+ false
+ }
+}
+
+fn are_exprs_equal(cx: &LateContext<'_, '_>, expr1: &Expr<'_>, expr2: &Expr<'_>) -> bool {
+ SpanlessEq::new(cx).ignore_fn().eq_expr(expr1, expr2)
+}
+
+/// Returns true iff expr is some zero literal
+fn is_zero(expr: &Expr<'_>) -> bool {
+ if let ExprKind::Lit(Lit { node: lit, .. }) = &expr.kind {
+ match lit {
+ LitKind::Int(0, _) => true,
+ LitKind::Float(symb, LitFloatType::Unsuffixed)
+ | LitKind::Float(symb, LitFloatType::Suffixed(FloatTy::F64)) => {
+ symb.as_str().parse::<f64>().unwrap() == 0.0
+ },
+ LitKind::Float(symb, LitFloatType::Suffixed(FloatTy::F32)) => symb.as_str().parse::<f32>().unwrap() == 0.0,
+ _ => false,
+ }
+ } else {
+ false
+ }
+}
+
+fn check_custom_abs(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
+ if let Some((cond, body, Some(else_body))) = higher::if_block(&expr) {
+ if let ExprKind::Block(
+ Block {
+ stmts: [],
+ expr:
+ Some(Expr {
+ kind: ExprKind::Unary(UnOp::UnNeg, else_expr),
+ ..
+ }),
+ ..
+ },
+ _,
+ ) = else_body.kind
+ {
+ if let ExprKind::Block(
+ Block {
+ stmts: [],
+ expr: Some(body),
+ ..
+ },
+ _,
+ ) = &body.kind
+ {
+ if are_exprs_equal(cx, else_expr, body) {
+ if is_testing_positive(cx, cond, body) {
+ span_lint_and_sugg(
+ cx,
+ SUBOPTIMAL_FLOPS,
+ expr.span,
+ "This looks like you've implemented your own absolute value function",
+ "try",
+ format!("{}.abs()", Sugg::hir(cx, body, "..")),
+ Applicability::MachineApplicable,
+ );
+ } else if is_testing_negative(cx, cond, body) {
+ span_lint_and_sugg(
+ cx,
+ SUBOPTIMAL_FLOPS,
+ expr.span,
+ "This looks like you've implemented your own negative absolute value function",
+ "try",
+ format!("-{}.abs()", Sugg::hir(cx, body, "..")),
+ Applicability::MachineApplicable,
+ );
+ }
+ }
+ }
+ }
+ if let ExprKind::Block(
+ Block {
+ stmts: [],
+ expr:
+ Some(Expr {
+ kind: ExprKind::Unary(UnOp::UnNeg, else_expr),
+ ..
+ }),
+ ..
+ },
+ _,
+ ) = &body.kind
+ {
+ if let ExprKind::Block(
+ Block {
+ stmts: [],
+ expr: Some(body),
+ ..
+ },
+ _,
+ ) = &else_body.kind
+ {
+ if are_exprs_equal(cx, else_expr, body) {
+ if is_testing_negative(cx, cond, body) {
+ span_lint_and_sugg(
+ cx,
+ SUBOPTIMAL_FLOPS,
+ expr.span,
+ "This looks like you've implemented your own absolute value function",
+ "try",
+ format!("{}.abs()", Sugg::hir(cx, body, "..")),
+ Applicability::MachineApplicable,
+ );
+ } else if is_testing_positive(cx, cond, body) {
+ span_lint_and_sugg(
+ cx,
+ SUBOPTIMAL_FLOPS,
+ expr.span,
+ "This looks like you've implemented your own negative absolute value function",
+ "try",
+ format!("-{}.abs()", Sugg::hir(cx, body, "..")),
+ Applicability::MachineApplicable,
+ );
+ }
+ }
+ }
}
}
}
}
} else {
check_expm1(cx, expr);
- check_fma(cx, expr);
+ check_mul_add(cx, expr);
+ check_custom_abs(cx, expr);
}
}
}
projects / rust.git / blobdiff