From 84b8fd54240bd4b903dcc076006e1d35aaaa240d Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Sat, 1 Jul 2023 12:32:38 -0500 Subject: [PATCH 1/3] New lint [`inefficient_pow`] --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/inefficient_pow.rs | 43 ++++++++++++++ clippy_lints/src/methods/mod.rs | 36 ++++++++++++ tests/ui/inefficient_pow.fixed | 54 +++++++++++++++++ tests/ui/inefficient_pow.rs | 54 +++++++++++++++++ tests/ui/inefficient_pow.stderr | 64 +++++++++++++++++++++ 7 files changed, 253 insertions(+) create mode 100644 clippy_lints/src/methods/inefficient_pow.rs create mode 100644 tests/ui/inefficient_pow.fixed create mode 100644 tests/ui/inefficient_pow.rs create mode 100644 tests/ui/inefficient_pow.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index acc9d8ffbf7f..e148feede2a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4901,6 +4901,7 @@ Released 2018-09-13 [`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask +[`inefficient_pow`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_pow [`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string [`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 47995fea9f9e..6e5e003236ca 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -345,6 +345,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::GET_LAST_WITH_LEN_INFO, crate::methods::GET_UNWRAP_INFO, crate::methods::IMPLICIT_CLONE_INFO, + crate::methods::INEFFICIENT_POW_INFO, crate::methods::INEFFICIENT_TO_STRING_INFO, crate::methods::INSPECT_FOR_EACH_INFO, crate::methods::INTO_ITER_ON_REF_INFO, diff --git a/clippy_lints/src/methods/inefficient_pow.rs b/clippy_lints/src/methods/inefficient_pow.rs new file mode 100644 index 000000000000..ac94aa04f463 --- /dev/null +++ b/clippy_lints/src/methods/inefficient_pow.rs @@ -0,0 +1,43 @@ +use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro, source::snippet_opt}; +use rustc_ast::{LitIntType, LitKind}; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; + +use super::INEFFICIENT_POW; + +#[expect( + clippy::cast_possible_truncation, + clippy::cast_precision_loss, + clippy::cast_sign_loss, + clippy::float_cmp +)] +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, recv: &Expr<'_>, arg: &Expr<'_>) { + if let ExprKind::Lit(kind) = recv.kind + && let LitKind::Int(val, suffix) = kind.node + && let power_used = f32::log2(val as f32) + // Precision loss means it's not a power of two + && power_used == (power_used as u32) as f32 + // `0` would be `1.pow()`, which we shouldn't lint (but should be disallowed in a separate lint) + && power_used != 0.0 + && let power_used = power_used as u32 + && !is_from_proc_macro(cx, expr) + && let Some(arg_snippet) = snippet_opt(cx, arg.span) + { + let suffix = match suffix { + LitIntType::Signed(int) => int.name_str(), + LitIntType::Unsigned(int) => int.name_str(), + LitIntType::Unsuffixed => "", + }; + + span_lint_and_sugg( + cx, + INEFFICIENT_POW, + expr.span, + "inefficient `.pow()`", + "use `<<` instead", + format!("1{suffix} << ({arg_snippet} * {power_used})"), + Applicability::MaybeIncorrect, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index be71072acd6c..93204e3c0666 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -32,6 +32,7 @@ mod get_first; mod get_last_with_len; mod get_unwrap; mod implicit_clone; +mod inefficient_pow; mod inefficient_to_string; mod inspect_for_each; mod into_iter_on_ref; @@ -3444,6 +3445,37 @@ declare_clippy_lint! { "`format!`ing every element in a collection, then collecting the strings into a new `String`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for calls to `.pow()` that get the power of two of `n`, power of four, etc. + /// + /// ### Why is this bad? + /// It's not, but it's not optimized down to a simple `<<`, thus, it's slower. This lint is + /// available for when that additional performance is absolutely necessary, at the cost of + /// readability. + /// + /// ### Known issues + /// If the linted `pow` would overflow, the suggested `<<` will give an incorrect value with + /// overflow checks off. In these cases, `pow` will return 0 whilst `<<` will continue on as + /// usual. It may also fail to compile if `arithmetic_overflow` is denied. If this happens, + /// it likely means the original code was incorrect regardless as it would always return `0`. + /// + /// ### Example + /// ```rust,ignore + /// let _ = 2u32.pow(n); + /// let _ = 32u32.pow(n); + /// ``` + /// Use instead: + /// ```rust + /// let _ = 1u32 << n; + /// let _ = 1u32 << (n * 5); + /// ``` + #[clippy::version = "1.72.0"] + pub INEFFICIENT_POW, + restriction, + "usage of `.pow()` when `<<` is faster" +} + declare_clippy_lint! { /// ### What it does /// Checks for usage of `.skip(0)` on iterators. @@ -3602,6 +3634,7 @@ impl_lint_pass!(Methods => [ FORMAT_COLLECT, STRING_LIT_CHARS_ANY, ITER_SKIP_ZERO, + INEFFICIENT_POW, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4008,6 +4041,9 @@ impl Methods { unnecessary_lazy_eval::check(cx, expr, recv, arg, "or"); } }, + ("pow", [arg]) => { + inefficient_pow::check(cx, expr, recv, arg); + } ("push", [arg]) => { path_buf_push_overwrite::check(cx, expr, arg); }, diff --git a/tests/ui/inefficient_pow.fixed b/tests/ui/inefficient_pow.fixed new file mode 100644 index 000000000000..d583d85bae8c --- /dev/null +++ b/tests/ui/inefficient_pow.fixed @@ -0,0 +1,54 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow( + arithmetic_overflow, + clippy::erasing_op, + clippy::identity_op, + clippy::no_effect, + unused +)] +#![warn(clippy::inefficient_pow)] + +#[macro_use] +extern crate proc_macros; + +fn main() { + _ = 1i32 << (1 * 1); + _ = 1i32 << (0 * 1); + _ = 1i32 << (3 * 1); + _ = 1i32 << (100 * 1); + _ = 1i32 << (3 * 2); + _ = 1i32 << (3 * 5); + _ = 1i32 << (3 * 6); + let n = 3; + _ = 1i32 << (n * 6); + // Overflows, so wrong return value, but that's fine. `arithmetic_overflow` will deny this anyway + _ = 1i32 << (3 * 12); + _ = 1i32 << (3 * 16); + // Don't lint + _ = 0i32.pow(3); + _ = 1i32.pow(3); + external! { + _ = 2i32.pow(1); + _ = 2i32.pow(0); + _ = 2i32.pow(3); + _ = 2i32.pow(100); + _ = 4i32.pow(3); + _ = 32i32.pow(3); + _ = 64i32.pow(3); + _ = 4096i32.pow(3); + _ = 65536i32.pow(3); + } + with_span! { + span + _ = 2i32.pow(1); + _ = 2i32.pow(0); + _ = 2i32.pow(3); + _ = 2i32.pow(100); + _ = 4i32.pow(3); + _ = 32i32.pow(3); + _ = 64i32.pow(3); + _ = 4096i32.pow(3); + _ = 65536i32.pow(3); + } +} diff --git a/tests/ui/inefficient_pow.rs b/tests/ui/inefficient_pow.rs new file mode 100644 index 000000000000..bb0af496c5e7 --- /dev/null +++ b/tests/ui/inefficient_pow.rs @@ -0,0 +1,54 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow( + arithmetic_overflow, + clippy::erasing_op, + clippy::identity_op, + clippy::no_effect, + unused +)] +#![warn(clippy::inefficient_pow)] + +#[macro_use] +extern crate proc_macros; + +fn main() { + _ = 2i32.pow(1); + _ = 2i32.pow(0); + _ = 2i32.pow(3); + _ = 2i32.pow(100); + _ = 4i32.pow(3); + _ = 32i32.pow(3); + _ = 64i32.pow(3); + let n = 3; + _ = 64i32.pow(n); + // Overflows, so wrong return value, but that's fine. `arithmetic_overflow` will deny this anyway + _ = 4096i32.pow(3); + _ = 65536i32.pow(3); + // Don't lint + _ = 0i32.pow(3); + _ = 1i32.pow(3); + external! { + _ = 2i32.pow(1); + _ = 2i32.pow(0); + _ = 2i32.pow(3); + _ = 2i32.pow(100); + _ = 4i32.pow(3); + _ = 32i32.pow(3); + _ = 64i32.pow(3); + _ = 4096i32.pow(3); + _ = 65536i32.pow(3); + } + with_span! { + span + _ = 2i32.pow(1); + _ = 2i32.pow(0); + _ = 2i32.pow(3); + _ = 2i32.pow(100); + _ = 4i32.pow(3); + _ = 32i32.pow(3); + _ = 64i32.pow(3); + _ = 4096i32.pow(3); + _ = 65536i32.pow(3); + } +} diff --git a/tests/ui/inefficient_pow.stderr b/tests/ui/inefficient_pow.stderr new file mode 100644 index 000000000000..1e95f29e5241 --- /dev/null +++ b/tests/ui/inefficient_pow.stderr @@ -0,0 +1,64 @@ +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:16:9 + | +LL | _ = 2i32.pow(1); + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (1 * 1)` + | + = note: `-D clippy::inefficient-pow` implied by `-D warnings` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:17:9 + | +LL | _ = 2i32.pow(0); + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (0 * 1)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:18:9 + | +LL | _ = 2i32.pow(3); + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 1)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:19:9 + | +LL | _ = 2i32.pow(100); + | ^^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (100 * 1)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:20:9 + | +LL | _ = 4i32.pow(3); + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 2)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:21:9 + | +LL | _ = 32i32.pow(3); + | ^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 5)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:22:9 + | +LL | _ = 64i32.pow(3); + | ^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 6)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:24:9 + | +LL | _ = 64i32.pow(n); + | ^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (n * 6)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:26:9 + | +LL | _ = 4096i32.pow(3); + | ^^^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 12)` + +error: inefficient `.pow()` + --> $DIR/inefficient_pow.rs:27:9 + | +LL | _ = 65536i32.pow(3); + | ^^^^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 16)` + +error: aborting due to 10 previous errors + From 53b007463c23b0ed02fb2c4f19dba0ab148c0d80 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Sat, 1 Jul 2023 12:42:39 -0500 Subject: [PATCH 2/3] Update mod.rs --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 93204e3c0666..2417b90b56c0 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3466,7 +3466,7 @@ declare_clippy_lint! { /// let _ = 32u32.pow(n); /// ``` /// Use instead: - /// ```rust + /// ```rust,ignore /// let _ = 1u32 << n; /// let _ = 1u32 << (n * 5); /// ``` From a5bc1494a5c88d55f9fe9eb05bc41431f874cee6 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Sat, 1 Jul 2023 17:11:08 -0500 Subject: [PATCH 3/3] Refine suggestion --- clippy_lints/src/methods/inefficient_pow.rs | 12 ++++++++++-- tests/ui/inefficient_pow.fixed | 8 ++++---- tests/ui/inefficient_pow.stderr | 8 ++++---- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/methods/inefficient_pow.rs b/clippy_lints/src/methods/inefficient_pow.rs index ac94aa04f463..a4382123e0bd 100644 --- a/clippy_lints/src/methods/inefficient_pow.rs +++ b/clippy_lints/src/methods/inefficient_pow.rs @@ -1,4 +1,6 @@ -use clippy_utils::{diagnostics::span_lint_and_sugg, is_from_proc_macro, source::snippet_opt}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_from_proc_macro; +use clippy_utils::source::snippet_opt; use rustc_ast::{LitIntType, LitKind}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; @@ -29,6 +31,12 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, recv: LitIntType::Unsigned(int) => int.name_str(), LitIntType::Unsuffixed => "", }; + // `lhs * 1` is useless + let rhs = if power_used == 1 { + arg_snippet.to_string() + } else { + format!("({arg_snippet} * {power_used})") + }; span_lint_and_sugg( cx, @@ -36,7 +44,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, recv: expr.span, "inefficient `.pow()`", "use `<<` instead", - format!("1{suffix} << ({arg_snippet} * {power_used})"), + format!("1{suffix} << {rhs}"), Applicability::MaybeIncorrect, ); } diff --git a/tests/ui/inefficient_pow.fixed b/tests/ui/inefficient_pow.fixed index d583d85bae8c..63f7f2ab93c6 100644 --- a/tests/ui/inefficient_pow.fixed +++ b/tests/ui/inefficient_pow.fixed @@ -13,10 +13,10 @@ extern crate proc_macros; fn main() { - _ = 1i32 << (1 * 1); - _ = 1i32 << (0 * 1); - _ = 1i32 << (3 * 1); - _ = 1i32 << (100 * 1); + _ = 1i32 << 1; + _ = 1i32 << 0; + _ = 1i32 << 3; + _ = 1i32 << 100; _ = 1i32 << (3 * 2); _ = 1i32 << (3 * 5); _ = 1i32 << (3 * 6); diff --git a/tests/ui/inefficient_pow.stderr b/tests/ui/inefficient_pow.stderr index 1e95f29e5241..0e7d194f4746 100644 --- a/tests/ui/inefficient_pow.stderr +++ b/tests/ui/inefficient_pow.stderr @@ -2,7 +2,7 @@ error: inefficient `.pow()` --> $DIR/inefficient_pow.rs:16:9 | LL | _ = 2i32.pow(1); - | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (1 * 1)` + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << 1` | = note: `-D clippy::inefficient-pow` implied by `-D warnings` @@ -10,19 +10,19 @@ error: inefficient `.pow()` --> $DIR/inefficient_pow.rs:17:9 | LL | _ = 2i32.pow(0); - | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (0 * 1)` + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << 0` error: inefficient `.pow()` --> $DIR/inefficient_pow.rs:18:9 | LL | _ = 2i32.pow(3); - | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << (3 * 1)` + | ^^^^^^^^^^^ help: use `<<` instead: `1i32 << 3` error: inefficient `.pow()` --> $DIR/inefficient_pow.rs:19:9 | LL | _ = 2i32.pow(100); - | ^^^^^^^^^^^^^ help: use `<<` instead: `1i32 << (100 * 1)` + | ^^^^^^^^^^^^^ help: use `<<` instead: `1i32 << 100` error: inefficient `.pow()` --> $DIR/inefficient_pow.rs:20:9