diff --git a/compiler/rustc_lint/src/traits.rs b/compiler/rustc_lint/src/traits.rs index e632f29e672c0..1c9f23932d17e 100644 --- a/compiler/rustc_lint/src/traits.rs +++ b/compiler/rustc_lint/src/traits.rs @@ -18,23 +18,27 @@ declare_lint! { /// /// ### Explanation /// - /// `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. + /// A generic trait bound of the form `T: Drop` is most likely misleading + /// and not what the programmer intended (they probably should have used + /// `std::mem::needs_drop` instead). /// - /// The most likely use case of a drop bound is to distinguish between - /// types that have destructors and types that don't. Combined with - /// specialization, a naive coder would write an implementation that - /// assumed a type could be trivially dropped, then write a specialization - /// for `T: Drop` that actually calls the destructor. Except that doing so - /// is not correct; String, for example, doesn't actually implement Drop, - /// but because String contains a Vec, assuming it can be trivially dropped - /// will leak memory. + /// `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 an implementation that assumes that a type can be trivially + /// dropped while also supplying a specialization for `T: Drop` that + /// actually calls the destructor. However, this breaks down e.g. when `T` + /// is `String`, which does not implement `Drop` itself but contains a + /// `Vec`, which does implement `Drop`, so assuming `T` can be trivially + /// dropped would lead to a memory leak here. + /// + /// Furthermore, the `Drop` trait only contains one method, `Drop::drop`, + /// which may not be called explicitly in user code (`E0040`), so there is + /// really no use case for using `Drop` in trait bounds, save perhaps for + /// some obscure corner cases, which can use `#[allow(drop_bounds)]`. pub DROP_BOUNDS, Warn, - "bounds of the form `T: Drop` are useless" + "bounds of the form `T: Drop` are most likely incorrect" } declare_lint_pass!( @@ -65,8 +69,8 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints { None => return, }; let msg = format!( - "bounds on `{}` are useless, consider instead \ - using `{}` to detect if a type has a destructor", + "bounds on `{}` are most likely incorrect, consider instead \ + using `{}` to detect whether a type can be trivially dropped", predicate, cx.tcx.def_path_str(needs_drop) ); diff --git a/src/test/ui/drop-bounds/drop-bounds.stderr b/src/test/ui/drop-bounds/drop-bounds.stderr index 15ba4c9a64989..3ffb855a55dc8 100644 --- a/src/test/ui/drop-bounds/drop-bounds.stderr +++ b/src/test/ui/drop-bounds/drop-bounds.stderr @@ -1,4 +1,4 @@ -error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:2:11 | LL | fn foo<T: Drop>() {} @@ -10,37 +10,37 @@ note: the lint level is defined here LL | #![deny(drop_bounds)] | ^^^^^^^^^^^ -error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `U: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:5:8 | LL | U: Drop, | ^^^^ -error: bounds on `impl Drop: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `impl Drop: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:8:17 | LL | fn baz(_x: impl Drop) {} | ^^^^ -error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:9:15 | LL | struct Foo<T: Drop> { | ^^^^ -error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `U: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:12:24 | LL | struct Bar<U> where U: Drop { | ^^^^ -error: bounds on `Self: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `Self: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:15:12 | LL | trait Baz: Drop { | ^^^^ -error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor +error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped --> $DIR/drop-bounds.rs:17:9 | LL | impl<T: Drop> Baz for T {