Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-23 21:23:56 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #1575 from tristianc/1537-drop_copy

Browse source 
Calls to mem::drop on Copy types
This commit is contained in:
Oliver Schneider 2017-03-24 13:39:30 +01:00 committed by GitHub
commit 269b8d33c9
9 changed files with 263 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -330,6 +330,7 @@ All notable changes to this project will be documented in this file.
[`doc_markdown`]: https://github.com/Manishearth/rust-clippy/wiki#doc_markdown
[`double_neg`]: https://github.com/Manishearth/rust-clippy/wiki#double_neg
[`double_parens`]: https://github.com/Manishearth/rust-clippy/wiki#double_parens
[`drop_copy`]: https://github.com/Manishearth/rust-clippy/wiki#drop_copy
[`drop_ref`]: https://github.com/Manishearth/rust-clippy/wiki#drop_ref
[`duplicate_underscore_argument`]: https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument
[`empty_enum`]: https://github.com/Manishearth/rust-clippy/wiki#empty_enum
@ -351,6 +352,7 @@ All notable changes to this project will be documented in this file.
[`for_kv_map`]: https://github.com/Manishearth/rust-clippy/wiki#for_kv_map
[`for_loop_over_option`]: https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option
[`for_loop_over_result`]: https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result
[`forget_copy`]: https://github.com/Manishearth/rust-clippy/wiki#forget_copy
[`forget_ref`]: https://github.com/Manishearth/rust-clippy/wiki#forget_ref
[`get_unwrap`]: https://github.com/Manishearth/rust-clippy/wiki#get_unwrap
[`identity_op`]: https://github.com/Manishearth/rust-clippy/wiki#identity_op

4
README.md
View file

@ -180,7 +180,7 @@ transparently:
## Lints
There are 194 lints included in this crate:
There are 196 lints included in this crate:
name | default | triggers on
-----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@ -218,6 +218,7 @@ name
[doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown) | warn | presence of `_`, `::` or camel-case outside backticks in documentation
[double_neg](https://github.com/Manishearth/rust-clippy/wiki#double_neg) | warn | `--x`, which is a double negation of `x` and not a pre-decrement as in C/C++
[double_parens](https://github.com/Manishearth/rust-clippy/wiki#double_parens) | warn | Warn on unnecessary double parentheses
[drop_copy](https://github.com/Manishearth/rust-clippy/wiki#drop_copy) | warn | calls to `std::mem::drop` with a value that implements Copy
[drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref) | warn | calls to `std::mem::drop` with a reference instead of an owned value
[duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument) | warn | function arguments having names which only differ by an underscore
[empty_enum](https://github.com/Manishearth/rust-clippy/wiki#empty_enum) | allow | enum with no variants
@ -238,6 +239,7 @@ name
[for_kv_map](https://github.com/Manishearth/rust-clippy/wiki#for_kv_map) | warn | looping on a map using `iter` when `keys` or `values` would do
[for_loop_over_option](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_option) | warn | for-looping over an `Option`, which is more clearly expressed as an `if let`
[for_loop_over_result](https://github.com/Manishearth/rust-clippy/wiki#for_loop_over_result) | warn | for-looping over a `Result`, which is more clearly expressed as an `if let`
[forget_copy](https://github.com/Manishearth/rust-clippy/wiki#forget_copy) | warn | calls to `std::mem::forget` with a value that implements Copy
[forget_ref](https://github.com/Manishearth/rust-clippy/wiki#forget_ref) | warn | calls to `std::mem::forget` with a reference instead of an owned value
[get_unwrap](https://github.com/Manishearth/rust-clippy/wiki#get_unwrap) | warn | using `.get().unwrap()` or `.get_mut().unwrap()` when using `[]` would work instead
[identity_op](https://github.com/Manishearth/rust-clippy/wiki#identity_op) | warn | using identity operations, e.g. `x + 0` or `y / 1`

96
clippy_lints/src/drop_forget_ref.rs
View file

@ -1,7 +1,7 @@
use rustc::lint::*;
use rustc::ty;
use rustc::hir::*;
use utils::{match_def_path, paths, span_note_and_lint};
use utils::{match_def_path, paths, span_note_and_lint, is_copy};
/// **What it does:** Checks for calls to `std::mem::drop` with a reference
/// instead of an owned value.
@ -45,12 +45,65 @@ declare_lint! {
"calls to `std::mem::forget` with a reference instead of an owned value"
}
/// **What it does:** Checks for calls to `std::mem::drop` with a value
/// that derives the Copy trait
///
/// **Why is this bad?** Calling `std::mem::drop` [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.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let x:i32 = 42; // i32 implements Copy
/// std::mem::drop(x) // A copy of x is passed to the function, leaving the original unaffected
/// ```
declare_lint! {
pub DROP_COPY,
Warn,
"calls to `std::mem::drop` with a value that implements Copy"
}
/// **What it does:** Checks for calls to `std::mem::forget` with a value that
/// derives the Copy trait
///
/// **Why is this bad?** 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.
///
/// An alternative, but also valid, explanation is that Copy types do not implement
/// the Drop trait, which means they have no destructors. Without a destructor, there
/// is nothing for `std::mem::forget` to ignore.
///
/// **Known problems:** None.
///
/// **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
/// ```
declare_lint! {
pub FORGET_COPY,
Warn,
"calls to `std::mem::forget` with a value that implements Copy"
}
const DROP_REF_SUMMARY: &str = "calls to `std::mem::drop` with a reference instead of an owned value. \
Dropping a reference does nothing.";
const FORGET_REF_SUMMARY: &str = "calls to `std::mem::forget` with a reference instead of an owned value. \
Forgetting a reference does nothing.";
const DROP_COPY_SUMMARY: &str = "calls to `std::mem::drop` with a value that implements Copy. \
Dropping a copy leaves the original intact.";
const FORGET_COPY_SUMMARY: &str = "calls to `std::mem::forget` with a value that implements Copy. \
Forgetting a copy leaves the original intact.";
#[allow(missing_copy_implementations)]
pub struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(DROP_REF, FORGET_REF)
lint_array!(DROP_REF, FORGET_REF, DROP_COPY, FORGET_COPY)
}
}
@ -64,24 +117,39 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
let def_id = cx.tables.qpath_def(qpath, path.id).def_id();
let lint;
let msg;
if match_def_path(cx.tcx, def_id, &paths::DROP) {
lint = DROP_REF;
msg = "call to `std::mem::drop` with a reference argument. \
Dropping a reference does nothing";
} else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) {
lint = FORGET_REF;
msg = "call to `std::mem::forget` with a reference argument. \
Forgetting a reference does nothing";
} else {
return;
}
let arg = &args[0];
let arg_ty = cx.tables.expr_ty(arg);
if let ty::TyRef(..) = arg_ty.sty {
if match_def_path(cx.tcx, def_id, &paths::DROP) {
lint = DROP_REF;
msg = DROP_REF_SUMMARY.to_string();
} else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) {
lint = FORGET_REF;
msg = FORGET_REF_SUMMARY.to_string();
} else {
return;
}
span_note_and_lint(cx,
lint,
expr.span,
msg,
&msg,
arg.span,
&format!("argument has type {}", arg_ty.sty));
} else if is_copy(cx, arg_ty, cx.tcx.hir.get_parent(arg.id)) {
if match_def_path(cx.tcx, def_id, &paths::DROP) {
lint = DROP_COPY;
msg = DROP_COPY_SUMMARY.to_string();
} else if match_def_path(cx.tcx, def_id, &paths::MEM_FORGET) {
lint = FORGET_COPY;
msg = FORGET_COPY_SUMMARY.to_string();
} else {
return;
}
span_note_and_lint(cx,
lint,
expr.span,
&msg,
arg.span,
&format!("argument has type {}", arg_ty.sty));
}

2
clippy_lints/src/lib.rs
View file

@ -374,7 +374,9 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
derive::EXPL_IMPL_CLONE_ON_COPY,
doc::DOC_MARKDOWN,
double_parens::DOUBLE_PARENS,
drop_forget_ref::DROP_COPY,
drop_forget_ref::DROP_REF,
drop_forget_ref::FORGET_COPY,
drop_forget_ref::FORGET_REF,
entry::MAP_ENTRY,
enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,

62
tests/ui/drop_forget_copy.rs Normal file
View file

@ -0,0 +1,62 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(drop_copy, forget_copy)]
#![allow(toplevel_ref_arg, drop_ref, forget_ref, unused_mut)]
use std::mem::{drop, forget};
use std::vec::Vec;
#[derive(Copy, Clone)]
struct SomeStruct {
}
struct AnotherStruct {
x: u8,
y: u8,
z: Vec<u8>
}
impl Clone for AnotherStruct {
fn clone(& self) -> AnotherStruct {
AnotherStruct{x: self.x, y: self.y, z: self.z.clone()}
}
}
fn main() {
let s1 = SomeStruct {};
let s2 = s1;
let s3 = &s1;
let mut s4 = s1;
let ref s5 = s1;
drop(s1);
drop(s2);
drop(s3);
drop(s4);
drop(s5);
forget(s1);
forget(s2);
forget(s3);
forget(s4);
forget(s5);
let a1 = AnotherStruct {x: 255, y: 0, z: vec![1, 2, 3]};
let a2 = &a1;
let mut a3 = a1.clone();
let ref a4 = a1;
let a5 = a1.clone();
drop(a2);
drop(a3);
drop(a4);
drop(a5);
forget(a2);
let a3 = &a1;
forget(a3);
forget(a4);
let a5 = a1.clone();
forget(a5);
}

84
tests/ui/drop_forget_copy.stderr Normal file
View file

@ -0,0 +1,84 @@
error: calls to `std::mem::drop` with a value that implements Copy. Dropping a copy leaves the original intact.
--> $DIR/drop_forget_copy.rs:33:5
|
33 | drop(s1);
| ^^^^^^^^
|
note: lint level defined here
--> $DIR/drop_forget_copy.rs:4:9
|
4 | #![deny(drop_copy, forget_copy)]
| ^^^^^^^^^
note: argument has type SomeStruct
--> $DIR/drop_forget_copy.rs:33:10
|
33 | drop(s1);
| ^^
error: calls to `std::mem::drop` with a value that implements Copy. Dropping a copy leaves the original intact.
--> $DIR/drop_forget_copy.rs:34:5
|
34 | drop(s2);
| ^^^^^^^^
|
note: argument has type SomeStruct
--> $DIR/drop_forget_copy.rs:34:10
|
34 | drop(s2);
| ^^
error: calls to `std::mem::drop` with a value that implements Copy. Dropping a copy leaves the original intact.
--> $DIR/drop_forget_copy.rs:36:5
|
36 | drop(s4);
| ^^^^^^^^
|
note: argument has type SomeStruct
--> $DIR/drop_forget_copy.rs:36:10
|
36 | drop(s4);
| ^^
error: calls to `std::mem::forget` with a value that implements Copy. Forgetting a copy leaves the original intact.
--> $DIR/drop_forget_copy.rs:39:5
|
39 | forget(s1);
| ^^^^^^^^^^
|
note: lint level defined here
--> $DIR/drop_forget_copy.rs:4:20
|
4 | #![deny(drop_copy, forget_copy)]
| ^^^^^^^^^^^
note: argument has type SomeStruct
--> $DIR/drop_forget_copy.rs:39:12
|
39 | forget(s1);
| ^^
error: calls to `std::mem::forget` with a value that implements Copy. Forgetting a copy leaves the original intact.
--> $DIR/drop_forget_copy.rs:40:5
|
40 | forget(s2);
| ^^^^^^^^^^
|
note: argument has type SomeStruct
--> $DIR/drop_forget_copy.rs:40:12
|
40 | forget(s2);
| ^^
error: calls to `std::mem::forget` with a value that implements Copy. Forgetting a copy leaves the original intact.
--> $DIR/drop_forget_copy.rs:42:5
|
42 | forget(s4);
| ^^^^^^^^^^
|
note: argument has type SomeStruct
--> $DIR/drop_forget_copy.rs:42:12
|
42 | forget(s4);
| ^^
error: aborting due to 6 previous errors

36
tests/ui/drop_forget_ref.stderr
View file

@ -1,4 +1,4 @@
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:12:5
|
12 | drop(&SomeStruct);
@ -15,7 +15,7 @@ note: argument has type &SomeStruct
12 | drop(&SomeStruct);
| ^^^^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:13:5
|
13 | forget(&SomeStruct);
@ -32,7 +32,7 @@ note: argument has type &SomeStruct
13 | forget(&SomeStruct);
| ^^^^^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:16:5
|
16 | drop(&owned1);
@ -44,7 +44,7 @@ note: argument has type &SomeStruct
16 | drop(&owned1);
| ^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:17:5
|
17 | drop(&&owned1);
@ -56,7 +56,7 @@ note: argument has type &&SomeStruct
17 | drop(&&owned1);
| ^^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:18:5
|
18 | drop(&mut owned1);
@ -68,7 +68,7 @@ note: argument has type &mut SomeStruct
18 | drop(&mut owned1);
| ^^^^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:21:5
|
21 | forget(&owned2);
@ -80,7 +80,7 @@ note: argument has type &SomeStruct
21 | forget(&owned2);
| ^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:22:5
|
22 | forget(&&owned2);
@ -92,7 +92,7 @@ note: argument has type &&SomeStruct
22 | forget(&&owned2);
| ^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:23:5
|
23 | forget(&mut owned2);
@ -104,7 +104,7 @@ note: argument has type &mut SomeStruct
23 | forget(&mut owned2);
| ^^^^^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:27:5
|
27 | drop(reference1);
@ -116,7 +116,7 @@ note: argument has type &SomeStruct
27 | drop(reference1);
| ^^^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:28:5
|
28 | forget(&*reference1);
@ -128,7 +128,7 @@ note: argument has type &SomeStruct
28 | forget(&*reference1);
| ^^^^^^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:31:5
|
31 | drop(reference2);
@ -140,7 +140,7 @@ note: argument has type &mut SomeStruct
31 | drop(reference2);
| ^^^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:33:5
|
33 | forget(reference3);
@ -152,7 +152,7 @@ note: argument has type &mut SomeStruct
33 | forget(reference3);
| ^^^^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:36:5
|
36 | drop(reference4);
@ -164,7 +164,7 @@ note: argument has type &SomeStruct
36 | drop(reference4);
| ^^^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:37:5
|
37 | forget(reference4);
@ -176,7 +176,7 @@ note: argument has type &SomeStruct
37 | forget(reference4);
| ^^^^^^^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:42:5
|
42 | drop(&val);
@ -188,7 +188,7 @@ note: argument has type &T
42 | drop(&val);
| ^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:48:5
|
48 | forget(&val);
@ -200,7 +200,7 @@ note: argument has type &T
48 | forget(&val);
| ^^^^
error: call to `std::mem::drop` with a reference argument. Dropping a reference does nothing
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing.
--> $DIR/drop_forget_ref.rs:56:5
|
56 | std::mem::drop(&SomeStruct);
@ -212,7 +212,7 @@ note: argument has type &SomeStruct
56 | std::mem::drop(&SomeStruct);
| ^^^^^^^^^^^
error: call to `std::mem::forget` with a reference argument. Forgetting a reference does nothing
error: calls to `std::mem::forget` with a reference instead of an owned value. Forgetting a reference does nothing.
--> $DIR/drop_forget_ref.rs:59:5
|
59 | std::mem::forget(&SomeStruct);

2
tests/ui/mem_forget.rs
View file

@ -1,6 +1,7 @@
#![feature(plugin)]
#![plugin(clippy)]
use std::sync::Arc;
use std::rc::Rc;
@ -8,6 +9,7 @@ use std::mem::forget as forgetSomething;
use std::mem as memstuff;
#[deny(mem_forget)]
#[allow(forget_copy)]
fn main() {
let five: i32 = 5;
forgetSomething(five);

16
tests/ui/mem_forget.stderr
View file

@ -1,25 +1,25 @@
error: usage of mem::forget on Drop type
--> $DIR/mem_forget.rs:16:5
--> $DIR/mem_forget.rs:18:5
|
16 | memstuff::forget(six);
18 | memstuff::forget(six);
| ^^^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/mem_forget.rs:10:8
--> $DIR/mem_forget.rs:11:8
|
10 | #[deny(mem_forget)]
11 | #[deny(mem_forget)]
| ^^^^^^^^^^
error: usage of mem::forget on Drop type
--> $DIR/mem_forget.rs:20:5
--> $DIR/mem_forget.rs:22:5
|
20 | std::mem::forget(seven);
22 | std::mem::forget(seven);
| ^^^^^^^^^^^^^^^^^^^^^^^
error: usage of mem::forget on Drop type
--> $DIR/mem_forget.rs:24:5
--> $DIR/mem_forget.rs:26:5
|
24 | forgetSomething(eight);
26 | forgetSomething(eight);
| ^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 3 previous errors