Skip to content

Implement Drop::pin_drop for !Unpin types #144537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
105 changes: 104 additions & 1 deletion compiler/rustc_hir_analysis/src/check/always_applicable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
}
50 changes: 50 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
8 changes: 6 additions & 2 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,7 @@ symbols! {
pic,
pie,
pin,
pin_drop,
pin_ergonomics,
pin_macro,
platform_intrinsics,
Expand Down
29 changes: 28 additions & 1 deletion library/core/src/ops/drop.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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>) {}
}
6 changes: 4 additions & 2 deletions tests/ui/async-await/async-drop/unexpected-sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
Expand Down
6 changes: 2 additions & 4 deletions tests/ui/async-await/async-drop/unexpected-sort.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading