Skip to content

Commit 6c0f231

Browse files
committed
Add a lint to warn on T: Drop bounds
**What it does:** Checks for generics with `std::ops::Drop` as bounds. **Why is this bad?** `Drop` bounds do not really accomplish anything. A type may have compiler-generated drop glue without implementing the `Drop` trait itself. The `Drop` trait also only has one method, `Drop::drop`, and that function is by fiat not callable in user code. So there is really no use case for using `Drop` in trait bounds. **Known problems:** None. **Example:** ```rust fn foo<T: Drop>() {} ```
1 parent 32ee306 commit 6c0f231

File tree

5 files changed

+84
-0
lines changed

5 files changed

+84
-0
lines changed

clippy_lints/src/drop_bounds.rs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
use crate::utils::{match_def_path, paths, span_lint};
2+
use if_chain::if_chain;
3+
use rustc::hir::*;
4+
use rustc::hir::GenericBound::Trait;
5+
use rustc::lint::{LateLintPass, LintArray, LintPass};
6+
use rustc::{declare_tool_lint, lint_array};
7+
8+
/// **What it does:** Checks for generics with `std::ops::Drop` as bounds.
9+
///
10+
/// **Why is this bad?** `Drop` bounds do not really accomplish anything.
11+
/// A type may have compiler-generated drop glue without implementing the
12+
/// `Drop` trait itself. The `Drop` trait also only has one method,
13+
/// `Drop::drop`, and that function is by fiat not callable in user code.
14+
/// So there is really no use case for using `Drop` in trait bounds.
15+
///
16+
/// **Known problems:** None.
17+
///
18+
/// **Example:**
19+
/// ```rust
20+
/// fn foo<T: Drop>() {}
21+
/// ```
22+
declare_clippy_lint! {
23+
pub DROP_BOUNDS,
24+
correctness,
25+
"Bounds of the form `T: Drop` are useless"
26+
}
27+
28+
const DROP_BOUNDS_SUMMARY: &str = "Bounds of the form `T: Drop` are usless. \
29+
Use `std::mem::needs_drop` to detect if a type has drop glue.";
30+
31+
pub struct Pass;
32+
33+
impl LintPass for Pass {
34+
fn get_lints(&self) -> LintArray {
35+
lint_array!(DROP_BOUNDS)
36+
}
37+
38+
fn name(&self) -> &'static str {
39+
"DropBounds"
40+
}
41+
}
42+
43+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
44+
fn check_generic_param(&mut self, cx: &rustc::lint::LateContext<'a, 'tcx>, p: &'tcx GenericParam) {
45+
for bound in &p.bounds {
46+
if_chain! {
47+
if let Trait(t, _) = bound;
48+
if let Some(def_id) = t.trait_ref.path.def.opt_def_id();
49+
if match_def_path(cx.tcx, def_id, &paths::DROP_TRAIT);
50+
then {
51+
span_lint(
52+
cx,
53+
DROP_BOUNDS,
54+
t.span,
55+
&DROP_BOUNDS_SUMMARY.to_string()
56+
);
57+
}
58+
}
59+
}
60+
}
61+
}

clippy_lints/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ pub mod derive;
144144
pub mod doc;
145145
pub mod double_comparison;
146146
pub mod double_parens;
147+
pub mod drop_bounds;
147148
pub mod drop_forget_ref;
148149
pub mod duration_subsec;
149150
pub mod else_if_without_else;
@@ -467,6 +468,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
467468
reg.register_late_lint_pass(box derive::Derive);
468469
reg.register_late_lint_pass(box types::CharLitAsU8);
469470
reg.register_late_lint_pass(box vec::Pass);
471+
reg.register_late_lint_pass(box drop_bounds::Pass);
470472
reg.register_late_lint_pass(box drop_forget_ref::Pass);
471473
reg.register_late_lint_pass(box empty_enum::EmptyEnum);
472474
reg.register_late_lint_pass(box types::AbsurdExtremeComparisons);
@@ -653,6 +655,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
653655
derive::DERIVE_HASH_XOR_EQ,
654656
double_comparison::DOUBLE_COMPARISONS,
655657
double_parens::DOUBLE_PARENS,
658+
drop_bounds::DROP_BOUNDS,
656659
drop_forget_ref::DROP_COPY,
657660
drop_forget_ref::DROP_REF,
658661
drop_forget_ref::FORGET_COPY,
@@ -1017,6 +1020,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
10171020
copies::IFS_SAME_COND,
10181021
copies::IF_SAME_THEN_ELSE,
10191022
derive::DERIVE_HASH_XOR_EQ,
1023+
drop_bounds::DROP_BOUNDS,
10201024
drop_forget_ref::DROP_COPY,
10211025
drop_forget_ref::DROP_REF,
10221026
drop_forget_ref::FORGET_COPY,

clippy_lints/src/utils/paths.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "der
2424
pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"];
2525
pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
2626
pub const DROP: [&str; 3] = ["core", "mem", "drop"];
27+
pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"];
2728
pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
2829
pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
2930
pub const FMT_ARGUMENTS_NEWV1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];

tests/ui/drop_bounds.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
fn foo<T: Drop>() {}
2+
struct K;
3+
impl Drop for K {
4+
fn drop(&mut self) {}
5+
}
6+
fn main() {
7+
foo::<K>();
8+
}

tests/ui/drop_bounds.stderr

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: Bounds of the form `T: Drop` are usless. Use `std::mem::needs_drop` to detect if a type has drop glue.
2+
--> $DIR/drop_bounds.rs:1:11
3+
|
4+
LL | fn foo<T: Drop>() {}
5+
| ^^^^
6+
|
7+
= note: #[deny(clippy::drop_bounds)] on by default
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)