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>) {}
```
This commit is contained in:
Michael Howell 2021-07-03 11:20:01 -07:00
parent 18073052d8
commit dbd4fd5716
3 changed files with 116 additions and 1 deletions

View File

@ -37,10 +37,47 @@
"bounds of the form `T: Drop` are useless"
}
declare_lint! {
/// The `dyn_drop` lint checks for trait objects with `std::ops::Drop`.
///
/// ### Example
///
/// ```rust
/// fn foo(_x: Box<dyn Drop>) {}
/// ```
///
/// {{produces}}
///
/// ### 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>) {}
/// ```
pub DYN_DROP,
Warn,
"trait objects of the form `dyn Drop` are useless"
}
declare_lint_pass!(
/// Lint for bounds of the form `T: Drop`, which usually
/// indicate an attempt to emulate `std::mem::needs_drop`.
DropTraitConstraints => [DROP_BOUNDS]
DropTraitConstraints => [DROP_BOUNDS, DYN_DROP]
);
impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
@ -75,4 +112,28 @@ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
}
}
}
fn check_ty(&mut self, cx: &LateContext<'_>, ty: &'tcx hir::Ty<'tcx>) {
let bounds = match &ty.kind {
hir::TyKind::TraitObject(bounds, _lifetime, _syntax) => bounds,
_ => return,
};
for bound in &bounds[..] {
let def_id = bound.trait_ref.trait_def_id();
if cx.tcx.lang_items().drop_trait() == def_id {
cx.struct_span_lint(DYN_DROP, bound.span, |lint| {
let needs_drop = match cx.tcx.get_diagnostic_item(sym::needs_drop) {
Some(needs_drop) => needs_drop,
None => return,
};
let msg = format!(
"types that do not implement `Drop` can still have drop glue, consider \
instead using `{}` to detect whether a type is trivially dropped",
cx.tcx.def_path_str(needs_drop)
);
lint.build(&msg).emit()
});
}
}
}
}

View File

@ -0,0 +1,16 @@
#![deny(dyn_drop)]
#![allow(bare_trait_objects)]
fn foo(_: Box<dyn Drop>) {} //~ ERROR
fn bar(_: &dyn Drop) {} //~ERROR
fn baz(_: *mut Drop) {} //~ ERROR
struct Foo {
_x: Box<dyn Drop> //~ ERROR
}
trait Bar {
type T: ?Sized;
}
struct Baz {}
impl Bar for Baz {
type T = dyn Drop; //~ ERROR
}
fn main() {}

View File

@ -0,0 +1,38 @@
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
--> $DIR/dyn-drop.rs:3:19
|
LL | fn foo(_: Box<dyn Drop>) {}
| ^^^^
|
note: the lint level is defined here
--> $DIR/dyn-drop.rs:1:9
|
LL | #![deny(dyn_drop)]
| ^^^^^^^^
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
--> $DIR/dyn-drop.rs:4:16
|
LL | fn bar(_: &dyn Drop) {}
| ^^^^
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
--> $DIR/dyn-drop.rs:5:16
|
LL | fn baz(_: *mut Drop) {}
| ^^^^
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
--> $DIR/dyn-drop.rs:7:15
|
LL | _x: Box<dyn Drop>
| ^^^^
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
--> $DIR/dyn-drop.rs:14:16
|
LL | type T = dyn Drop;
| ^^^^
error: aborting due to 5 previous errors