Auto merge of #8630 - Jarcho:forget_non_drop, r=Manishearth

Add lints `drop_non_drop` and `forget_non_drop`

fixes #1897

changelog: Add lints `drop_non_drop` and `forget_non_drop`
This commit is contained in:
bors 2022-04-06 23:04:20 +00:00
commit 0d66404941
17 changed files with 269 additions and 128 deletions

View File

@ -3256,6 +3256,7 @@ Released 2018-09-13
[`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg
[`double_parens`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_parens
[`drop_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy
[`drop_non_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_non_drop
[`drop_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref
[`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument
[`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
@ -3308,6 +3309,7 @@ Released 2018-09-13
[`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map
[`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
[`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
[`forget_non_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_non_drop
[`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
[`format_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#format_in_format_args
[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect

View File

@ -1,9 +1,8 @@
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::ty::is_copy;
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind};
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
use clippy_utils::is_must_use_func_call;
use clippy_utils::ty::{is_copy, is_must_use_ty, is_type_lang_item};
use rustc_hir::{Expr, ExprKind, LangItem};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
@ -103,6 +102,75 @@ declare_clippy_lint! {
"calls to `std::mem::forget` with a value that implements Copy"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for calls to `std::mem::drop` with a value that does not implement `Drop`.
///
/// ### Why is this bad?
/// Calling `std::mem::drop` is no different than dropping such a type. A different value may
/// have been intended.
///
/// ### Example
/// ```rust
/// struct Foo;
/// let x = Foo;
/// std::mem::drop(x);
/// ```
#[clippy::version = "1.61.0"]
pub DROP_NON_DROP,
suspicious,
"call to `std::mem::drop` with a value which does not implement `Drop`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for calls to `std::mem::forget` with a value that does not implement `Drop`.
///
/// ### Why is this bad?
/// Calling `std::mem::forget` is no different than dropping such a type. A different value may
/// have been intended.
///
/// ### Example
/// ```rust
/// struct Foo;
/// let x = Foo;
/// std::mem::forget(x);
/// ```
#[clippy::version = "1.61.0"]
pub FORGET_NON_DROP,
suspicious,
"call to `std::mem::forget` with a value which does not implement `Drop`"
}
declare_clippy_lint! {
/// ### What it does
/// Prevents the safe `std::mem::drop` function from being called on `std::mem::ManuallyDrop`.
///
/// ### Why is this bad?
/// The safe `drop` function does not drop the inner value of a `ManuallyDrop`.
///
/// ### Known problems
/// Does not catch cases if the user binds `std::mem::drop`
/// to a different name and calls it that way.
///
/// ### Example
/// ```rust
/// struct S;
/// drop(std::mem::ManuallyDrop::new(S));
/// ```
/// Use instead:
/// ```rust
/// struct S;
/// unsafe {
/// std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S));
/// }
/// ```
#[clippy::version = "1.49.0"]
pub UNDROPPED_MANUALLY_DROPS,
correctness,
"use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value"
}
const DROP_REF_SUMMARY: &str = "calls to `std::mem::drop` with a reference instead of an owned value. \
Dropping a reference does nothing";
const FORGET_REF_SUMMARY: &str = "calls to `std::mem::forget` with a reference instead of an owned value. \
@ -111,60 +179,65 @@ const DROP_COPY_SUMMARY: &str = "calls to `std::mem::drop` with a value that imp
Dropping a copy leaves the original intact";
const FORGET_COPY_SUMMARY: &str = "calls to `std::mem::forget` with a value that implements `Copy`. \
Forgetting a copy leaves the original intact";
const DROP_NON_DROP_SUMMARY: &str = "call to `std::mem::drop` with a value that does not implement `Drop`. \
Dropping such a type only extends it's contained lifetimes";
const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \
Forgetting such a type is the same as dropping it";
declare_lint_pass!(DropForgetRef => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COPY]);
declare_lint_pass!(DropForgetRef => [
DROP_REF,
FORGET_REF,
DROP_COPY,
FORGET_COPY,
DROP_NON_DROP,
FORGET_NON_DROP,
UNDROPPED_MANUALLY_DROPS
]);
impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Call(path, args) = expr.kind;
if let ExprKind::Path(ref qpath) = path.kind;
if args.len() == 1;
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id();
then {
let lint;
let msg;
let arg = &args[0];
let arg_ty = cx.typeck_results().expr_ty(arg);
if let ty::Ref(..) = arg_ty.kind() {
match cx.tcx.get_diagnostic_name(def_id) {
Some(sym::mem_drop) => {
lint = DROP_REF;
msg = DROP_REF_SUMMARY.to_string();
},
Some(sym::mem_forget) => {
lint = FORGET_REF;
msg = FORGET_REF_SUMMARY.to_string();
},
_ => return,
}
span_lint_and_note(cx,
lint,
expr.span,
&msg,
Some(arg.span),
&format!("argument has type `{}`", arg_ty));
} else if is_copy(cx, arg_ty) {
match cx.tcx.get_diagnostic_name(def_id) {
Some(sym::mem_drop) => {
lint = DROP_COPY;
msg = DROP_COPY_SUMMARY.to_string();
},
Some(sym::mem_forget) => {
lint = FORGET_COPY;
msg = FORGET_COPY_SUMMARY.to_string();
},
_ => return,
}
span_lint_and_note(cx,
lint,
expr.span,
&msg,
Some(arg.span),
&format!("argument has type {}", arg_ty));
if let ExprKind::Call(path, [arg]) = expr.kind
&& let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
&& let Some(fn_name) = cx.tcx.get_diagnostic_name(def_id)
{
let arg_ty = cx.typeck_results().expr_ty(arg);
let (lint, msg) = match fn_name {
sym::mem_drop if arg_ty.is_ref() => (DROP_REF, DROP_REF_SUMMARY),
sym::mem_forget if arg_ty.is_ref() => (FORGET_REF, FORGET_REF_SUMMARY),
sym::mem_drop if is_copy(cx, arg_ty) => (DROP_COPY, DROP_COPY_SUMMARY),
sym::mem_forget if is_copy(cx, arg_ty) => (FORGET_COPY, FORGET_COPY_SUMMARY),
sym::mem_drop if is_type_lang_item(cx, arg_ty, LangItem::ManuallyDrop) => {
span_lint_and_help(
cx,
UNDROPPED_MANUALLY_DROPS,
expr.span,
"the inner value of this ManuallyDrop will not be dropped",
None,
"to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop",
);
return;
}
}
sym::mem_drop
if !(arg_ty.needs_drop(cx.tcx, cx.param_env)
|| is_must_use_func_call(cx, arg)
|| is_must_use_ty(cx, arg_ty)) =>
{
(DROP_NON_DROP, DROP_NON_DROP_SUMMARY)
},
sym::mem_forget if !arg_ty.needs_drop(cx.tcx, cx.param_env) => {
(FORGET_NON_DROP, FORGET_NON_DROP_SUMMARY)
},
_ => return,
};
span_lint_and_note(
cx,
lint,
expr.span,
msg,
Some(arg.span),
&format!("argument has type `{}`", arg_ty),
);
}
}
}

View File

@ -50,9 +50,12 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(double_comparison::DOUBLE_COMPARISONS),
LintId::of(double_parens::DOUBLE_PARENS),
LintId::of(drop_forget_ref::DROP_COPY),
LintId::of(drop_forget_ref::DROP_NON_DROP),
LintId::of(drop_forget_ref::DROP_REF),
LintId::of(drop_forget_ref::FORGET_COPY),
LintId::of(drop_forget_ref::FORGET_NON_DROP),
LintId::of(drop_forget_ref::FORGET_REF),
LintId::of(drop_forget_ref::UNDROPPED_MANUALLY_DROPS),
LintId::of(duration_subsec::DURATION_SUBSEC),
LintId::of(entry::MAP_ENTRY),
LintId::of(enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
@ -297,7 +300,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(types::REDUNDANT_ALLOCATION),
LintId::of(types::TYPE_COMPLEXITY),
LintId::of(types::VEC_BOX),
LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
LintId::of(unicode::INVISIBLE_CHARACTERS),
LintId::of(uninit_vec::UNINIT_VEC),
LintId::of(unit_hash::UNIT_HASH),

View File

@ -22,6 +22,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
LintId::of(drop_forget_ref::DROP_REF),
LintId::of(drop_forget_ref::FORGET_COPY),
LintId::of(drop_forget_ref::FORGET_REF),
LintId::of(drop_forget_ref::UNDROPPED_MANUALLY_DROPS),
LintId::of(enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
LintId::of(eq_op::EQ_OP),
LintId::of(erasing_op::ERASING_OP),
@ -62,7 +63,6 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE),
LintId::of(transmute::WRONG_TRANSMUTE),
LintId::of(transmuting_null::TRANSMUTING_NULL),
LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
LintId::of(unicode::INVISIBLE_CHARACTERS),
LintId::of(uninit_vec::UNINIT_VEC),
LintId::of(unit_hash::UNIT_HASH),

View File

@ -123,9 +123,12 @@ store.register_lints(&[
double_comparison::DOUBLE_COMPARISONS,
double_parens::DOUBLE_PARENS,
drop_forget_ref::DROP_COPY,
drop_forget_ref::DROP_NON_DROP,
drop_forget_ref::DROP_REF,
drop_forget_ref::FORGET_COPY,
drop_forget_ref::FORGET_NON_DROP,
drop_forget_ref::FORGET_REF,
drop_forget_ref::UNDROPPED_MANUALLY_DROPS,
duration_subsec::DURATION_SUBSEC,
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
empty_enum::EMPTY_ENUM,
@ -507,7 +510,6 @@ store.register_lints(&[
types::TYPE_COMPLEXITY,
types::VEC_BOX,
undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS,
undropped_manually_drops::UNDROPPED_MANUALLY_DROPS,
unicode::INVISIBLE_CHARACTERS,
unicode::NON_ASCII_LITERAL,
unicode::UNICODE_NOT_NFC,

View File

@ -10,6 +10,8 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
LintId::of(casts::CAST_ENUM_CONSTRUCTOR),
LintId::of(casts::CAST_ENUM_TRUNCATION),
LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF),
LintId::of(drop_forget_ref::DROP_NON_DROP),
LintId::of(drop_forget_ref::FORGET_NON_DROP),
LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE),
LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
LintId::of(format_impl::PRINT_IN_FORMAT_IMPL),

View File

@ -378,7 +378,6 @@ mod transmuting_null;
mod try_err;
mod types;
mod undocumented_unsafe_blocks;
mod undropped_manually_drops;
mod unicode;
mod uninit_vec;
mod unit_hash;
@ -815,7 +814,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(disallowed_methods::DisallowedMethods::new(disallowed_methods.clone())));
store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86AttSyntax));
store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax));
store.register_late_pass(|| Box::new(undropped_manually_drops::UndroppedManuallyDrops));
store.register_late_pass(|| Box::new(strings::StrToString));
store.register_late_pass(|| Box::new(strings::StringToString));
store.register_late_pass(|| Box::new(zero_sized_map_values::ZeroSizedMapValues));

View File

@ -1,59 +0,0 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::path_res;
use clippy_utils::ty::is_type_lang_item;
use rustc_hir::{lang_items, 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
/// Prevents the safe `std::mem::drop` function from being called on `std::mem::ManuallyDrop`.
///
/// ### Why is this bad?
/// The safe `drop` function does not drop the inner value of a `ManuallyDrop`.
///
/// ### Known problems
/// Does not catch cases if the user binds `std::mem::drop`
/// to a different name and calls it that way.
///
/// ### Example
/// ```rust
/// struct S;
/// drop(std::mem::ManuallyDrop::new(S));
/// ```
/// Use instead:
/// ```rust
/// struct S;
/// unsafe {
/// std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S));
/// }
/// ```
#[clippy::version = "1.49.0"]
pub UNDROPPED_MANUALLY_DROPS,
correctness,
"use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value"
}
declare_lint_pass!(UndroppedManuallyDrops => [UNDROPPED_MANUALLY_DROPS]);
impl<'tcx> LateLintPass<'tcx> for UndroppedManuallyDrops {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Call(fun, [arg_0, ..]) = expr.kind;
if path_res(cx, fun).opt_def_id() == cx.tcx.get_diagnostic_item(sym::mem_drop);
let ty = cx.typeck_results().expr_ty(arg_0);
if is_type_lang_item(cx, ty, lang_items::LangItem::ManuallyDrop);
then {
span_lint_and_help(
cx,
UNDROPPED_MANUALLY_DROPS,
expr.span,
"the inner value of this ManuallyDrop will not be dropped",
None,
"to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop",
);
}
}
}
}

View File

@ -5,7 +5,7 @@ LL | drop(s1);
| ^^^^^^^^
|
= note: `-D clippy::drop-copy` implied by `-D warnings`
note: argument has type SomeStruct
note: argument has type `SomeStruct`
--> $DIR/drop_forget_copy.rs:33:10
|
LL | drop(s1);
@ -17,7 +17,7 @@ error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a
LL | drop(s2);
| ^^^^^^^^
|
note: argument has type SomeStruct
note: argument has type `SomeStruct`
--> $DIR/drop_forget_copy.rs:34:10
|
LL | drop(s2);
@ -29,7 +29,7 @@ error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a
LL | drop(s4);
| ^^^^^^^^
|
note: argument has type SomeStruct
note: argument has type `SomeStruct`
--> $DIR/drop_forget_copy.rs:36:10
|
LL | drop(s4);
@ -42,7 +42,7 @@ LL | forget(s1);
| ^^^^^^^^^^
|
= note: `-D clippy::forget-copy` implied by `-D warnings`
note: argument has type SomeStruct
note: argument has type `SomeStruct`
--> $DIR/drop_forget_copy.rs:39:12
|
LL | forget(s1);
@ -54,7 +54,7 @@ error: calls to `std::mem::forget` with a value that implements `Copy`. Forgetti
LL | forget(s2);
| ^^^^^^^^^^
|
note: argument has type SomeStruct
note: argument has type `SomeStruct`
--> $DIR/drop_forget_copy.rs:40:12
|
LL | forget(s2);
@ -66,7 +66,7 @@ error: calls to `std::mem::forget` with a value that implements `Copy`. Forgetti
LL | forget(s4);
| ^^^^^^^^^^
|
note: argument has type SomeStruct
note: argument has type `SomeStruct`
--> $DIR/drop_forget_copy.rs:42:12
|
LL | forget(s4);

40
tests/ui/drop_non_drop.rs Normal file
View File

@ -0,0 +1,40 @@
#![warn(clippy::drop_non_drop)]
use core::mem::drop;
fn make_result<T>(t: T) -> Result<T, ()> {
Ok(t)
}
#[must_use]
fn must_use<T>(t: T) -> T {
t
}
fn drop_generic<T>(t: T) {
// Don't lint
drop(t)
}
fn main() {
struct Foo;
// Lint
drop(Foo);
// Don't lint
drop(make_result(Foo));
// Don't lint
drop(must_use(Foo));
struct Bar;
impl Drop for Bar {
fn drop(&mut self) {}
}
// Don't lint
drop(Bar);
struct Baz<T>(T);
// Lint
drop(Baz(Foo));
// Don't lint
drop(Baz(Bar));
}

View File

@ -0,0 +1,27 @@
error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends it's contained lifetimes
--> $DIR/drop_non_drop.rs:22:5
|
LL | drop(Foo);
| ^^^^^^^^^
|
= note: `-D clippy::drop-non-drop` implied by `-D warnings`
note: argument has type `main::Foo`
--> $DIR/drop_non_drop.rs:22:10
|
LL | drop(Foo);
| ^^^
error: call to `std::mem::drop` with a value that does not implement `Drop`. Dropping such a type only extends it's contained lifetimes
--> $DIR/drop_non_drop.rs:37:5
|
LL | drop(Baz(Foo));
| ^^^^^^^^^^^^^^
|
note: argument has type `main::Baz<main::Foo>`
--> $DIR/drop_non_drop.rs:37:10
|
LL | drop(Baz(Foo));
| ^^^^^^^^
error: aborting due to 2 previous errors

View File

@ -1,7 +1,7 @@
#![warn(clippy::drop_ref)]
#![allow(clippy::toplevel_ref_arg)]
#![allow(clippy::map_err_ignore)]
#![allow(clippy::unnecessary_wraps)]
#![allow(clippy::unnecessary_wraps, clippy::drop_non_drop)]
use std::mem::drop;

View File

@ -0,0 +1,27 @@
#![warn(clippy::forget_non_drop)]
use core::mem::forget;
fn forget_generic<T>(t: T) {
// Don't lint
forget(t)
}
fn main() {
struct Foo;
// Lint
forget(Foo);
struct Bar;
impl Drop for Bar {
fn drop(&mut self) {}
}
// Don't lint
forget(Bar);
struct Baz<T>(T);
// Lint
forget(Baz(Foo));
// Don't lint
forget(Baz(Bar));
}

View File

@ -0,0 +1,27 @@
error: call to `std::mem::forget` with a value that does not implement `Drop`. Forgetting such a type is the same as dropping it
--> $DIR/forget_non_drop.rs:13:5
|
LL | forget(Foo);
| ^^^^^^^^^^^
|
= note: `-D clippy::forget-non-drop` implied by `-D warnings`
note: argument has type `main::Foo`
--> $DIR/forget_non_drop.rs:13:12
|
LL | forget(Foo);
| ^^^
error: call to `std::mem::forget` with a value that does not implement `Drop`. Forgetting such a type is the same as dropping it
--> $DIR/forget_non_drop.rs:24:5
|
LL | forget(Baz(Foo));
| ^^^^^^^^^^^^^^^^
|
note: argument has type `main::Baz<main::Foo>`
--> $DIR/forget_non_drop.rs:24:12
|
LL | forget(Baz(Foo));
| ^^^^^^^^
error: aborting due to 2 previous errors

View File

@ -1,6 +1,6 @@
#![warn(clippy::forget_ref)]
#![allow(clippy::toplevel_ref_arg)]
#![allow(clippy::unnecessary_wraps)]
#![allow(clippy::unnecessary_wraps, clippy::forget_non_drop)]
use std::mem::forget;

View File

@ -1,7 +1,7 @@
// run-rustfix
// rustfix-only-machine-applicable
#![allow(clippy::implicit_clone)]
#![allow(clippy::implicit_clone, clippy::drop_non_drop)]
use std::ffi::OsString;
use std::path::Path;

View File

@ -1,7 +1,7 @@
// run-rustfix
// rustfix-only-machine-applicable
#![allow(clippy::implicit_clone)]
#![allow(clippy::implicit_clone, clippy::drop_non_drop)]
use std::ffi::OsString;
use std::path::Path;