Skip to content

Commit

Permalink
size_of_in_element_count: Disable lint on division by byte-size
Browse files Browse the repository at this point in the history
It is fairly common to divide some length in bytes by the byte-size of a
single element before creating a `from_raw_parts` slice or similar
operation. This lint would erroneously disallow such expressions.

Just in case, instead of simply disabling this lint in the RHS of a
division, keep track of the inversion and enable it again on recursive
division.
  • Loading branch information
MarijnS95 committed Jan 19, 2021
1 parent 391bb21 commit d4bf59b
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 6 deletions.
14 changes: 9 additions & 5 deletions clippy_lints/src/size_of_in_element_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ declare_clippy_lint! {

declare_lint_pass!(SizeOfInElementCount => [SIZE_OF_IN_ELEMENT_COUNT]);

fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tcx>> {
fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, inverted: bool) -> Option<Ty<'tcx>> {
match expr.kind {
ExprKind::Call(count_func, _func_args) => {
if_chain! {
if !inverted;
if let ExprKind::Path(ref count_func_qpath) = count_func.kind;
if let Some(def_id) = cx.qpath_res(count_func_qpath, count_func.hir_id).opt_def_id();
if match_def_path(cx, def_id, &paths::MEM_SIZE_OF)
Expand All @@ -50,10 +51,13 @@ fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tc
}
}
},
ExprKind::Binary(op, left, right) if BinOpKind::Mul == op.node || BinOpKind::Div == op.node => {
get_size_of_ty(cx, left).or_else(|| get_size_of_ty(cx, right))
ExprKind::Binary(op, left, right) if BinOpKind::Mul == op.node => {
get_size_of_ty(cx, left, inverted).or_else(|| get_size_of_ty(cx, right, inverted))
},
ExprKind::Cast(expr, _) => get_size_of_ty(cx, expr),
ExprKind::Binary(op, left, right) if BinOpKind::Div == op.node => {
get_size_of_ty(cx, left, inverted).or_else(|| get_size_of_ty(cx, right, !inverted))
},
ExprKind::Cast(expr, _) => get_size_of_ty(cx, expr, inverted),
_ => None,
}
}
Expand Down Expand Up @@ -128,7 +132,7 @@ impl<'tcx> LateLintPass<'tcx> for SizeOfInElementCount {

// Find a size_of call in the count parameter expression and
// check that it's the same type
if let Some(ty_used_for_size_of) = get_size_of_ty(cx, count_expr);
if let Some(ty_used_for_size_of) = get_size_of_ty(cx, count_expr, false);
if TyS::same_type(pointee_ty, ty_used_for_size_of);
then {
span_lint_and_help(
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/size_of_in_element_count/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ fn main() {
// Count expression involving divisions of size_of (Should trigger the lint)
unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };

// Count expression involving divisions by size_of (Should not trigger the lint)
unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / size_of::<u8>()) };

// Count expression involving divisions by multiple size_of (Should not trigger the lint)
unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / (2 * size_of::<u8>())) };

// Count expression involving recursive divisions by size_of (Should trigger the lint)
unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / (2 / size_of::<u8>())) };

// No size_of calls (Should not trigger the lint)
unsafe { copy(x.as_ptr(), y.as_mut_ptr(), SIZE) };

Expand Down
10 changes: 9 additions & 1 deletion tests/ui/size_of_in_element_count/expressions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,13 @@ LL | unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>()
|
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type

error: aborting due to 3 previous errors
error: found a count of bytes instead of a count of elements of `T`
--> $DIR/expressions.rs:30:47
|
LL | unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / (2 / size_of::<u8>())) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type

error: aborting due to 4 previous errors

0 comments on commit d4bf59b

Please sign in to comment.