Skip to content

Commit dbd4fd5

Browse files
committed
feat(rustc_lint): add dyn_drop
Based on the conversation in #86747. Explanation ----------- A trait object bound of the form `dyn Drop` is most likely misleading and not what the programmer intended. `Drop` bounds do not actually indicate whether a type can be trivially dropped or not, because a composite type containing `Drop` types does not necessarily implement `Drop` itself. Naïvely, one might be tempted to write a deferred drop system, to pull cleaning up memory out of a latency-sensitive code path, using `dyn Drop` trait objects. However, this breaks down e.g. when `T` is `String`, which does not implement `Drop`, but should probably be accepted. To write a trait object bound that accepts anything, use a placeholder trait with a blanket implementation. ```rust trait Placeholder {} impl<T> Placeholder for T {} fn foo(_x: Box<dyn Placeholder>) {} ```
1 parent 1807305 commit dbd4fd5

File tree

3 files changed

+116
-1
lines changed

3 files changed

+116
-1
lines changed

compiler/rustc_lint/src/traits.rs

+62-1
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,47 @@ declare_lint! {
3737
"bounds of the form `T: Drop` are useless"
3838
}
3939

40+
declare_lint! {
41+
/// The `dyn_drop` lint checks for trait objects with `std::ops::Drop`.
42+
///
43+
/// ### Example
44+
///
45+
/// ```rust
46+
/// fn foo(_x: Box<dyn Drop>) {}
47+
/// ```
48+
///
49+
/// {{produces}}
50+
///
51+
/// ### Explanation
52+
///
53+
/// A trait object bound of the form `dyn Drop` is most likely misleading
54+
/// and not what the programmer intended.
55+
///
56+
/// `Drop` bounds do not actually indicate whether a type can be trivially
57+
/// dropped or not, because a composite type containing `Drop` types does
58+
/// not necessarily implement `Drop` itself. Naïvely, one might be tempted
59+
/// to write a deferred drop system, to pull cleaning up memory out of a
60+
/// latency-sensitive code path, using `dyn Drop` trait objects. However,
61+
/// this breaks down e.g. when `T` is `String`, which does not implement
62+
/// `Drop`, but should probably be accepted.
63+
///
64+
/// To write a trait object bound that accepts anything, use a placeholder
65+
/// trait with a blanket implementation.
66+
///
67+
/// ```rust
68+
/// trait Placeholder {}
69+
/// impl<T> Placeholder for T {}
70+
/// fn foo(_x: Box<dyn Placeholder>) {}
71+
/// ```
72+
pub DYN_DROP,
73+
Warn,
74+
"trait objects of the form `dyn Drop` are useless"
75+
}
76+
4077
declare_lint_pass!(
4178
/// Lint for bounds of the form `T: Drop`, which usually
4279
/// indicate an attempt to emulate `std::mem::needs_drop`.
43-
DropTraitConstraints => [DROP_BOUNDS]
80+
DropTraitConstraints => [DROP_BOUNDS, DYN_DROP]
4481
);
4582

4683
impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
@@ -75,4 +112,28 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
75112
}
76113
}
77114
}
115+
116+
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) {
117+
let bounds = match &ty.kind {
118+
hir::TyKind::TraitObject(bounds, _lifetime, _syntax) => bounds,
119+
_ => return,
120+
};
121+
for bound in &bounds[..] {
122+
let def_id = bound.trait_ref.trait_def_id();
123+
if cx.tcx.lang_items().drop_trait() == def_id {
124+
cx.struct_span_lint(DYN_DROP, bound.span, |lint| {
125+
let needs_drop = match cx.tcx.get_diagnostic_item(sym::needs_drop) {
126+
Some(needs_drop) => needs_drop,
127+
None => return,
128+
};
129+
let msg = format!(
130+
"types that do not implement `Drop` can still have drop glue, consider \
131+
instead using `{}` to detect whether a type is trivially dropped",
132+
cx.tcx.def_path_str(needs_drop)
133+
);
134+
lint.build(&msg).emit()
135+
});
136+
}
137+
}
138+
}
78139
}

src/test/ui/dyn-drop/dyn-drop.rs

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#![deny(dyn_drop)]
2+
#![allow(bare_trait_objects)]
3+
fn foo(_: Box<dyn Drop>) {} //~ ERROR
4+
fn bar(_: &dyn Drop) {} //~ERROR
5+
fn baz(_: *mut Drop) {} //~ ERROR
6+
struct Foo {
7+
_x: Box<dyn Drop> //~ ERROR
8+
}
9+
trait Bar {
10+
type T: ?Sized;
11+
}
12+
struct Baz {}
13+
impl Bar for Baz {
14+
type T = dyn Drop; //~ ERROR
15+
}
16+
fn main() {}

src/test/ui/dyn-drop/dyn-drop.stderr

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
2+
--> $DIR/dyn-drop.rs:3:19
3+
|
4+
LL | fn foo(_: Box<dyn Drop>) {}
5+
| ^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/dyn-drop.rs:1:9
9+
|
10+
LL | #![deny(dyn_drop)]
11+
| ^^^^^^^^
12+
13+
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
14+
--> $DIR/dyn-drop.rs:4:16
15+
|
16+
LL | fn bar(_: &dyn Drop) {}
17+
| ^^^^
18+
19+
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
20+
--> $DIR/dyn-drop.rs:5:16
21+
|
22+
LL | fn baz(_: *mut Drop) {}
23+
| ^^^^
24+
25+
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
26+
--> $DIR/dyn-drop.rs:7:15
27+
|
28+
LL | _x: Box<dyn Drop>
29+
| ^^^^
30+
31+
error: types that do not implement `Drop` can still have drop glue, consider instead using `std::mem::needs_drop` to detect whether a type is trivially dropped
32+
--> $DIR/dyn-drop.rs:14:16
33+
|
34+
LL | type T = dyn Drop;
35+
| ^^^^
36+
37+
error: aborting due to 5 previous errors
38+

0 commit comments

Comments
 (0)