Skip to content

feat(rustc_lint): add dyn_drop #86848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion compiler/rustc_lint/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,47 @@ declare_lint! {
"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 {
Expand Down Expand Up @@ -75,4 +112,28 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
}
}
}

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()
});
}
}
}
}
16 changes: 16 additions & 0 deletions src/test/ui/dyn-drop/dyn-drop.rs
Original file line number Diff line number Diff line change
@@ -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() {}
38 changes: 38 additions & 0 deletions src/test/ui/dyn-drop/dyn-drop.stderr
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// anything.

#![deny(rust_2018_compatibility)]
#![allow(dyn_drop)]

macro_rules! foo {
() => {
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/traits/object/issue-33140-traitobject-crate.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// check-pass

#![warn(order_dependent_trait_objects)]
#![allow(dyn_drop)]

// Check that traitobject 0.1.0 compiles

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
warning: conflicting implementations of trait `Trait` for type `(dyn std::marker::Send + std::marker::Sync + 'static)`: (E0119)
--> $DIR/issue-33140-traitobject-crate.rs:85:1
--> $DIR/issue-33140-traitobject-crate.rs:86:1
|
LL | unsafe impl Trait for dyn (::std::marker::Send) + Sync { }
| ------------------------------------------------------ first implementation here
Expand All @@ -15,7 +15,7 @@ LL | #![warn(order_dependent_trait_objects)]
= note: for more information, see issue #56484 <https://github.com/rust-lang/rust/issues/56484>

warning: conflicting implementations of trait `Trait` for type `(dyn std::marker::Send + std::marker::Sync + 'static)`: (E0119)
--> $DIR/issue-33140-traitobject-crate.rs:88:1
--> $DIR/issue-33140-traitobject-crate.rs:89:1
|
LL | unsafe impl Trait for dyn (::std::marker::Send) + Send + Sync { }
| ------------------------------------------------------------- first implementation here
Expand All @@ -27,7 +27,7 @@ LL | unsafe impl Trait for dyn (::std::marker::Sync) + Send { }
= note: for more information, see issue #56484 <https://github.com/rust-lang/rust/issues/56484>

warning: conflicting implementations of trait `Trait` for type `(dyn std::marker::Send + std::marker::Sync + 'static)`: (E0119)
--> $DIR/issue-33140-traitobject-crate.rs:92:1
--> $DIR/issue-33140-traitobject-crate.rs:93:1
|
LL | unsafe impl Trait for dyn (::std::marker::Sync) + Send { }
| ------------------------------------------------------ first implementation here
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/tests/ui/needless_lifetimes.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![warn(clippy::needless_lifetimes)]
#![allow(dead_code, clippy::needless_pass_by_value, clippy::unnecessary_wraps)]
#![allow(dead_code, clippy::needless_pass_by_value, clippy::unnecessary_wraps, dyn_drop)]

fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {}

Expand Down