Skip to content

Commit 1deef26

Browse files
committed
Improve wording of the drop_bounds lint
1 parent 5d34076 commit 1deef26

File tree

1 file changed

+20
-16
lines changed

1 file changed

+20
-16
lines changed

compiler/rustc_lint/src/traits.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,27 @@ declare_lint! {
1818
///
1919
/// ### Explanation
2020
///
21-
/// `Drop` bounds do not really accomplish anything. A type may have
22-
/// compiler-generated drop glue without implementing the `Drop` trait
23-
/// itself. The `Drop` trait also only has one method, `Drop::drop`, and
24-
/// that function is by fiat not callable in user code. So there is really
25-
/// no use case for using `Drop` in trait bounds.
21+
/// A generic trait bound of the form `T: Drop` is most likely misleading
22+
/// and not what the programmer intended (they probably should have used
23+
/// `std::mem::needs_drop` instead).
2624
///
27-
/// The most likely use case of a drop bound is to distinguish between
28-
/// types that have destructors and types that don't. Combined with
29-
/// specialization, a naive coder would write an implementation that
30-
/// assumed a type could be trivially dropped, then write a specialization
31-
/// for `T: Drop` that actually calls the destructor. Except that doing so
32-
/// is not correct; String, for example, doesn't actually implement Drop,
33-
/// but because String contains a Vec, assuming it can be trivially dropped
34-
/// will leak memory.
25+
/// `Drop` bounds do not actually indicate whether a type can be trivially
26+
/// dropped or not, because a composite type containing `Drop` types does
27+
/// not necessarily implement `Drop` itself. Naïvely, one might be tempted
28+
/// to write an implementation that assumes that a type can be trivially
29+
/// dropped while also supplying a specialization for `T: Drop` that
30+
/// actually calls the destructor. However, this breaks down e.g. when `T`
31+
/// is `String`, which does not implement `Drop` itself but contains a
32+
/// `Vec`, which does implement `Drop`, so assuming `T` can be trivially
33+
/// dropped would lead to a memory leak here.
34+
///
35+
/// Furthermore, the `Drop` trait only contains one method, `Drop::drop`,
36+
/// which may not be called explicitly in user code (`E0040`), so there is
37+
/// really no use case for using `Drop` in trait bounds, save perhaps for
38+
/// some obscure corner cases, which can use `#[allow(drop_bounds)]`.
3539
pub DROP_BOUNDS,
3640
Warn,
37-
"bounds of the form `T: Drop` are useless"
41+
"bounds of the form `T: Drop` are most likely incorrect"
3842
}
3943

4044
declare_lint_pass!(
@@ -65,8 +69,8 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
6569
None => return,
6670
};
6771
let msg = format!(
68-
"bounds on `{}` are useless, consider instead \
69-
using `{}` to detect if a type has a destructor",
72+
"bounds on `{}` are most likely incorrect, consider instead \
73+
using `{}` to detect whether a type can be trivially dropped",
7074
predicate,
7175
cx.tcx.def_path_str(needs_drop)
7276
);

0 commit comments

Comments
 (0)