From b6f194b48c2b7b1cd6a19592a8376056a826d223 Mon Sep 17 00:00:00 2001 From: Catherine <114838443+Centri3@users.noreply.github.com> Date: Wed, 21 Jun 2023 15:56:24 -0500 Subject: [PATCH] move to `drop_forget_ref` --- clippy_lints/src/declared_lints.rs | 2 +- clippy_lints/src/drop_forget_ref.rs | 46 ++++++++++++++++++++---- clippy_lints/src/lib.rs | 2 -- clippy_lints/src/mem_forget.rs | 54 ----------------------------- tests/ui/mem_forget.stderr | 23 ++++++++++++ 5 files changed, 64 insertions(+), 63 deletions(-) delete mode 100644 clippy_lints/src/mem_forget.rs diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9c1e1f6702d..0eec18a91bc 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, @@ -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, diff --git a/clippy_lints/src/drop_forget_ref.rs b/clippy_lints/src/drop_forget_ref.rs index 9c60edb1794..36b231023a7 100644 --- a/clippy_lints/src/drop_forget_ref.rs +++ b/clippy_lints/src/drop_forget_ref.rs @@ -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 @@ -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`. \ @@ -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 { @@ -121,18 +144,29 @@ 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()) }, + 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" + } + ))) + } else { + (FORGET_NON_DROP, FORGET_NON_DROP_SUMMARY.into()) + } + } _ => return, }; span_lint_and_note( cx, lint, expr.span, - msg, + &msg, Some(arg.span), &format!("argument has type `{arg_ty}`"), ); diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 3fb4e6c8fa5..71b8af7a821 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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))); diff --git a/clippy_lints/src/mem_forget.rs b/clippy_lints/src/mem_forget.rs deleted file mode 100644 index fdd55260483..00000000000 --- a/clippy_lints/src/mem_forget.rs +++ /dev/null @@ -1,54 +0,0 @@ -use clippy_utils::diagnostics::span_lint; -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 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" -} - -declare_lint_pass!(MemForget => [MEM_FORGET]); - -impl<'tcx> LateLintPass<'tcx> for MemForget { - fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - if let ExprKind::Call(path_expr, [ref first_arg, ..]) = e.kind - && let ExprKind::Path(ref qpath) = path_expr.kind - && let Some(def_id) = cx.qpath_res(qpath, path_expr.hir_id).opt_def_id() - && cx.tcx.is_diagnostic_item(sym::mem_forget, def_id) - && let forgot_ty = cx.typeck_results().expr_ty(first_arg) - && forgot_ty.needs_drop(cx.tcx, cx.param_env) - { - span_lint( - cx, - MEM_FORGET, - e.span, - &format!( - "usage of `mem::forget` on {}", - if forgot_ty.ty_adt_def().map_or(false, |def| def.has_dtor(cx.tcx)) { - "`Drop` type" - } else { - "type with `Drop` fields" - } - ), - ); - } - } -} diff --git a/tests/ui/mem_forget.stderr b/tests/ui/mem_forget.stderr index 50711120a15..6e3c31804d7 100644 --- a/tests/ui/mem_forget.stderr +++ b/tests/ui/mem_forget.stderr @@ -4,6 +4,11 @@ error: usage of `mem::forget` on `Drop` type LL | memstuff::forget(six); | ^^^^^^^^^^^^^^^^^^^^^ | +note: argument has type `std::sync::Arc` + --> $DIR/mem_forget.rs:14:22 + | +LL | memstuff::forget(six); + | ^^^ = note: `-D clippy::mem-forget` implied by `-D warnings` error: usage of `mem::forget` on `Drop` type @@ -11,18 +16,36 @@ error: usage of `mem::forget` on `Drop` type | LL | std::mem::forget(seven); | ^^^^^^^^^^^^^^^^^^^^^^^ + | +note: argument has type `std::rc::Rc` + --> $DIR/mem_forget.rs:17:22 + | +LL | std::mem::forget(seven); + | ^^^^^ error: usage of `mem::forget` on `Drop` type --> $DIR/mem_forget.rs:20:5 | LL | forgetSomething(eight); | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: argument has type `std::vec::Vec` + --> $DIR/mem_forget.rs:20:21 + | +LL | forgetSomething(eight); + | ^^^^^ 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` + --> $DIR/mem_forget.rs:23:22 + | +LL | std::mem::forget(string); + | ^^^^^^ error: aborting due to 4 previous errors