Skip to content

Commit edafd5c

Browse files
committed
Auto merge of #144537 - frank-king:feature/pin-drop, r=petrochenkov
Add `Drop::pin_drop` for pinned drops This PR is part of the `pin_ergonomics` experiment (the tracking issue is #130494). It allows implementing `Drop` with a pinned `self` receiver, which is required for safe pin-projection. Implementations: - [x] At least and at most one of `drop` and `pin_drop` should be implemented. - [x] No direct call of `drop` or `pin_drop`. They should only be called by the drop glue. - [x] `pin_drop` must and must only be used with types that support pin-projection (i.e. types with `#[pin_v2]`). - [ ] Allows writing `fn drop(&pin mut self)` and desugars to `fn pin_drop(&pin mut self)`. (Will be in the next PRs)
2 parents cb40c25 + fa600c5 commit edafd5c

28 files changed

Lines changed: 747 additions & 41 deletions

File tree

compiler/rustc_attr_parsing/src/attributes/pin_v2.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,5 @@ impl NoArgsAttributeParser for PinV2Parser {
1515
Allow(Target::Struct),
1616
Allow(Target::Union),
1717
]);
18-
const CREATE: fn(Span) -> AttributeKind = |_| AttributeKind::PinV2;
18+
const CREATE: fn(Span) -> AttributeKind = AttributeKind::PinV2;
1919
}

compiler/rustc_hir/src/attrs/data_structures.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ pub enum AttributeKind {
12121212
},
12131213

12141214
/// Represents `#[pin_v2]`
1215-
PinV2,
1215+
PinV2(Span),
12161216

12171217
/// Represents `#[prelude_import]`
12181218
PreludeImport,

compiler/rustc_hir/src/attrs/encode_cross_crate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl AttributeKind {
8686
PatchableFunctionEntry { .. } => Yes,
8787
Path(..) => No,
8888
PatternComplexityLimit { .. } => No,
89-
PinV2 => Yes,
89+
PinV2(..) => Yes,
9090
PreludeImport => No,
9191
ProcMacro => No,
9292
ProcMacroAttribute => No,

compiler/rustc_hir_analysis/src/check/always_applicable.rs

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
1212
use rustc_middle::span_bug;
1313
use rustc_middle::ty::util::CheckRegions;
1414
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt, TypingMode};
15+
use rustc_span::sym;
1516
use rustc_trait_selection::regions::InferCtxtRegionExt;
1617
use rustc_trait_selection::traits::{self, ObligationCtxt};
1718

@@ -72,7 +73,11 @@ pub(crate) fn check_drop_impl(
7273
drop_impl_did,
7374
adt_def.did(),
7475
adt_to_impl_args,
75-
)
76+
)?;
77+
78+
check_drop_xor_pin_drop(tcx, adt_def.did(), drop_impl_did)?;
79+
80+
Ok(())
7681
}
7782
_ => {
7883
span_bug!(tcx.def_span(drop_impl_did), "incoherent impl of Drop");
@@ -363,3 +368,68 @@ fn ensure_impl_predicates_are_implied_by_item_defn<'tcx>(
363368

364369
Ok(())
365370
}
371+
372+
/// This function checks at least and at most one of `Drop::drop` and `Drop::pin_drop` is implemented.
373+
/// It also checks that `Drop::pin_drop` must be implemented if `#[pin_v2]` is present on the type.
374+
fn check_drop_xor_pin_drop<'tcx>(
375+
tcx: TyCtxt<'tcx>,
376+
adt_def_id: DefId,
377+
drop_impl_did: LocalDefId,
378+
) -> Result<(), ErrorGuaranteed> {
379+
let mut drop_span = None;
380+
let mut pin_drop_span = None;
381+
for item in tcx.associated_items(drop_impl_did).in_definition_order() {
382+
match item.kind {
383+
ty::AssocKind::Fn { name: sym::drop, .. } => {
384+
drop_span = Some(tcx.def_span(item.def_id))
385+
}
386+
ty::AssocKind::Fn { name: sym::pin_drop, .. } => {
387+
pin_drop_span = Some(tcx.def_span(item.def_id))
388+
}
389+
_ => {}
390+
}
391+
}
392+
393+
match (drop_span, pin_drop_span) {
394+
(None, None) => {
395+
if tcx.features().pin_ergonomics() {
396+
return Err(tcx.dcx().emit_err(crate::errors::MissingOneOfTraitItem {
397+
span: tcx.def_span(drop_impl_did),
398+
note: None,
399+
missing_items_msg: "drop`, `pin_drop".to_string(),
400+
}));
401+
} else {
402+
return Err(tcx
403+
.dcx()
404+
.span_delayed_bug(tcx.def_span(drop_impl_did), "missing `Drop::drop`"));
405+
}
406+
}
407+
(Some(span), None) => {
408+
if tcx.adt_def(adt_def_id).is_pin_project() {
409+
let pin_v2_span = rustc_hir::find_attr!(tcx, adt_def_id, PinV2(attr) => *attr);
410+
let adt_name = tcx.item_name(adt_def_id);
411+
return Err(tcx.dcx().emit_err(crate::errors::PinV2WithoutPinDrop {
412+
span,
413+
pin_v2_span,
414+
adt_name,
415+
}));
416+
}
417+
}
418+
(None, Some(span)) => {
419+
if !tcx.features().pin_ergonomics() {
420+
return Err(tcx.dcx().span_delayed_bug(
421+
span,
422+
"`Drop::pin_drop` should be guarded by the library feature gate",
423+
));
424+
}
425+
}
426+
(Some(drop_span), Some(pin_drop_span)) => {
427+
return Err(tcx.dcx().emit_err(crate::errors::ConflictImplDropAndPinDrop {
428+
span: tcx.def_span(drop_impl_did),
429+
drop_span,
430+
pin_drop_span,
431+
}));
432+
}
433+
}
434+
Ok(())
435+
}

compiler/rustc_hir_analysis/src/check/check.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use rustc_middle::ty::{
2424
TypeVisitable, TypeVisitableExt, Unnormalized, fold_regions,
2525
};
2626
use rustc_session::lint::builtin::UNINHABITED_STATIC;
27+
use rustc_span::sym;
2728
use rustc_target::spec::{AbiMap, AbiMapping};
2829
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
2930
use rustc_trait_selection::traits;
@@ -1347,6 +1348,15 @@ fn check_impl_items_against_trait<'tcx>(
13471348
if !is_implemented_here {
13481349
let full_impl_span = tcx.hir_span_with_body(tcx.local_def_id_to_hir_id(impl_id));
13491350
match tcx.eval_default_body_stability(trait_item_id, full_impl_span) {
1351+
// When the feature `pin_ergonomics` is disabled, we report `Drop::drop` is missing,
1352+
// instead of `Drop::drop` is unstable that might be confusing.
1353+
EvalResult::Deny { .. }
1354+
if !tcx.features().pin_ergonomics()
1355+
&& tcx.is_lang_item(trait_ref.def_id, hir::LangItem::Drop)
1356+
&& tcx.item_name(trait_item_id) == sym::drop =>
1357+
{
1358+
missing_items.push(tcx.associated_item(trait_item_id));
1359+
}
13501360
EvalResult::Deny { feature, reason, issue, .. } => default_body_is_unstable(
13511361
tcx,
13521362
full_impl_span,

compiler/rustc_hir_analysis/src/check/mod.rs

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

compiler/rustc_hir_analysis/src/errors.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1968,3 +1968,35 @@ pub(crate) struct EiiDefkindMismatchStaticSafety {
19681968
pub span: Span,
19691969
pub eii_name: Symbol,
19701970
}
1971+
1972+
#[derive(Diagnostic)]
1973+
#[diag("conflicting implementations of `Drop::drop` and `Drop::pin_drop`")]
1974+
pub(crate) struct ConflictImplDropAndPinDrop {
1975+
#[primary_span]
1976+
pub span: Span,
1977+
#[label("`drop(&mut self)` implemented here")]
1978+
pub drop_span: Span,
1979+
#[label("`pin_drop(&pin mut self)` implemented here")]
1980+
pub pin_drop_span: Span,
1981+
}
1982+
1983+
#[derive(Diagnostic)]
1984+
#[diag("`{$adt_name}` must implement `pin_drop`")]
1985+
#[help("structurally pinned types must keep `Pin`'s safety contract")]
1986+
pub(crate) struct PinV2WithoutPinDrop {
1987+
#[primary_span]
1988+
#[suggestion(
1989+
"implement `pin_drop` instead",
1990+
code = "fn pin_drop(&pin mut self)",
1991+
applicability = "maybe-incorrect"
1992+
)]
1993+
pub span: Span,
1994+
#[note("`{$adt_name}` is marked `#[pin_v2]` here")]
1995+
#[suggestion(
1996+
"remove the `#[pin_v2]` attribute if it is not intended for structurally pinning",
1997+
code = "",
1998+
applicability = "maybe-incorrect"
1999+
)]
2000+
pub pin_v2_span: Option<Span>,
2001+
pub adt_name: Symbol,
2002+
}

compiler/rustc_hir_typeck/src/callee.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@ 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+
&& !tcx.is_lang_item(tcx.parent(body_id), LangItem::Drop)
45+
{
4346
let sugg = if let Some(receiver) = receiver.filter(|s| !s.is_empty()) {
4447
errors::ExplicitDestructorCallSugg::Snippet {
4548
lo: expr_span.shrink_to_lo(),

compiler/rustc_middle/src/ty/adt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ impl AdtDefData {
349349
debug!("found non-exhaustive variant list for {:?}", did);
350350
flags = flags | AdtFlags::IS_VARIANT_LIST_NON_EXHAUSTIVE;
351351
}
352-
if find_attr!(tcx, did, PinV2) {
352+
if find_attr!(tcx, did, PinV2(..)) {
353353
debug!("found pin-project type {:?}", did);
354354
flags |= AdtFlags::IS_PIN_PROJECT;
355355
}

compiler/rustc_mir_build/src/check_unsafety.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,11 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
461461
CallToFunctionWith { function: func_did, missing, build_enabled },
462462
);
463463
}
464+
if let Some(trait_did) = self.tcx.trait_of_assoc(func_did)
465+
&& self.tcx.is_lang_item(trait_did, hir::LangItem::Drop)
466+
{
467+
self.requires_unsafe(expr.span, CallDropExplicitly(func_did));
468+
}
464469
}
465470
}
466471
ExprKind::RawBorrow { arg, .. } => {
@@ -661,6 +666,8 @@ enum UnsafeOpKind {
661666
build_enabled: Vec<Symbol>,
662667
},
663668
UnsafeBinderCast,
669+
/// Calling `Drop::drop` or `Drop::pin_drop` explicitly.
670+
CallDropExplicitly(DefId),
664671
}
665672

666673
use UnsafeOpKind::*;
@@ -829,6 +836,9 @@ impl UnsafeOpKind {
829836
unsafe_not_inherited_note,
830837
},
831838
),
839+
CallDropExplicitly(_) => {
840+
span_bug!(span, "`Drop::drop` or `Drop::pin_drop` should not be called explicitly")
841+
}
832842
}
833843
}
834844

@@ -1034,6 +1044,13 @@ impl UnsafeOpKind {
10341044
UnsafeBinderCast => {
10351045
dcx.emit_err(UnsafeBinderCastRequiresUnsafe { span, unsafe_not_inherited_note });
10361046
}
1047+
CallDropExplicitly(did) => {
1048+
dcx.emit_err(CallDropExplicitlyRequiresUnsafe {
1049+
span,
1050+
unsafe_not_inherited_note,
1051+
function: tcx.def_path_str(*did),
1052+
});
1053+
}
10371054
}
10381055
}
10391056
}

0 commit comments

Comments
 (0)