diff --git a/compiler/rustc_lint/src/traits.rs b/compiler/rustc_lint/src/traits.rs index e632f29e672c0..e713ce7c71bec 100644 --- a/compiler/rustc_lint/src/traits.rs +++ b/compiler/rustc_lint/src/traits.rs @@ -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) {} + /// ``` + /// + /// {{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 Placeholder for T {} + /// fn foo(_x: Box) {} + /// ``` + 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 @@ 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() + }); + } + } + } } diff --git a/src/test/ui/dyn-drop/dyn-drop.rs b/src/test/ui/dyn-drop/dyn-drop.rs new file mode 100644 index 0000000000000..e1668a3f188d5 --- /dev/null +++ b/src/test/ui/dyn-drop/dyn-drop.rs @@ -0,0 +1,16 @@ +#![deny(dyn_drop)] +#![allow(bare_trait_objects)] +fn foo(_: Box) {} //~ ERROR +fn bar(_: &dyn Drop) {} //~ERROR +fn baz(_: *mut Drop) {} //~ ERROR +struct Foo { + _x: Box //~ ERROR +} +trait Bar { + type T: ?Sized; +} +struct Baz {} +impl Bar for Baz { + type T = dyn Drop; //~ ERROR +} +fn main() {} diff --git a/src/test/ui/dyn-drop/dyn-drop.stderr b/src/test/ui/dyn-drop/dyn-drop.stderr new file mode 100644 index 0000000000000..1b1dbc4d12d4c --- /dev/null +++ b/src/test/ui/dyn-drop/dyn-drop.stderr @@ -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) {} + | ^^^^ + | +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 + | ^^^^ + +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 + diff --git a/src/test/ui/dyn-keyword/issue-56327-dyn-trait-in-macro-is-okay.rs b/src/test/ui/dyn-keyword/issue-56327-dyn-trait-in-macro-is-okay.rs index 2b46f57c2e26f..59e7f9a6083ce 100644 --- a/src/test/ui/dyn-keyword/issue-56327-dyn-trait-in-macro-is-okay.rs +++ b/src/test/ui/dyn-keyword/issue-56327-dyn-trait-in-macro-is-okay.rs @@ -10,6 +10,7 @@ // anything. #![deny(rust_2018_compatibility)] +#![allow(dyn_drop)] macro_rules! foo { () => { diff --git a/src/test/ui/traits/object/issue-33140-traitobject-crate.rs b/src/test/ui/traits/object/issue-33140-traitobject-crate.rs index 46b68f1c9fe07..8abd92da362d3 100644 --- a/src/test/ui/traits/object/issue-33140-traitobject-crate.rs +++ b/src/test/ui/traits/object/issue-33140-traitobject-crate.rs @@ -1,6 +1,7 @@ // check-pass #![warn(order_dependent_trait_objects)] +#![allow(dyn_drop)] // Check that traitobject 0.1.0 compiles diff --git a/src/test/ui/traits/object/issue-33140-traitobject-crate.stderr b/src/test/ui/traits/object/issue-33140-traitobject-crate.stderr index 781decb5ae281..77d71360b806b 100644 --- a/src/test/ui/traits/object/issue-33140-traitobject-crate.stderr +++ b/src/test/ui/traits/object/issue-33140-traitobject-crate.stderr @@ -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 @@ -15,7 +15,7 @@ LL | #![warn(order_dependent_trait_objects)] = note: for more information, see issue #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 @@ -27,7 +27,7 @@ LL | unsafe impl Trait for dyn (::std::marker::Sync) + Send { } = note: for more information, see issue #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 diff --git a/src/tools/clippy/tests/ui/needless_lifetimes.rs b/src/tools/clippy/tests/ui/needless_lifetimes.rs index bda0801e51c7f..1d77382bf2cd1 100644 --- a/src/tools/clippy/tests/ui/needless_lifetimes.rs +++ b/src/tools/clippy/tests/ui/needless_lifetimes.rs @@ -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) {}