Implement Drop::pin_drop for !Unpin types

Author: frank-king

compiler/rustc_hir_analysis/messages.ftl

@@ -117,6 +117,11 @@ hir_analysis_coercion_between_struct_same_note = expected coercion between the s
 
 hir_analysis_coercion_between_struct_single_note = expected a single field to be coerced, none found
 
+hir_analysis_conflict_impl_drop_and_pin_drop = conflict implementation of `Drop::drop` and `Drop::pin_drop`
+    .drop_label = `drop(&mut self)` implemented here
+    .pin_drop_label = `pin_drop(&pin mut self)` implemented here
+    .suggestion = remove this implementation
+
 hir_analysis_const_bound_for_non_const_trait = `{$modifier}` can only be applied to `const` traits
     .label = can't be applied to `{$trait_name}`
     .note = `{$trait_name}` can't be used with `{$modifier}` because it isn't `const`
@@ -215,6 +220,11 @@ hir_analysis_functions_names_duplicated = functions names are duplicated
 
 hir_analysis_generic_args_on_overridden_impl = could not resolve generic parameters on overridden impl
 
+hir_analysis_impl_drop_for_negative_unpin_type = could not impl `Drop::drop(&mut self)` for a type that implements `!Unpin`
+    .note = `Unpin` implemented for `{$self_ty}`
+    .help = `impl !Unpin` is intended for pinned projections, which requires the value pinned until it's deallocated
+    .suggestion = consider implement `Drop::pin_drop(&pin mut self)` instead
+
 hir_analysis_impl_not_marked_default = `{$ident}` specializes an item from a parent `impl`, but that item is not marked `default`
     .label = cannot specialize default item `{$ident}`
     .ok_label = parent `impl` is here
@@ -223,6 +233,10 @@ hir_analysis_impl_not_marked_default = `{$ident}` specializes an item from a par
 hir_analysis_impl_not_marked_default_err = `{$ident}` specializes an item from a parent `impl`, but that item is not marked `default`
     .note = parent implementation is in crate `{$cname}`
 
+hir_analysis_impl_pin_drop_for_not_negative_unpin_type = implementing `Drop::pin_drop(&pin mut self)` requires `Self: !Unpin`
+    .suggestion = impl `Drop::drop(&mut self)` instead
+    .impl_negative_unpin_sugg = or impl `!Unpin` for `{$self_ty}` instead
+
 hir_analysis_inherent_dyn = cannot define inherent `impl` for a dyn auto trait
     .label = impl requires at least one non-auto trait
     .note = define and implement a new trait or type instead

compiler/rustc_hir_analysis/src/check/always_applicable.rs

@@ -7,11 +7,13 @@
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::codes::*;
 use rustc_errors::{ErrorGuaranteed, struct_span_code_err};
+use rustc_hir as hir;
 use rustc_infer::infer::{RegionResolutionError, TyCtxtInferExt};
 use rustc_infer::traits::{ObligationCause, ObligationCauseCode};
 use rustc_middle::span_bug;
 use rustc_middle::ty::util::CheckRegions;
 use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypingMode};
+use rustc_span::{Span, sym};
 use rustc_trait_selection::regions::InferCtxtRegionExt;
 use rustc_trait_selection::traits::{self, ObligationCtxt};
 
@@ -70,7 +72,9 @@ pub(crate) fn check_drop_impl(
                 drop_impl_did,
                 adt_def.did(),
                 adt_to_impl_args,
-            )
+            )?;
+
+            ensure_coherent_unpin(tcx, drop_impl_did, adt_def.did(), adt_to_impl_args)
         }
         _ => {
             span_bug!(tcx.def_span(drop_impl_did), "incoherent impl of Drop");
@@ -294,3 +298,102 @@ fn ensure_impl_predicates_are_implied_by_item_defn<'tcx>(
 
     Ok(())
 }
+
+/// This function checks whether `Self: !Unpin` for implementation of `Drop`.
+/// - If `Drop::drop` is implemented, there must be no `Self: !Unpin` implementation.
+/// - If `Drop::pin_drop` is implemented, there must be a `Self: !Unpin` implementation.
+fn ensure_coherent_unpin<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    drop_impl_did: LocalDefId,
+    adt_def_id: DefId,
+    adt_to_impl_args: GenericArgsRef<'tcx>,
+) -> Result<(), ErrorGuaranteed> {
+    let item_impl = tcx.hir_expect_item(drop_impl_did).expect_impl();
+    let mut drop_spans = None;
+    let mut pin_drop_spans = None;
+    for &impl_item_id in item_impl.items {
+        let impl_item = tcx.hir_impl_item(impl_item_id);
+        if let hir::ImplItemKind::Fn(fn_sig, _) = impl_item.kind {
+            match impl_item.ident.name {
+                sym::drop => drop_spans = Some((fn_sig.span, impl_item.span)),
+                sym::pin_drop => pin_drop_spans = Some((fn_sig.span, impl_item.span)),
+                _ => {}
+            }
+        }
+    }
+    let self_ty = Ty::new_adt(tcx, tcx.adt_def(adt_def_id), adt_to_impl_args);
+    let negative_unpin_impl = find_negative_unpin_impl(tcx, self_ty, tcx.def_span(drop_impl_did));
+
+    match (drop_spans, pin_drop_spans) {
+        (None, None) => {
+            return Err(tcx
+                .dcx()
+                .span_delayed_bug(tcx.def_span(drop_impl_did), "unexpected empty impl of `Drop`"));
+        }
+        (Some((span, _)), None) => {
+            if let Some(negative_unpin_impl) = negative_unpin_impl {
+                return Err(tcx.dcx().emit_err(crate::errors::ImplDropForNegativeUnpinType {
+                    span,
+                    negative_unpin_span: tcx.def_span(negative_unpin_impl),
+                    self_ty,
+                }));
+            }
+        }
+        (None, Some((span, _))) => {
+            debug_assert!(
+                tcx.features().pin_ergonomics(),
+                "`Drop::pin_drop` should be guarded by the library feature gate"
+            );
+            if negative_unpin_impl.is_none() {
+                return Err(tcx.dcx().emit_err(
+                    crate::errors::ImplPinDropForNotNegativeUnpinType {
+                        span,
+                        impl_sugg_span: tcx.def_span(drop_impl_did).shrink_to_lo(),
+                        impl_generics_snippet: tcx
+                            .sess
+                            .source_map()
+                            .span_to_snippet(item_impl.generics.span)
+                            .unwrap_or_default(),
+                        self_ty,
+                    },
+                ));
+            }
+        }
+        (
+            Some((drop_span, drop_span_with_body)),
+            Some((pin_drop_span, pin_drop_span_with_body)),
+        ) => {
+            let remove_sugg = match negative_unpin_impl {
+                // without `impl !Unpin`, should pick `drop(&mut self)`, and remove `pin_drop(&pin mut self)`
+                None => pin_drop_span_with_body,
+                // with `impl !Unpin`, should pick `pin_drop(&pin mut self)`, and remove `drop(&mut self)`
+                Some(_) => drop_span_with_body,
+            };
+            return Err(tcx.dcx().emit_err(crate::errors::ConflictImplDropAndPinDrop {
+                span: tcx.def_span(drop_impl_did),
+                drop_span,
+                pin_drop_span,
+                remove_sugg,
+            }));
+        }
+    }
+    Ok(())
+}
+
+fn find_negative_unpin_impl<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    self_ty: Ty<'tcx>,
+    span: Span,
+) -> Option<LocalDefId> {
+    let mut negative_unpin_impl_did = None;
+    tcx.for_each_relevant_impl(tcx.require_lang_item(hir::LangItem::Unpin, span), self_ty, |did| {
+        if tcx.impl_polarity(did) == ty::ImplPolarity::Negative {
+            if negative_unpin_impl_did.is_some() {
+                tcx.dcx()
+                    .span_delayed_bug(tcx.def_span(did), "duplicated `!Unpin` implementations");
+            }
+            negative_unpin_impl_did = Some(did.expect_local());
+        }
+    });
+    negative_unpin_impl_did
+}

compiler/rustc_hir_analysis/src/errors.rs

@@ -1717,3 +1717,53 @@ pub(crate) struct AsyncDropWithoutSyncDrop {
     #[primary_span]
     pub span: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag(hir_analysis_impl_drop_for_negative_unpin_type)]
+#[help]
+pub(crate) struct ImplDropForNegativeUnpinType<'tcx> {
+    #[suggestion(
+        applicability = "machine-applicable",
+        code = "fn pin_drop(&pin mut self)",
+        style = "short"
+    )]
+    #[primary_span]
+    pub span: Span,
+    #[note]
+    pub negative_unpin_span: Span,
+    pub self_ty: Ty<'tcx>,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_analysis_impl_pin_drop_for_not_negative_unpin_type)]
+pub(crate) struct ImplPinDropForNotNegativeUnpinType<'tcx> {
+    #[suggestion(
+        applicability = "machine-applicable",
+        code = "fn drop(&mut self)",
+        style = "short"
+    )]
+    #[primary_span]
+    pub span: Span,
+    #[suggestion(
+        hir_analysis_impl_negative_unpin_sugg,
+        applicability = "machine-applicable",
+        code = "impl{impl_generics_snippet} !Unpin for {self_ty}` {}\n",
+        style = "short"
+    )]
+    pub impl_sugg_span: Span,
+    pub impl_generics_snippet: String,
+    pub self_ty: Ty<'tcx>,
+}
+
+#[derive(Diagnostic)]
+#[diag(hir_analysis_conflict_impl_drop_and_pin_drop)]
+pub(crate) struct ConflictImplDropAndPinDrop {
+    #[primary_span]
+    pub span: Span,
+    #[label(hir_analysis_drop_label)]
+    pub drop_span: Span,
+    #[label(hir_analysis_pin_drop_label)]
+    pub pin_drop_span: Span,
+    #[suggestion(applicability = "machine-applicable", code = "", style = "normal")]
+    pub remove_sugg: Span,
+}

compiler/rustc_hir_typeck/src/callee.rs

@@ -36,9 +36,13 @@ pub(crate) fn check_legal_trait_for_method_call(
     receiver: Option<Span>,
     expr_span: Span,
     trait_id: DefId,
-    _body_id: DefId,
+    body_id: DefId,
 ) -> Result<(), ErrorGuaranteed> {
-    if tcx.is_lang_item(trait_id, LangItem::Drop) {
+    if tcx.is_lang_item(trait_id, LangItem::Drop)
+        // Allow calling `Drop::pin_drop` in `Drop::drop`
+        && let Some(parent) = tcx.opt_parent(body_id)
+        && !tcx.is_lang_item(parent, LangItem::Drop)
+    {
         let sugg = if let Some(receiver) = receiver.filter(|s| !s.is_empty()) {
             errors::ExplicitDestructorCallSugg::Snippet {
                 lo: expr_span.shrink_to_lo(),

compiler/rustc_span/src/symbol.rs

@@ -1616,6 +1616,7 @@ symbols! {
         pic,
         pie,
         pin,
+        pin_drop,
         pin_ergonomics,
         pin_macro,
         platform_intrinsics,

library/core/src/ops/drop.rs

@@ -1,3 +1,5 @@
+use crate::pin::Pin;
+
 /// Custom code within the destructor.
 ///
 /// When a value is no longer needed, Rust will run a "destructor" on that value.
@@ -205,6 +207,7 @@
 #[stable(feature = "rust1", since = "1.0.0")]
 #[const_trait]
 #[rustc_const_unstable(feature = "const_destruct", issue = "133214")]
+#[rustc_must_implement_one_of(drop, pin_drop)]
 pub trait Drop {
     /// Executes the destructor for this type.
     ///
@@ -238,5 +241,29 @@ pub trait Drop {
     /// [`mem::drop`]: drop
     /// [`ptr::drop_in_place`]: crate::ptr::drop_in_place
     #[stable(feature = "rust1", since = "1.0.0")]
-    fn drop(&mut self);
+    fn drop(&mut self) {
+        // SAFETY: `self` is pinned till after dropped.
+        Drop::pin_drop(unsafe { Pin::new_unchecked(self) })
+    }
+
+    /// Execute the destructor for this type, but different to [`Drop::drop`], it requires `self`
+    /// to be pinned.
+    ///
+    /// By implementing this method instead of [`Drop::drop`], the receiver type [`Pin<&mut Self>`]
+    /// makes sure that the value is pinned until it is deallocated (See [`std::pin` module docs] for
+    /// more details), which enables us to support field projections of `Self` type safely.
+    ///
+    /// Currently, this method is used together with a negative implentation of [`Unpin`], which means
+    /// that the current type is never [`Unpin`]. For a given type `Foo`, you can impl `pin_drop` for it
+    /// *if and only if* you have `impl !Unpin for Foo`.
+    ///
+    /// See also [`Drop::drop`].
+    ///
+    /// [`Drop::drop`]: crate::ops::Drop::drop
+    /// [`Unpin`]: crate::marker::Unpin
+    /// [`!Unpin`]: crate::marker::Unpin
+    /// [`Pin<&mut Self>`]: crate::pin::Pin
+    /// [`std::pin` module docs]: crate::pin
+    #[unstable(feature = "pin_ergonomics", issue = "130494")]
+    fn pin_drop(self: Pin<&mut Self>) {}
 }

tests/ui/async-await/async-drop/unexpected-sort.rs

@@ -6,10 +6,12 @@
 #![feature(async_drop)]
 use std::future::AsyncDrop;
 struct a;
-impl Drop for a { //~ ERROR: not all trait items implemented, missing: `drop`
+impl Drop for a {
+    //~ ERROR: not all trait items implemented, missing one of: `drop`, `pin_drop`
     fn b() {} //~ ERROR: method `b` is not a member of trait `Drop`
 }
-impl AsyncDrop for a { //~ ERROR: not all trait items implemented, missing: `drop`
+impl AsyncDrop for a {
+    //~ ERROR: not all trait items implemented, missing: `drop`
     type c = ();
     //~^ ERROR: type `c` is not a member of trait `AsyncDrop`
 }

tests/ui/async-await/async-drop/unexpected-sort.stderr

@@ -10,13 +10,11 @@ error[E0437]: type `c` is not a member of trait `AsyncDrop`
 LL |     type c = ();
    |     ^^^^^^^^^^^^ not a member of trait `AsyncDrop`
 
-error[E0046]: not all trait items implemented, missing: `drop`
+error[E0046]: not all trait items implemented, missing one of: `drop`, `pin_drop`
   --> $DIR/unexpected-sort.rs:9:1
    |
 LL | impl Drop for a {
-   | ^^^^^^^^^^^^^^^ missing `drop` in implementation
-   |
-   = help: implement the missing item: `fn drop(&mut self) { todo!() }`
+   | ^^^^^^^^^^^^^^^ missing one of `drop`, `pin_drop` in implementation
 
 error[E0046]: not all trait items implemented, missing: `drop`
   --> $DIR/unexpected-sort.rs:12:1