Skip to content

Commit

Permalink
Add [manual_slice_size_calculation]
Browse files Browse the repository at this point in the history
  • Loading branch information
schubart committed Apr 6, 2023
1 parent 26e245d commit b66aa09
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4674,6 +4674,7 @@ Released 2018-09-13
[`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
[`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
[`manual_slice_size_calculation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_size_calculation
[`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
[`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat
[`manual_string_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_string_new
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
crate::manual_retain::MANUAL_RETAIN_INFO,
crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
crate::manual_string_new::MANUAL_STRING_NEW_INFO,
crate::manual_strip::MANUAL_STRIP_INFO,
crate::map_unit_fn::OPTION_MAP_UNIT_FN_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ mod manual_main_separator_str;
mod manual_non_exhaustive;
mod manual_rem_euclid;
mod manual_retain;
mod manual_slice_size_calculation;
mod manual_string_new;
mod manual_strip;
mod map_unit_fn;
Expand Down Expand Up @@ -957,6 +958,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
});
store.register_late_pass(|_| Box::new(lines_filter_map_ok::LinesFilterMapOk));
store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule));
store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
90 changes: 90 additions & 0 deletions clippy_lints/src/manual_slice_size_calculation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::in_constant;
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::sym;

declare_clippy_lint! {
/// ### What it does
/// When `a` is `&[T]`, detect `a.len() * size_of::<T>()` and suggest `size_of_val(a)`
/// instead.
///
/// ### Why is this better?
/// * Shorter to write
/// * Removes the need for the human and the compiler to worry about overflow in the
/// multiplication
/// * Potentially faster at runtime as rust emits special no-wrapping flags when it
/// calculates the byte length
/// * Less turbofishing
///
/// ### Example
/// ```rust
/// # let data : &[i32] = &[1, 2, 3];
/// let newlen = data.len() * std::mem::size_of::<i32>();
/// ```
/// Use instead:
/// ```rust
/// # let data : &[i32] = &[1, 2, 3];
/// let newlen = std::mem::size_of_val(data);
/// ```
#[clippy::version = "1.70.0"]
pub MANUAL_SLICE_SIZE_CALCULATION,
complexity,
"manual slice size calculation"
}
declare_lint_pass!(ManualSliceSizeCalculation => [MANUAL_SLICE_SIZE_CALCULATION]);

impl<'tcx> LateLintPass<'tcx> for ManualSliceSizeCalculation {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
// Does not apply inside const because size_of_value is not cost in stable.
if !in_constant(cx, expr.hir_id)
&& let ExprKind::Binary(ref op, left, right) = expr.kind
&& BinOpKind::Mul == op.node
&& let Some(_receiver) = simplify(cx, left, right)
{
span_lint_and_help(
cx,
MANUAL_SLICE_SIZE_CALCULATION,
expr.span,
"manual slice size calculation",
None,
"consider using std::mem::size_of_value instead");
}
}
}

fn simplify<'tcx>(
cx: &LateContext<'tcx>,
expr1: &'tcx Expr<'tcx>,
expr2: &'tcx Expr<'tcx>,
) -> Option<&'tcx Expr<'tcx>> {
simplify_half(cx, expr1, expr2).or_else(|| simplify_half(cx, expr2, expr1))
}

fn simplify_half<'tcx>(
cx: &LateContext<'tcx>,
expr1: &'tcx Expr<'tcx>,
expr2: &'tcx Expr<'tcx>,
) -> Option<&'tcx Expr<'tcx>> {
if
// expr1 is `[T1].len()`?
let ExprKind::MethodCall(method_path, receiver, _, _) = expr1.kind
&& method_path.ident.name == sym::len
&& let receiver_ty = cx.typeck_results().expr_ty(receiver)
&& let ty::Slice(ty1) = receiver_ty.peel_refs().kind()
// expr2 is `size_of::<T2>()`?
&& let ExprKind::Call(func, _) = expr2.kind
&& let ExprKind::Path(ref func_qpath) = func.kind
&& let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::mem_size_of, def_id)
&& let Some(ty2) = cx.typeck_results().node_substs(func.hir_id).types().next()
// T1 == T2?
&& *ty1 == ty2
{
Some(receiver)
} else {
None
}
}
30 changes: 30 additions & 0 deletions tests/ui/manual_slice_size_calculation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![allow(unused)]
#![warn(clippy::manual_slice_size_calculation)]

use core::mem::{align_of, size_of};

fn main() {
let v_i32 = Vec::<i32>::new();
let s_i32 = v_i32.as_slice();

// True positives:
let _ = s_i32.len() * size_of::<i32>(); // WARNING
let _ = size_of::<i32>() * s_i32.len(); // WARNING
let _ = size_of::<i32>() * s_i32.len() * 5; // WARNING

// True negatives:
let _ = size_of::<i32>() + s_i32.len(); // Ok, not a multiplication
let _ = size_of::<i32>() * s_i32.partition_point(|_| true); // Ok, not len()
let _ = size_of::<i32>() * v_i32.len(); // Ok, not a slice
let _ = align_of::<i32>() * s_i32.len(); // Ok, not size_of()
let _ = size_of::<u32>() * s_i32.len(); // Ok, different types

// False negatives:
let _ = 5 * size_of::<i32>() * s_i32.len(); // Ok (MISSED OPPORTUNITY)
let _ = size_of::<i32>() * 5 * s_i32.len(); // Ok (MISSED OPPORTUNITY)
}

const fn _const(s_i32: &[i32]) {
// True negative:
let _ = s_i32.len() * size_of::<i32>(); // Ok, can't use size_of_val in const
}
27 changes: 27 additions & 0 deletions tests/ui/manual_slice_size_calculation.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error: manual slice size calculation
--> $DIR/manual_slice_size_calculation.rs:11:13
|
LL | let _ = s_i32.len() * size_of::<i32>(); // WARNING
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using std::mem::size_of_value instead
= note: `-D clippy::manual-slice-size-calculation` implied by `-D warnings`

error: manual slice size calculation
--> $DIR/manual_slice_size_calculation.rs:12:13
|
LL | let _ = size_of::<i32>() * s_i32.len(); // WARNING
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using std::mem::size_of_value instead

error: manual slice size calculation
--> $DIR/manual_slice_size_calculation.rs:13:13
|
LL | let _ = size_of::<i32>() * s_i32.len() * 5; // WARNING
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using std::mem::size_of_value instead

error: aborting due to 3 previous errors

0 comments on commit b66aa09

Please sign in to comment.