Skip to content

Commit 82e9931

Browse files
committed
Auto merge of #109732 - Urgau:uplift_drop_forget_ref_lints, r=davidtwco
Uplift `clippy::{drop,forget}_{ref,copy}` lints This PR aims at uplifting the `clippy::drop_ref`, `clippy::drop_copy`, `clippy::forget_ref` and `clippy::forget_copy` lints. Those lints are/were declared in the correctness category of clippy because they lint on useless and most probably is not what the developer wanted. ## `drop_ref` and `forget_ref` The `drop_ref` and `forget_ref` lint checks for calls to `std::mem::drop` or `std::mem::forget` with a reference instead of an owned value. ### Example ```rust let mut lock_guard = mutex.lock(); std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex // still locked operation_that_requires_mutex_to_be_unlocked(); ``` ### Explanation Calling `drop` or `forget` on a reference will only drop the reference itself, which is a no-op. It will not call the `drop` or `forget` method on the underlying referenced value, which is likely what was intended. ## `drop_copy` and `forget_copy` The `drop_copy` and `forget_copy` lint checks for calls to `std::mem::forget` or `std::mem::drop` with a value that derives the Copy trait. ### Example ```rust let x: i32 = 42; // i32 implements Copy std::mem::forget(x) // A copy of x is passed to the function, leaving the // original unaffected ``` ### Explanation Calling `std::mem::forget` [does nothing for types that implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html) since the value will be copied and moved into the function on invocation. ----- Followed the instructions for uplift a clippy describe here: rust-lang/rust#99696 (review) cc `@m-ou-se` (as T-libs-api leader because the uplifting was discussed in a recent meeting)
2 parents 26fb53e + fd784bb commit 82e9931

16 files changed

+97
-767
lines changed

clippy_lints/src/declared_lints.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,12 +132,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
132132
crate::doc::NEEDLESS_DOCTEST_MAIN_INFO,
133133
crate::doc::UNNECESSARY_SAFETY_DOC_INFO,
134134
crate::double_parens::DOUBLE_PARENS_INFO,
135-
crate::drop_forget_ref::DROP_COPY_INFO,
136135
crate::drop_forget_ref::DROP_NON_DROP_INFO,
137-
crate::drop_forget_ref::DROP_REF_INFO,
138-
crate::drop_forget_ref::FORGET_COPY_INFO,
139136
crate::drop_forget_ref::FORGET_NON_DROP_INFO,
140-
crate::drop_forget_ref::FORGET_REF_INFO,
141137
crate::drop_forget_ref::UNDROPPED_MANUALLY_DROPS_INFO,
142138
crate::duplicate_mod::DUPLICATE_MOD_INFO,
143139
crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO,

clippy_lints/src/drop_forget_ref.rs

Lines changed: 5 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -7,102 +7,6 @@ use rustc_lint::{LateContext, LateLintPass};
77
use rustc_session::{declare_lint_pass, declare_tool_lint};
88
use rustc_span::sym;
99

10-
declare_clippy_lint! {
11-
/// ### What it does
12-
/// Checks for calls to `std::mem::drop` with a reference
13-
/// instead of an owned value.
14-
///
15-
/// ### Why is this bad?
16-
/// Calling `drop` on a reference will only drop the
17-
/// reference itself, which is a no-op. It will not call the `drop` method (from
18-
/// the `Drop` trait implementation) on the underlying referenced value, which
19-
/// is likely what was intended.
20-
///
21-
/// ### Example
22-
/// ```ignore
23-
/// let mut lock_guard = mutex.lock();
24-
/// std::mem::drop(&lock_guard) // Should have been drop(lock_guard), mutex
25-
/// // still locked
26-
/// operation_that_requires_mutex_to_be_unlocked();
27-
/// ```
28-
#[clippy::version = "pre 1.29.0"]
29-
pub DROP_REF,
30-
correctness,
31-
"calls to `std::mem::drop` with a reference instead of an owned value"
32-
}
33-
34-
declare_clippy_lint! {
35-
/// ### What it does
36-
/// Checks for calls to `std::mem::forget` with a reference
37-
/// instead of an owned value.
38-
///
39-
/// ### Why is this bad?
40-
/// Calling `forget` on a reference will only forget the
41-
/// reference itself, which is a no-op. It will not forget the underlying
42-
/// referenced
43-
/// value, which is likely what was intended.
44-
///
45-
/// ### Example
46-
/// ```rust
47-
/// let x = Box::new(1);
48-
/// std::mem::forget(&x) // Should have been forget(x), x will still be dropped
49-
/// ```
50-
#[clippy::version = "pre 1.29.0"]
51-
pub FORGET_REF,
52-
correctness,
53-
"calls to `std::mem::forget` with a reference instead of an owned value"
54-
}
55-
56-
declare_clippy_lint! {
57-
/// ### What it does
58-
/// Checks for calls to `std::mem::drop` with a value
59-
/// that derives the Copy trait
60-
///
61-
/// ### Why is this bad?
62-
/// Calling `std::mem::drop` [does nothing for types that
63-
/// implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html), since the
64-
/// value will be copied and moved into the function on invocation.
65-
///
66-
/// ### Example
67-
/// ```rust
68-
/// let x: i32 = 42; // i32 implements Copy
69-
/// std::mem::drop(x) // A copy of x is passed to the function, leaving the
70-
/// // original unaffected
71-
/// ```
72-
#[clippy::version = "pre 1.29.0"]
73-
pub DROP_COPY,
74-
correctness,
75-
"calls to `std::mem::drop` with a value that implements Copy"
76-
}
77-
78-
declare_clippy_lint! {
79-
/// ### What it does
80-
/// Checks for calls to `std::mem::forget` with a value that
81-
/// derives the Copy trait
82-
///
83-
/// ### Why is this bad?
84-
/// Calling `std::mem::forget` [does nothing for types that
85-
/// implement Copy](https://doc.rust-lang.org/std/mem/fn.drop.html) since the
86-
/// value will be copied and moved into the function on invocation.
87-
///
88-
/// An alternative, but also valid, explanation is that Copy types do not
89-
/// implement
90-
/// the Drop trait, which means they have no destructors. Without a destructor,
91-
/// there
92-
/// is nothing for `std::mem::forget` to ignore.
93-
///
94-
/// ### Example
95-
/// ```rust
96-
/// let x: i32 = 42; // i32 implements Copy
97-
/// std::mem::forget(x) // A copy of x is passed to the function, leaving the
98-
/// // original unaffected
99-
/// ```
100-
#[clippy::version = "pre 1.29.0"]
101-
pub FORGET_COPY,
102-
correctness,
103-
"calls to `std::mem::forget` with a value that implements Copy"
104-
}
105-
10610
declare_clippy_lint! {
10711
/// ### What it does
10812
/// Checks for calls to `std::mem::drop` with a value that does not implement `Drop`.
@@ -172,24 +76,12 @@ declare_clippy_lint! {
17276
"use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value"
17377
}
17478

175-
const DROP_REF_SUMMARY: &str = "calls to `std::mem::drop` with a reference instead of an owned value. \
176-
Dropping a reference does nothing";
177-
const FORGET_REF_SUMMARY: &str = "calls to `std::mem::forget` with a reference instead of an owned value. \
178-
Forgetting a reference does nothing";
179-
const DROP_COPY_SUMMARY: &str = "calls to `std::mem::drop` with a value that implements `Copy`. \
180-
Dropping a copy leaves the original intact";
181-
const FORGET_COPY_SUMMARY: &str = "calls to `std::mem::forget` with a value that implements `Copy`. \
182-
Forgetting a copy leaves the original intact";
18379
const DROP_NON_DROP_SUMMARY: &str = "call to `std::mem::drop` with a value that does not implement `Drop`. \
18480
Dropping such a type only extends its contained lifetimes";
18581
const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \
18682
Forgetting such a type is the same as dropping it";
18783

18884
declare_lint_pass!(DropForgetRef => [
189-
DROP_REF,
190-
FORGET_REF,
191-
DROP_COPY,
192-
FORGET_COPY,
19385
DROP_NON_DROP,
19486
FORGET_NON_DROP,
19587
UNDROPPED_MANUALLY_DROPS
@@ -206,10 +98,11 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
20698
let is_copy = is_copy(cx, arg_ty);
20799
let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr);
208100
let (lint, msg) = match fn_name {
209-
sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => (DROP_REF, DROP_REF_SUMMARY),
210-
sym::mem_forget if arg_ty.is_ref() => (FORGET_REF, FORGET_REF_SUMMARY),
211-
sym::mem_drop if is_copy && !drop_is_single_call_in_arm => (DROP_COPY, DROP_COPY_SUMMARY),
212-
sym::mem_forget if is_copy => (FORGET_COPY, FORGET_COPY_SUMMARY),
101+
// early return for uplifted lints: drop_ref, drop_copy, forget_ref, forget_copy
102+
sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => return,
103+
sym::mem_forget if arg_ty.is_ref() => return,
104+
sym::mem_drop if is_copy && !drop_is_single_call_in_arm => return,
105+
sym::mem_forget if is_copy => return,
213106
sym::mem_drop if is_type_lang_item(cx, arg_ty, LangItem::ManuallyDrop) => {
214107
span_lint_and_help(
215108
cx,

clippy_lints/src/renamed_lints.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,13 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
3232
("clippy::zero_width_space", "clippy::invisible_characters"),
3333
("clippy::clone_double_ref", "suspicious_double_ref_op"),
3434
("clippy::drop_bounds", "drop_bounds"),
35+
("clippy::drop_copy", "drop_copy"),
36+
("clippy::drop_ref", "drop_ref"),
3537
("clippy::for_loop_over_option", "for_loops_over_fallibles"),
3638
("clippy::for_loop_over_result", "for_loops_over_fallibles"),
3739
("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"),
40+
("clippy::forget_copy", "forget_copy"),
41+
("clippy::forget_ref", "forget_ref"),
3842
("clippy::into_iter_on_array", "array_into_iter"),
3943
("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"),
4044
("clippy::invalid_ref", "invalid_value"),

tests/ui/drop_forget_copy.rs

Lines changed: 0 additions & 86 deletions
This file was deleted.

tests/ui/drop_forget_copy.stderr

Lines changed: 0 additions & 112 deletions
This file was deleted.

0 commit comments

Comments
 (0)