From 4a4f998c39b1b7e06c34a5fc8ed90e8752142e20 Mon Sep 17 00:00:00 2001 From: cgm616 Date: Fri, 16 Oct 2020 11:12:37 -0400 Subject: [PATCH 1/2] Add new lint for undropped ManuallyDrop values --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 4 ++ clippy_lints/src/undropped_manually_drops.rs | 49 ++++++++++++++++++++ src/lintlist/mod.rs | 7 +++ tests/ui/undropped_manually_drops.rs | 18 +++++++ 5 files changed, 79 insertions(+) create mode 100644 clippy_lints/src/undropped_manually_drops.rs create mode 100644 tests/ui/undropped_manually_drops.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d82f970b8bf2..697e6166fdf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1979,6 +1979,7 @@ Released 2018-09-13 [`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err [`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity [`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds +[`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc [`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented [`uninit_assumed_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_assumed_init diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d4d2f92a6a69..49ff8ad366e6 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -314,6 +314,7 @@ mod transmuting_null; mod trivially_copy_pass_by_ref; mod try_err; mod types; +mod undropped_manually_drops; mod unicode; mod unit_return_expecting_ord; mod unnamed_address; @@ -862,6 +863,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &types::UNIT_CMP, &types::UNNECESSARY_CAST, &types::VEC_BOX, + &undropped_manually_drops::UNDROPPED_MANUALLY_DROPS, &unicode::INVISIBLE_CHARACTERS, &unicode::NON_ASCII_LITERAL, &unicode::UNICODE_NOT_NFC, @@ -1137,6 +1139,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods)); store.register_early_pass(|| box asm_syntax::InlineAsmX86AttSyntax); store.register_early_pass(|| box asm_syntax::InlineAsmX86IntelSyntax); + store.register_late_pass(|| box undropped_manually_drops::UndroppedManuallyDrops); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ @@ -1790,6 +1793,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::ABSURD_EXTREME_COMPARISONS), LintId::of(&types::CAST_REF_TO_MUT), LintId::of(&types::UNIT_CMP), + LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS), LintId::of(&unicode::INVISIBLE_CHARACTERS), LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD), LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), diff --git a/clippy_lints/src/undropped_manually_drops.rs b/clippy_lints/src/undropped_manually_drops.rs new file mode 100644 index 000000000000..48a050777b77 --- /dev/null +++ b/clippy_lints/src/undropped_manually_drops.rs @@ -0,0 +1,49 @@ +use rustc_lint::{LateLintPass, LateContext}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_hir::*; +use crate::utils::{match_function_call, is_type_lang_item, paths, span_lint_and_help}; + +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:** None. + /// + /// **Example:** + /// + /// ```rust + /// struct S; + /// drop(std::mem::ManuallyDrop::new(S)); + /// ``` + /// Use instead: + /// ```rust + /// struct S; + /// unsafe { + /// std::mem::ManuallyDrop::drop(std::mem::ManuallyDrop::new(S)); + /// } + /// ``` + 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 LateLintPass<'tcx> for UndroppedManuallyDrops { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let Some(ref args) = match_function_call(cx, expr, &paths::DROP) { + let ty = cx.typeck_results().expr_ty(&args[0]); + if is_type_lang_item(cx, ty, lang_items::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`, use std::mem::ManuallyDrop::drop" + ); + } + } + } +} \ No newline at end of file diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 6301d623a2b1..25a69e78c8f4 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2412,6 +2412,13 @@ vec![ deprecation: None, module: "trait_bounds", }, + Lint { + name: "undropped_manually_drops", + group: "correctness", + desc: "use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value", + deprecation: None, + module: "undropped_manually_drops", + }, Lint { name: "unicode_not_nfc", group: "pedantic", diff --git a/tests/ui/undropped_manually_drops.rs b/tests/ui/undropped_manually_drops.rs new file mode 100644 index 000000000000..bea62e1751e3 --- /dev/null +++ b/tests/ui/undropped_manually_drops.rs @@ -0,0 +1,18 @@ +#![warn(clippy::undropped_manually_drops)] + +struct S; + +fn main() { + let f = drop; + let manual = std::mem::ManuallyDrop::new(S); + + // These lines will not drop `S` + drop(std::mem::ManuallyDrop::new(S)); + f(manual); + + // These lines will + unsafe { + std::mem::ManuallyDrop::drop(std::mem::ManuallyDrop::new(S)); + std::mem::ManuallyDrop::drop(manual); + } +} From e70817e712fd4d4e930ead0d587031e2b4a97a2e Mon Sep 17 00:00:00 2001 From: cgm616 Date: Fri, 16 Oct 2020 16:20:03 -0400 Subject: [PATCH 2/2] Update tests and add known problems to docs --- clippy_lints/src/lib.rs | 1 + clippy_lints/src/undropped_manually_drops.rs | 15 ++++++------- tests/ui/undropped_manually_drops.rs | 22 +++++++++++++------- tests/ui/undropped_manually_drops.stderr | 19 +++++++++++++++++ 4 files changed, 43 insertions(+), 14 deletions(-) create mode 100644 tests/ui/undropped_manually_drops.stderr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 49ff8ad366e6..97e7cfd1bb26 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1524,6 +1524,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNIT_CMP), LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), + LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS), LintId::of(&unicode::INVISIBLE_CHARACTERS), LintId::of(&unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD), LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS), diff --git a/clippy_lints/src/undropped_manually_drops.rs b/clippy_lints/src/undropped_manually_drops.rs index 48a050777b77..5443f1601fcb 100644 --- a/clippy_lints/src/undropped_manually_drops.rs +++ b/clippy_lints/src/undropped_manually_drops.rs @@ -1,14 +1,15 @@ -use rustc_lint::{LateLintPass, LateContext}; +use crate::utils::{is_type_lang_item, match_function_call, paths, span_lint_and_help}; +use rustc_hir::{lang_items, Expr}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_hir::*; -use crate::utils::{match_function_call, is_type_lang_item, paths, span_lint_and_help}; 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:** None. + /// **Known problems:** Does not catch cases if the user binds `std::mem::drop` + /// to a different name and calls it that way. /// /// **Example:** /// @@ -20,7 +21,7 @@ declare_clippy_lint! { /// ```rust /// struct S; /// unsafe { - /// std::mem::ManuallyDrop::drop(std::mem::ManuallyDrop::new(S)); + /// std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S)); /// } /// ``` pub UNDROPPED_MANUALLY_DROPS, @@ -41,9 +42,9 @@ impl LateLintPass<'tcx> for UndroppedManuallyDrops { expr.span, "the inner value of this ManuallyDrop will not be dropped", None, - "to drop a `ManuallyDrop`, use std::mem::ManuallyDrop::drop" + "to drop a `ManuallyDrop`, use std::mem::ManuallyDrop::drop", ); } } } -} \ No newline at end of file +} diff --git a/tests/ui/undropped_manually_drops.rs b/tests/ui/undropped_manually_drops.rs index bea62e1751e3..f4cfc92e1cd0 100644 --- a/tests/ui/undropped_manually_drops.rs +++ b/tests/ui/undropped_manually_drops.rs @@ -3,16 +3,24 @@ struct S; fn main() { - let f = drop; - let manual = std::mem::ManuallyDrop::new(S); + let f = std::mem::drop; + let g = std::mem::ManuallyDrop::drop; + let mut manual1 = std::mem::ManuallyDrop::new(S); + let mut manual2 = std::mem::ManuallyDrop::new(S); + let mut manual3 = std::mem::ManuallyDrop::new(S); + let mut manual4 = std::mem::ManuallyDrop::new(S); - // These lines will not drop `S` + // These lines will not drop `S` and should be linted drop(std::mem::ManuallyDrop::new(S)); - f(manual); + drop(manual1); - // These lines will + // FIXME: this line is not linted, though it should be + f(manual2); + + // These lines will drop `S` and should be okay. unsafe { - std::mem::ManuallyDrop::drop(std::mem::ManuallyDrop::new(S)); - std::mem::ManuallyDrop::drop(manual); + std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S)); + std::mem::ManuallyDrop::drop(&mut manual3); + g(&mut manual4); } } diff --git a/tests/ui/undropped_manually_drops.stderr b/tests/ui/undropped_manually_drops.stderr new file mode 100644 index 000000000000..2ac0fe98697e --- /dev/null +++ b/tests/ui/undropped_manually_drops.stderr @@ -0,0 +1,19 @@ +error: the inner value of this ManuallyDrop will not be dropped + --> $DIR/undropped_manually_drops.rs:14:5 + | +LL | drop(std::mem::ManuallyDrop::new(S)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::undropped-manually-drops` implied by `-D warnings` + = help: to drop a `ManuallyDrop`, use std::mem::ManuallyDrop::drop + +error: the inner value of this ManuallyDrop will not be dropped + --> $DIR/undropped_manually_drops.rs:15:5 + | +LL | drop(manual1); + | ^^^^^^^^^^^^^ + | + = help: to drop a `ManuallyDrop`, use std::mem::ManuallyDrop::drop + +error: aborting due to 2 previous errors +