Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add size_of_ref lint #10098

Merged
merged 1 commit into from
Dec 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4547,6 +4547,7 @@ Released 2018-09-13
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
[`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count
[`size_of_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_ref
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
[`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
[`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
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 @@ -537,6 +537,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::single_char_lifetime_names::SINGLE_CHAR_LIFETIME_NAMES_INFO,
crate::single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS_INFO,
crate::size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT_INFO,
crate::size_of_ref::SIZE_OF_REF_INFO,
crate::slow_vector_initialization::SLOW_VECTOR_INITIALIZATION_INFO,
crate::std_instead_of_core::ALLOC_INSTEAD_OF_CORE_INFO,
crate::std_instead_of_core::STD_INSTEAD_OF_ALLOC_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 @@ -265,6 +265,7 @@ mod shadow;
mod single_char_lifetime_names;
mod single_component_path_imports;
mod size_of_in_element_count;
mod size_of_ref;
mod slow_vector_initialization;
mod std_instead_of_core;
mod strings;
Expand Down Expand Up @@ -906,6 +907,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(semicolon_block::SemicolonBlock));
store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck));
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
// add lints here, do not remove this comment, it's used in `new_lint`
}

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

declare_clippy_lint! {
/// ### What it does
///
/// Checks for calls to `std::mem::size_of_val()` where the argument is
/// a reference to a reference.
///
/// ### Why is this bad?
///
/// Calling `size_of_val()` with a reference to a reference as the argument
/// yields the size of the reference-type, not the size of the value behind
/// the reference.
///
/// ### Example
/// ```rust
/// struct Foo {
/// buffer: [u8],
/// }
///
/// impl Foo {
/// fn size(&self) -> usize {
/// // Note that `&self` as an argument is a `&&Foo`: Because `self`
/// // is already a reference, `&self` is a double-reference.
/// // The return value of `size_of_val()` therefor is the
/// // size of the reference-type, not the size of `self`.
/// std::mem::size_of_val(&self)
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// struct Foo {
/// buffer: [u8],
/// }
///
/// impl Foo {
/// fn size(&self) -> usize {
/// // Correct
/// std::mem::size_of_val(self)
/// }
/// }
/// ```
#[clippy::version = "1.67.0"]
pub SIZE_OF_REF,
suspicious,
"Argument to `std::mem::size_of_val()` is a double-reference, which is almost certainly unintended"
}
declare_lint_pass!(SizeOfRef => [SIZE_OF_REF]);

impl LateLintPass<'_> for SizeOfRef {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
if let ExprKind::Call(path, [arg]) = expr.kind
&& let Some(def_id) = path_def_id(cx, path)
&& cx.tcx.is_diagnostic_item(sym::mem_size_of_val, def_id)
&& let arg_ty = cx.typeck_results().expr_ty(arg)
&& peel_mid_ty_refs(arg_ty).1 > 1
{
span_lint_and_help(
cx,
SIZE_OF_REF,
expr.span,
"argument to `std::mem::size_of_val()` is a reference to a reference",
None,
"dereference the argument to `std::mem::size_of_val()` to get the size of the value instead of the size of the reference-type",
);
}
}
}
27 changes: 27 additions & 0 deletions tests/ui/size_of_ref.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#![allow(unused)]
#![warn(clippy::size_of_ref)]

use std::mem::size_of_val;

fn main() {
let x = 5;
let y = &x;

size_of_val(&x); // no lint
size_of_val(y); // no lint

size_of_val(&&x);
size_of_val(&y);
}

struct S {
field: u32,
data: Vec<u8>,
}

impl S {
/// Get size of object including `self`, in bytes.
pub fn size(&self) -> usize {
std::mem::size_of_val(&self) + (std::mem::size_of::<u8>() * self.data.capacity())
}
}
27 changes: 27 additions & 0 deletions tests/ui/size_of_ref.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error: argument to `std::mem::size_of_val()` is a reference to a reference
--> $DIR/size_of_ref.rs:13:5
|
LL | size_of_val(&&x);
| ^^^^^^^^^^^^^^^^
|
= help: dereference the argument to `std::mem::size_of_val()` to get the size of the value instead of the size of the reference-type
= note: `-D clippy::size-of-ref` implied by `-D warnings`

error: argument to `std::mem::size_of_val()` is a reference to a reference
--> $DIR/size_of_ref.rs:14:5
|
LL | size_of_val(&y);
| ^^^^^^^^^^^^^^^
|
= help: dereference the argument to `std::mem::size_of_val()` to get the size of the value instead of the size of the reference-type

error: argument to `std::mem::size_of_val()` is a reference to a reference
--> $DIR/size_of_ref.rs:25:9
|
LL | std::mem::size_of_val(&self) + (std::mem::size_of::<u8>() * self.data.capacity())
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: dereference the argument to `std::mem::size_of_val()` to get the size of the value instead of the size of the reference-type

error: aborting due to 3 previous errors