Skip to content

Commit 252d6fc

Browse files
committed
Add Drop::pin_drop for pinned drops
Fixes #122630 (accidentally).
1 parent b00998a commit 252d6fc

32 files changed

+896
-218
lines changed

Cargo.lock

Lines changed: 226 additions & 152 deletions
Large diffs are not rendered by default.

compiler/rustc_hir_analysis/messages.ftl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ hir_analysis_coercion_between_struct_same_note = expected coercion between the s
117117
118118
hir_analysis_coercion_between_struct_single_note = expected a single field to be coerced, none found
119119
120+
hir_analysis_conflict_impl_drop_and_pin_drop = conflict implementation of `Drop::drop` and `Drop::pin_drop`
121+
.drop_label = `drop(&mut self)` implemented here
122+
.pin_drop_label = `pin_drop(&pin mut self)` implemented here
123+
.suggestion = remove this implementation
124+
120125
hir_analysis_const_bound_for_non_const_trait = `{$modifier}` can only be applied to `const` traits
121126
.label = can't be applied to `{$trait_name}`
122127
.note = `{$trait_name}` can't be used with `{$modifier}` because it isn't `const`

compiler/rustc_hir_analysis/src/check/always_applicable.rs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
use rustc_data_structures::fx::FxHashSet;
88
use rustc_errors::codes::*;
99
use rustc_errors::{ErrorGuaranteed, struct_span_code_err};
10+
use rustc_hir as hir;
1011
use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
1112
use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
1213
use rustc_middle::span_bug;
1314
use rustc_middle::ty::util::CheckRegions;
1415
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypingMode};
16+
use rustc_span::sym;
1517
use rustc_trait_selection::regions::InferCtxtRegionExt;
1618
use rustc_trait_selection::traits::{self, ObligationCtxt};
1719

@@ -70,7 +72,11 @@ pub(crate) fn check_drop_impl(
7072
drop_impl_did,
7173
adt_def.did(),
7274
adt_to_impl_args,
73-
)
75+
)?;
76+
77+
check_drop_xor_pin_drop(tcx, drop_impl_did)?;
78+
79+
Ok(())
7480
}
7581
_ => {
7682
span_bug!(tcx.def_span(drop_impl_did), "incoherent impl of Drop");
@@ -294,3 +300,58 @@ fn ensure_impl_predicates_are_implied_by_item_defn<'tcx>(
294300

295301
Ok(())
296302
}
303+
304+
/// This function checks at least and at most one of `Drop::drop` and `Drop::pin_drop` is implemented.
305+
fn check_drop_xor_pin_drop<'tcx>(
306+
tcx: TyCtxt<'tcx>,
307+
drop_impl_did: LocalDefId,
308+
) -> Result<(), ErrorGuaranteed> {
309+
let item_impl = tcx.hir_expect_item(drop_impl_did).expect_impl();
310+
let mut drop_span = None;
311+
let mut pin_drop_span = None;
312+
for &impl_item_id in item_impl.items {
313+
let impl_item = tcx.hir_impl_item(impl_item_id);
314+
if let hir::ImplItemKind::Fn(fn_sig, _) = impl_item.kind {
315+
match impl_item.ident.name {
316+
sym::drop => drop_span = Some(fn_sig.span),
317+
sym::pin_drop => pin_drop_span = Some(fn_sig.span),
318+
_ => {}
319+
}
320+
}
321+
}
322+
323+
match (drop_span, pin_drop_span) {
324+
(None, None) => {
325+
if tcx.features().pin_ergonomics() {
326+
return Err(tcx.dcx().emit_err(crate::errors::MissingOneOfTraitItem {
327+
span: tcx.def_span(drop_impl_did),
328+
note: None,
329+
missing_items_msg: "drop`, `pin_drop".to_string(),
330+
}));
331+
} else {
332+
return Err(tcx
333+
.dcx()
334+
.span_delayed_bug(tcx.def_span(drop_impl_did), "missing `Drop::drop`"));
335+
}
336+
}
337+
// FIXME(pin_ergonomics): reject `Drop::drop` for types that support pin-projection.
338+
(Some(_span), None) => {}
339+
(None, Some(span)) => {
340+
if !tcx.features().pin_ergonomics() {
341+
return Err(tcx.dcx().span_delayed_bug(
342+
span,
343+
"`Drop::pin_drop` should be guarded by the library feature gate",
344+
));
345+
}
346+
// FIXME(pin_ergonomics): reject `Drop::pin_drop` for types that don't support pin-projection.
347+
}
348+
(Some(drop_span), Some(pin_drop_span)) => {
349+
return Err(tcx.dcx().emit_err(crate::errors::ConflictImplDropAndPinDrop {
350+
span: tcx.def_span(drop_impl_did),
351+
drop_span,
352+
pin_drop_span,
353+
}));
354+
}
355+
}
356+
Ok(())
357+
}

compiler/rustc_hir_analysis/src/check/check.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use rustc_middle::ty::{
2626
TypeVisitable, TypeVisitableExt, fold_regions,
2727
};
2828
use rustc_session::lint::builtin::UNINHABITED_STATIC;
29+
use rustc_span::sym;
2930
use rustc_target::spec::{AbiMap, AbiMapping};
3031
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
3132
use rustc_trait_selection::error_reporting::traits::on_unimplemented::OnUnimplementedDirective;
@@ -1260,6 +1261,19 @@ fn check_impl_items_against_trait<'tcx>(
12601261
// Unmarked default bodies are considered stable (at least for now).
12611262
EvalResult::Allow | EvalResult::Unmarked => {}
12621263
}
1264+
1265+
// FIXME(pin_ergonomics): hack for `Drop::drop` (see also the FIXME comments on top of
1266+
// the `Drop` trait)
1267+
// - When `pin_ergonomics` is not enabled, we manually check if `Drop::drop` is missing.
1268+
// - When `pin_ergonomics` is enabled, the presentness and exclusiveness of
1269+
// `Drop::drop` and `Drop::pin_drop` will be handled in
1270+
// `super::always_applicable::check_drop_xor_pin_drop`.
1271+
if !tcx.features().pin_ergonomics()
1272+
&& tcx.is_lang_item(trait_ref.def_id, hir::LangItem::Drop)
1273+
&& tcx.item_name(trait_item_id) == sym::drop
1274+
{
1275+
missing_items.push(tcx.associated_item(trait_item_id));
1276+
}
12631277
}
12641278

12651279
if let Some(required_items) = &must_implement_one_of {

compiler/rustc_hir_analysis/src/check/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ use rustc_middle::ty::{
9292
use rustc_middle::{bug, span_bug};
9393
use rustc_session::parse::feature_err;
9494
use rustc_span::def_id::CRATE_DEF_ID;
95-
use rustc_span::{BytePos, DUMMY_SP, Ident, Span, Symbol, kw, sym};
95+
use rustc_span::{BytePos, DUMMY_SP, Ident, Span, Symbol, kw};
9696
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
9797
use rustc_trait_selection::error_reporting::infer::ObligationCauseExt as _;
9898
use rustc_trait_selection::error_reporting::traits::suggestions::ReturnsVisitor;

compiler/rustc_hir_analysis/src/errors.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1707,3 +1707,14 @@ pub(crate) struct AsyncDropWithoutSyncDrop {
17071707
#[primary_span]
17081708
pub span: Span,
17091709
}
1710+
1711+
#[derive(Diagnostic)]
1712+
#[diag(hir_analysis_conflict_impl_drop_and_pin_drop)]
1713+
pub(crate) struct ConflictImplDropAndPinDrop {
1714+
#[primary_span]
1715+
pub span: Span,
1716+
#[label(hir_analysis_drop_label)]
1717+
pub drop_span: Span,
1718+
#[label(hir_analysis_pin_drop_label)]
1719+
pub pin_drop_span: Span,
1720+
}

compiler/rustc_hir_typeck/src/callee.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,13 @@ pub(crate) fn check_legal_trait_for_method_call(
3737
receiver: Option<Span>,
3838
expr_span: Span,
3939
trait_id: DefId,
40-
_body_id: DefId,
40+
body_id: DefId,
4141
) -> Result<(), ErrorGuaranteed> {
42-
if tcx.is_lang_item(trait_id, LangItem::Drop) {
42+
if tcx.is_lang_item(trait_id, LangItem::Drop)
43+
// Allow calling `Drop::pin_drop` in `Drop::drop`
44+
&& let Some(parent) = tcx.opt_parent(body_id)
45+
&& !tcx.is_lang_item(parent, LangItem::Drop)
46+
{
4347
let sugg = if let Some(receiver) = receiver.filter(|s| !s.is_empty()) {
4448
errors::ExplicitDestructorCallSugg::Snippet {
4549
lo: expr_span.shrink_to_lo(),

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,6 +1650,7 @@ symbols! {
16501650
pic,
16511651
pie,
16521652
pin,
1653+
pin_drop,
16531654
pin_ergonomics,
16541655
pin_macro,
16551656
platform_intrinsics,

library/core/src/ops/drop.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use crate::pin::Pin;
2+
13
/// Custom code within the destructor.
24
///
35
/// When a value is no longer needed, Rust will run a "destructor" on that value.
@@ -204,6 +206,13 @@
204206
#[lang = "drop"]
205207
#[stable(feature = "rust1", since = "1.0.0")]
206208
#[rustc_const_unstable(feature = "const_destruct", issue = "133214")]
209+
// FIXME(pin_ergonomics): we don't use
210+
// `#[rustc_must_implement_one_of(drop, pin_drop)]` for better diagnostics, as it emits
211+
// `missing one of: drop, pin_drop` even without the feature `pin_ergonomics` enabled,
212+
// which may surprise users that don't use `pin_ergonomics`.
213+
//
214+
// Alternatively, we manually check if `Drop::drop` is missing when `pin_ergonomics`
215+
// is not enabled. See also `rustc_hir_analysis::check::check::check_impl_items_against_trait`.
207216
pub const trait Drop {
208217
/// Executes the destructor for this type.
209218
///
@@ -237,5 +246,27 @@ pub const trait Drop {
237246
/// [`mem::drop`]: drop
238247
/// [`ptr::drop_in_place`]: crate::ptr::drop_in_place
239248
#[stable(feature = "rust1", since = "1.0.0")]
240-
fn drop(&mut self);
249+
fn drop(&mut self) {
250+
// SAFETY: `self` is pinned till after dropped.
251+
Drop::pin_drop(unsafe { Pin::new_unchecked(self) })
252+
}
253+
254+
/// Execute the destructor for this type, but different to [`Drop::drop`], it requires `self`
255+
/// to be pinned.
256+
///
257+
/// By implementing this method instead of [`Drop::drop`], the receiver type [`Pin<&mut Self>`]
258+
/// makes sure that the value is pinned until it is deallocated (See [`std::pin` module docs] for
259+
/// more details), which enables us to support field projections of `Self` type safely.
260+
///
261+
/// At least and at most one of [`Drop::drop`] and [`Drop::pin_drop`] should be implemented.
262+
///
263+
// FIXME(pin_ergonomics): add requirements: `pin_drop` must be and must only be used
264+
// for types that support pin-projection.
265+
/// [`Drop::drop`]: crate::ops::Drop::drop
266+
/// [`Unpin`]: crate::marker::Unpin
267+
/// [`!Unpin`]: crate::marker::Unpin
268+
/// [`Pin<&mut Self>`]: crate::pin::Pin
269+
/// [`std::pin` module docs]: crate::pin
270+
#[unstable(feature = "pin_ergonomics", issue = "130494")]
271+
fn pin_drop(self: Pin<&mut Self>) {}
241272
}

tests/codegen-units/item-collection/drop-glue-eager.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ struct StructWithDrop {
1010

1111
impl Drop for StructWithDrop {
1212
//~ MONO_ITEM fn <StructWithDrop as std::ops::Drop>::drop
13+
//~ MONO_ITEM fn <StructWithDrop as std::ops::Drop>::pin_drop
1314
fn drop(&mut self) {}
1415
}
1516

@@ -24,6 +25,7 @@ enum EnumWithDrop {
2425

2526
impl Drop for EnumWithDrop {
2627
//~ MONO_ITEM fn <EnumWithDrop as std::ops::Drop>::drop
28+
//~ MONO_ITEM fn <EnumWithDrop as std::ops::Drop>::pin_drop
2729
fn drop(&mut self) {}
2830
}
2931

@@ -34,6 +36,7 @@ enum EnumNoDrop {
3436
// We should be able to monomorphize drops for struct with lifetimes.
3537
impl<'a> Drop for StructWithDropAndLt<'a> {
3638
//~ MONO_ITEM fn <StructWithDropAndLt<'_> as std::ops::Drop>::drop
39+
//~ MONO_ITEM fn <StructWithDropAndLt<'_> as std::ops::Drop>::pin_drop
3740
fn drop(&mut self) {}
3841
}
3942

@@ -52,5 +55,6 @@ struct StructWithLtAndPredicate<'a: 'a> {
5255
// We should be able to monomorphize drops for struct with lifetimes.
5356
impl<'a> Drop for StructWithLtAndPredicate<'a> {
5457
//~ MONO_ITEM fn <StructWithLtAndPredicate<'_> as std::ops::Drop>::drop
58+
//~ MONO_ITEM fn <StructWithLtAndPredicate<'_> as std::ops::Drop>::pin_drop
5559
fn drop(&mut self) {}
5660
}

0 commit comments

Comments
 (0)