From 447940c889d92b9cfa436a8c7fb964b1e1e4803a Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 20 Apr 2016 13:10:23 -0700 Subject: [PATCH 1/7] Added lint for mem_forget --- src/lib.rs | 3 +++ src/mem_forget.rs | 38 ++++++++++++++++++++++++++++++++ src/utils/paths.rs | 1 + tests/compile-fail/mem_forget.rs | 21 ++++++++++++++++++ 4 files changed, 63 insertions(+) create mode 100644 src/mem_forget.rs create mode 100644 tests/compile-fail/mem_forget.rs diff --git a/src/lib.rs b/src/lib.rs index 5774f3bf7af4..07a142060cb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -78,6 +78,7 @@ pub mod len_zero; pub mod lifetimes; pub mod loops; pub mod map_clone; +pub mod mem_forget; pub mod matches; pub mod methods; pub mod minmax; @@ -236,6 +237,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents)); reg.register_late_lint_pass(box neg_multiply::NegMultiply); reg.register_late_lint_pass(box unsafe_removed_from_name::UnsafeNameRemoval); + reg.register_late_lint_pass(box mem_forget::MemForget); reg.register_lint_group("clippy_pedantic", vec![ array_indexing::INDEXING_SLICING, @@ -243,6 +245,7 @@ pub fn plugin_registrar(reg: &mut Registry) { enum_glob_use::ENUM_GLOB_USE, if_not_else::IF_NOT_ELSE, matches::SINGLE_MATCH_ELSE, + mem_forget::MEM_FORGET, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, methods::WRONG_PUB_SELF_CONVENTION, diff --git a/src/mem_forget.rs b/src/mem_forget.rs new file mode 100644 index 000000000000..836c60f84a97 --- /dev/null +++ b/src/mem_forget.rs @@ -0,0 +1,38 @@ +use rustc::lint::*; +use rustc::hir::{Expr, ExprCall, ExprPath}; +use utils::{match_def_path, paths, span_lint}; + +/// **What it does:** This lint checks for usage of `std::mem::forget(_)`. +/// +/// **Why is this bad?** `std::mem::forget(t)` prevents `t` from running its destructor, possibly causing leaks +/// +/// **Known problems:** None. +/// +/// **Example:** `std::mem::forget(_))` +declare_lint! { + pub MEM_FORGET, + Allow, + "std::mem::forget usage is likely to cause memory leaks" +} + +pub struct MemForget; + +impl LintPass for MemForget { + fn get_lints(&self) -> LintArray { + lint_array![MEM_FORGET] + } +} + +impl LateLintPass for MemForget { + fn check_expr(&mut self, cx: &LateContext, e: &Expr) { + if let ExprCall(ref path_expr, _) = e.node { + if let ExprPath(None, _) = path_expr.node { + let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); + + if match_def_path(cx, def_id, &paths::MEM_FORGET) { + span_lint(cx, MEM_FORGET, e.span, "usage of std::mem::forget"); + } + } + } + } +} diff --git a/src/utils/paths.rs b/src/utils/paths.rs index 88d0dd415aad..38985373085b 100644 --- a/src/utils/paths.rs +++ b/src/utils/paths.rs @@ -20,6 +20,7 @@ pub const HASHMAP: [&'static str; 5] = ["std", "collections", "hash", "map", "Ha pub const HASH: [&'static str; 2] = ["hash", "Hash"]; pub const IO_PRINT: [&'static str; 3] = ["std", "io", "_print"]; pub const LINKED_LIST: [&'static str; 3] = ["collections", "linked_list", "LinkedList"]; +pub const MEM_FORGET: [&'static str; 3] = ["core", "mem", "forget"]; pub const MUTEX: [&'static str; 4] = ["std", "sync", "mutex", "Mutex"]; pub const OPEN_OPTIONS: [&'static str; 3] = ["std", "fs", "OpenOptions"]; pub const OPTION: [&'static str; 3] = ["core", "option", "Option"]; diff --git a/tests/compile-fail/mem_forget.rs b/tests/compile-fail/mem_forget.rs new file mode 100644 index 000000000000..1b9cc810f968 --- /dev/null +++ b/tests/compile-fail/mem_forget.rs @@ -0,0 +1,21 @@ +#![feature(plugin)] +#![plugin(clippy)] + +use std::sync::Arc; + +use std::mem::forget as forgetSomething; +use std::mem as memstuff; + +#[deny(mem_forget)] +fn main() { + let five: i32 = 5; + forgetSomething(five); + //~^ ERROR usage of std::mem::forget + + let six: Arc = Arc::new(6); + memstuff::forget(six); + //~^ ERROR usage of std::mem::forget + + std::mem::forget(7); + //~^ ERROR usage of std::mem::forget +} From 7961f59303a0829f0724b236cad0cbb001930d9c Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 20 Apr 2016 13:11:55 -0700 Subject: [PATCH 2/7] Ran update_lints and updated CHANGELOG.md to reflect addition of mem_forget --- CHANGELOG.md | 3 ++- README.md | 3 ++- src/lib.rs | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 272f5d77f350..0ad943c3f8a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file. ## Unreleased -* New lint: [`temporary_cstring_as_ptr`] and [`unsafe_removed_from_name`] +* New lints: [`temporary_cstring_as_ptr`], [`unsafe_removed_from_name`], and [`mem_forget`] ## 0.0.63 — 2016-04-08 * Rustup to *rustc 1.9.0-nightly (7979dd608 2016-04-07)* @@ -135,6 +135,7 @@ All notable changes to this project will be documented in this file. [`match_overlapping_arm`]: https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm [`match_ref_pats`]: https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats [`match_same_arms`]: https://github.com/Manishearth/rust-clippy/wiki#match_same_arms +[`mem_forget`]: https://github.com/Manishearth/rust-clippy/wiki#mem_forget [`min_max`]: https://github.com/Manishearth/rust-clippy/wiki#min_max [`modulo_one`]: https://github.com/Manishearth/rust-clippy/wiki#modulo_one [`mut_mut`]: https://github.com/Manishearth/rust-clippy/wiki#mut_mut diff --git a/README.md b/README.md index 6aa1e1ecf340..7b7a74cca4ca 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 143 lints included in this crate: +There are 144 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -85,6 +85,7 @@ name [match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms [match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead [match_same_arms](https://github.com/Manishearth/rust-clippy/wiki#match_same_arms) | warn | `match` with identical arm bodies +[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | std::mem::forget usage is likely to cause memory leaks [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references) diff --git a/src/lib.rs b/src/lib.rs index 07a142060cb0..5f3b3999f951 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -78,8 +78,8 @@ pub mod len_zero; pub mod lifetimes; pub mod loops; pub mod map_clone; -pub mod mem_forget; pub mod matches; +pub mod mem_forget; pub mod methods; pub mod minmax; pub mod misc; From 12ae306630f5029d0c3c7b52fa7532f5a2f308da Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 20 Apr 2016 13:33:05 -0700 Subject: [PATCH 3/7] Ticks around std::mem::forget --- README.md | 2 +- src/mem_forget.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7b7a74cca4ca..8e8a2627557e 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ name [match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms [match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead [match_same_arms](https://github.com/Manishearth/rust-clippy/wiki#match_same_arms) | warn | `match` with identical arm bodies -[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | std::mem::forget usage is likely to cause memory leaks +[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `std::mem::forget` usage is likely to cause memory leaks [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references) diff --git a/src/mem_forget.rs b/src/mem_forget.rs index 836c60f84a97..651e56d709f4 100644 --- a/src/mem_forget.rs +++ b/src/mem_forget.rs @@ -12,7 +12,7 @@ use utils::{match_def_path, paths, span_lint}; declare_lint! { pub MEM_FORGET, Allow, - "std::mem::forget usage is likely to cause memory leaks" + "`std::mem::forget` usage is likely to cause memory leaks" } pub struct MemForget; From 5158a08c5b048a5e9ce094d4999db8cb6f13ab44 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 20 Apr 2016 18:55:41 -0700 Subject: [PATCH 4/7] Changed std::mem::forget errors to mem::forget --- README.md | 2 +- src/mem_forget.rs | 6 +++--- tests/compile-fail/mem_forget.rs | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 8e8a2627557e..0a4f83ffee62 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ name [match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms [match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead [match_same_arms](https://github.com/Manishearth/rust-clippy/wiki#match_same_arms) | warn | `match` with identical arm bodies -[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `std::mem::forget` usage is likely to cause memory leaks +[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `mem::forget` usage is likely to cause memory leaks [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references) diff --git a/src/mem_forget.rs b/src/mem_forget.rs index 651e56d709f4..d857adf10d65 100644 --- a/src/mem_forget.rs +++ b/src/mem_forget.rs @@ -8,11 +8,11 @@ use utils::{match_def_path, paths, span_lint}; /// /// **Known problems:** None. /// -/// **Example:** `std::mem::forget(_))` +/// **Example:** `mem::forget(_))` declare_lint! { pub MEM_FORGET, Allow, - "`std::mem::forget` usage is likely to cause memory leaks" + "`mem::forget` usage is likely to cause memory leaks" } pub struct MemForget; @@ -30,7 +30,7 @@ impl LateLintPass for MemForget { let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); if match_def_path(cx, def_id, &paths::MEM_FORGET) { - span_lint(cx, MEM_FORGET, e.span, "usage of std::mem::forget"); + span_lint(cx, MEM_FORGET, e.span, "usage of mem::forget"); } } } diff --git a/tests/compile-fail/mem_forget.rs b/tests/compile-fail/mem_forget.rs index 1b9cc810f968..5198b4ea8d3a 100644 --- a/tests/compile-fail/mem_forget.rs +++ b/tests/compile-fail/mem_forget.rs @@ -10,12 +10,12 @@ use std::mem as memstuff; fn main() { let five: i32 = 5; forgetSomething(five); - //~^ ERROR usage of std::mem::forget + //~^ ERROR usage of mem::forget let six: Arc = Arc::new(6); memstuff::forget(six); - //~^ ERROR usage of std::mem::forget + //~^ ERROR usage of mem::forget std::mem::forget(7); - //~^ ERROR usage of std::mem::forget + //~^ ERROR usage of mem::forget } From 77427b6ead79c54648c47f5048953590a59615bb Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Wed, 20 Apr 2016 19:24:31 -0700 Subject: [PATCH 5/7] Limited mem_forget error to only Drop types (fails) --- src/mem_forget.rs | 19 ++++++++++++------- tests/compile-fail/mem_forget.rs | 13 ++++++++++--- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/mem_forget.rs b/src/mem_forget.rs index d857adf10d65..0568e70023a7 100644 --- a/src/mem_forget.rs +++ b/src/mem_forget.rs @@ -1,18 +1,18 @@ use rustc::lint::*; use rustc::hir::{Expr, ExprCall, ExprPath}; -use utils::{match_def_path, paths, span_lint}; +use utils::{get_trait_def_id, implements_trait, match_def_path, paths, span_lint}; -/// **What it does:** This lint checks for usage of `std::mem::forget(_)`. +/// **What it does:** This lint checks for usage of `std::mem::forget(t)` where `t` is `Drop`. /// /// **Why is this bad?** `std::mem::forget(t)` prevents `t` from running its destructor, possibly causing leaks /// /// **Known problems:** None. /// -/// **Example:** `mem::forget(_))` +/// **Example:** `mem::forget(Rc::new(55)))` declare_lint! { pub MEM_FORGET, Allow, - "`mem::forget` usage is likely to cause memory leaks" + "`mem::forget` usage on `Drop` types is likely to cause memory leaks" } pub struct MemForget; @@ -25,12 +25,17 @@ impl LintPass for MemForget { impl LateLintPass for MemForget { fn check_expr(&mut self, cx: &LateContext, e: &Expr) { - if let ExprCall(ref path_expr, _) = e.node { + if let ExprCall(ref path_expr, ref args) = e.node { if let ExprPath(None, _) = path_expr.node { let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); - if match_def_path(cx, def_id, &paths::MEM_FORGET) { - span_lint(cx, MEM_FORGET, e.span, "usage of mem::forget"); + if let Some(drop_trait_id) = get_trait_def_id(cx, &paths::DROP) { + let forgot_ty = cx.tcx.expr_ty(&args[0]); + + if implements_trait(cx, forgot_ty, drop_trait_id, Vec::new()) { + span_lint(cx, MEM_FORGET, e.span, "usage of mem::forget on Drop type"); + } + } } } } diff --git a/tests/compile-fail/mem_forget.rs b/tests/compile-fail/mem_forget.rs index 5198b4ea8d3a..c8cebcb2a42d 100644 --- a/tests/compile-fail/mem_forget.rs +++ b/tests/compile-fail/mem_forget.rs @@ -2,6 +2,7 @@ #![plugin(clippy)] use std::sync::Arc; +use std::rc::Rc; use std::mem::forget as forgetSomething; use std::mem as memstuff; @@ -10,12 +11,18 @@ use std::mem as memstuff; fn main() { let five: i32 = 5; forgetSomething(five); - //~^ ERROR usage of mem::forget let six: Arc = Arc::new(6); memstuff::forget(six); - //~^ ERROR usage of mem::forget + //~^ ERROR usage of mem::forget on Drop type + + let seven: Rc = Rc::new(7); + std::mem::forget(seven); + //~^ ERROR usage of mem::forget on Drop type + + let eight: Vec = vec![8]; + forgetSomething(eight); + //~^ ERROR usage of mem::forget on Drop type std::mem::forget(7); - //~^ ERROR usage of mem::forget } From 8866ba9e2a744c5f033e7d1f5b0298da77550b91 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Thu, 21 Apr 2016 09:36:39 -0700 Subject: [PATCH 6/7] Fixed destructor detection in mem_forget --- src/mem_forget.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/mem_forget.rs b/src/mem_forget.rs index 0568e70023a7..1f627d614ff3 100644 --- a/src/mem_forget.rs +++ b/src/mem_forget.rs @@ -1,6 +1,6 @@ use rustc::lint::*; use rustc::hir::{Expr, ExprCall, ExprPath}; -use utils::{get_trait_def_id, implements_trait, match_def_path, paths, span_lint}; +use utils::{match_def_path, paths, span_lint}; /// **What it does:** This lint checks for usage of `std::mem::forget(t)` where `t` is `Drop`. /// @@ -29,12 +29,13 @@ impl LateLintPass for MemForget { if let ExprPath(None, _) = path_expr.node { let def_id = cx.tcx.def_map.borrow()[&path_expr.id].def_id(); if match_def_path(cx, def_id, &paths::MEM_FORGET) { - if let Some(drop_trait_id) = get_trait_def_id(cx, &paths::DROP) { - let forgot_ty = cx.tcx.expr_ty(&args[0]); + let forgot_ty = cx.tcx.expr_ty(&args[0]); - if implements_trait(cx, forgot_ty, drop_trait_id, Vec::new()) { - span_lint(cx, MEM_FORGET, e.span, "usage of mem::forget on Drop type"); - } + if match forgot_ty.ty_adt_def() { + Some(def) => def.has_dtor(), + _ => false + } { + span_lint(cx, MEM_FORGET, e.span, "usage of mem::forget on Drop type"); } } } From 1e2cc78e08b2a119a3d69a82e24234fabdeee019 Mon Sep 17 00:00:00 2001 From: Taylor Cramer Date: Thu, 21 Apr 2016 09:41:38 -0700 Subject: [PATCH 7/7] Ran script to update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0a4f83ffee62..d55536c033d8 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ name [match_overlapping_arm](https://github.com/Manishearth/rust-clippy/wiki#match_overlapping_arm) | warn | a match has overlapping arms [match_ref_pats](https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats) | warn | a match or `if let` has all arms prefixed with `&`; the match expression can be dereferenced instead [match_same_arms](https://github.com/Manishearth/rust-clippy/wiki#match_same_arms) | warn | `match` with identical arm bodies -[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `mem::forget` usage is likely to cause memory leaks +[mem_forget](https://github.com/Manishearth/rust-clippy/wiki#mem_forget) | allow | `mem::forget` usage on `Drop` types is likely to cause memory leaks [min_max](https://github.com/Manishearth/rust-clippy/wiki#min_max) | warn | `min(_, max(_, _))` (or vice versa) with bounds clamping the result to a constant [modulo_one](https://github.com/Manishearth/rust-clippy/wiki#modulo_one) | warn | taking a number modulo 1, which always returns 0 [mut_mut](https://github.com/Manishearth/rust-clippy/wiki#mut_mut) | allow | usage of double-mut refs, e.g. `&mut &mut ...` (either copy'n'paste error, or shows a fundamental misunderstanding of references)