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

Lint mem_forget if any fields are Drop #10996

Merged
merged 3 commits into from
Jun 25, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::double_parens::DOUBLE_PARENS_INFO,
crate::drop_forget_ref::DROP_NON_DROP_INFO,
crate::drop_forget_ref::FORGET_NON_DROP_INFO,
crate::drop_forget_ref::MEM_FORGET_INFO,
crate::drop_forget_ref::UNDROPPED_MANUALLY_DROPS_INFO,
crate::duplicate_mod::DUPLICATE_MOD_INFO,
crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO,
Expand Down Expand Up @@ -310,7 +311,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::matches::TRY_ERR_INFO,
crate::matches::WILDCARD_ENUM_MATCH_ARM_INFO,
crate::matches::WILDCARD_IN_OR_PATTERNS_INFO,
crate::mem_forget::MEM_FORGET_INFO,
crate::mem_replace::MEM_REPLACE_OPTION_WITH_NONE_INFO,
crate::mem_replace::MEM_REPLACE_WITH_DEFAULT_INFO,
crate::mem_replace::MEM_REPLACE_WITH_UNINIT_INFO,
Expand Down
54 changes: 46 additions & 8 deletions clippy_lints/src/drop_forget_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_hir::{Arm, Expr, ExprKind, LangItem, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
use std::borrow::Cow;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -76,6 +77,27 @@ declare_clippy_lint! {
"use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `std::mem::forget(t)` where `t` is
/// `Drop` or has a field that implements `Drop`.
///
/// ### Why is this bad?
/// `std::mem::forget(t)` prevents `t` from running its
/// destructor, possibly causing leaks.
///
/// ### Example
/// ```rust
/// # use std::mem;
/// # use std::rc::Rc;
/// mem::forget(Rc::new(55))
/// ```
#[clippy::version = "pre 1.29.0"]
pub MEM_FORGET,
restriction,
"`mem::forget` usage on `Drop` types, likely to cause memory leaks"
}

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 its contained lifetimes";
const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \
Expand All @@ -84,7 +106,8 @@ const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value t
declare_lint_pass!(DropForgetRef => [
DROP_NON_DROP,
FORGET_NON_DROP,
UNDROPPED_MANUALLY_DROPS
UNDROPPED_MANUALLY_DROPS,
MEM_FORGET,
]);

impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
Expand All @@ -97,7 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
let arg_ty = cx.typeck_results().expr_ty(arg);
let is_copy = is_copy(cx, arg_ty);
let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr);
let (lint, msg) = match fn_name {
let (lint, msg, note_span) = match fn_name {
// early return for uplifted lints: dropping_references, dropping_copy_types, forgetting_references, forgetting_copy_types
sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => return,
sym::mem_forget if arg_ty.is_ref() => return,
Expand All @@ -121,19 +144,34 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
|| drop_is_single_call_in_arm
) =>
{
(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)
(DROP_NON_DROP, DROP_NON_DROP_SUMMARY.into(), Some(arg.span))
},
sym::mem_forget => {
if arg_ty.needs_drop(cx.tcx, cx.param_env) {
(
MEM_FORGET,
Cow::Owned(format!(
"usage of `mem::forget` on {}",
if arg_ty.ty_adt_def().map_or(false, |def| def.has_dtor(cx.tcx)) {
"`Drop` type"
} else {
"type with `Drop` fields"
}
)),
None,
)
} else {
(FORGET_NON_DROP, FORGET_NON_DROP_SUMMARY.into(), Some(arg.span))
}
}
_ => return,
};
span_lint_and_note(
cx,
lint,
expr.span,
msg,
Some(arg.span),
&msg,
note_span,
&format!("argument has type `{arg_ty}`"),
);
}
Expand Down
2 changes: 0 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ mod manual_strip;
mod map_unit_fn;
mod match_result_ok;
mod matches;
mod mem_forget;
mod mem_replace;
mod methods;
mod min_ident_chars;
Expand Down Expand Up @@ -744,7 +743,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let missing_docs_in_crate_items = conf.missing_docs_in_crate_items;
store.register_late_pass(move |_| Box::new(doc::DocMarkdown::new(doc_valid_idents.clone())));
store.register_late_pass(|_| Box::new(neg_multiply::NegMultiply));
store.register_late_pass(|_| Box::new(mem_forget::MemForget));
store.register_late_pass(|_| Box::new(let_if_seq::LetIfSeq));
store.register_late_pass(|_| Box::new(mixed_read_write_in_expression::EvalOrderDependence));
store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(missing_docs_in_crate_items)));
Expand Down
46 changes: 0 additions & 46 deletions clippy_lints/src/mem_forget.rs

This file was deleted.

3 changes: 3 additions & 0 deletions tests/ui/mem_forget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,8 @@ fn main() {
let eight: Vec<i32> = vec![8];
forgetSomething(eight);

let string = String::new();
std::mem::forget(string);

std::mem::forget(7);
}
15 changes: 14 additions & 1 deletion tests/ui/mem_forget.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,32 @@ error: usage of `mem::forget` on `Drop` type
LL | memstuff::forget(six);
| ^^^^^^^^^^^^^^^^^^^^^
|
= note: argument has type `std::sync::Arc<i32>`
= note: `-D clippy::mem-forget` implied by `-D warnings`

error: usage of `mem::forget` on `Drop` type
--> $DIR/mem_forget.rs:17:5
|
LL | std::mem::forget(seven);
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: argument has type `std::rc::Rc<i32>`

error: usage of `mem::forget` on `Drop` type
--> $DIR/mem_forget.rs:20:5
|
LL | forgetSomething(eight);
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: argument has type `std::vec::Vec<i32>`

error: usage of `mem::forget` on type with `Drop` fields
--> $DIR/mem_forget.rs:23:5
|
LL | std::mem::forget(string);
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: argument has type `std::string::String`

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors