-
Notifications
You must be signed in to change notification settings - Fork 14k
Add Drop::pin_drop for pinned drops
#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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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::sym; | ||
| use rustc_trait_selection::regions::InferCtxtRegionExt; | ||
| use rustc_trait_selection::traits::{self, ObligationCtxt}; | ||
|
|
||
|
|
@@ -70,7 +72,11 @@ pub(crate) fn check_drop_impl( | |
| drop_impl_did, | ||
| adt_def.did(), | ||
| adt_to_impl_args, | ||
| ) | ||
| )?; | ||
|
|
||
| check_drop_xor_pin_drop(tcx, adt_def.did(), drop_impl_did)?; | ||
|
|
||
| Ok(()) | ||
| } | ||
| _ => { | ||
| span_bug!(tcx.def_span(drop_impl_did), "incoherent impl of Drop"); | ||
|
|
@@ -291,3 +297,68 @@ fn ensure_impl_predicates_are_implied_by_item_defn<'tcx>( | |
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| /// This function checks at least and at most one of `Drop::drop` and `Drop::pin_drop` is implemented. | ||
| /// It also checks that `Drop::pin_drop` must be implemented if `#[pin_v2]` is present on the type. | ||
| fn check_drop_xor_pin_drop<'tcx>( | ||
| tcx: TyCtxt<'tcx>, | ||
| adt_def_id: DefId, | ||
| drop_impl_did: LocalDefId, | ||
| ) -> Result<(), ErrorGuaranteed> { | ||
| let item_impl = tcx.hir_expect_item(drop_impl_did).expect_impl(); | ||
| let mut drop_span = None; | ||
| let mut pin_drop_span = 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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some HIR is accessed on a good path here, ideally queries should be used and HIR accesses should be avoided for better incremental compilation (or only performed when errors are reported). |
||
| match impl_item.ident.name { | ||
| sym::drop => drop_span = Some(fn_sig.span), | ||
| sym::pin_drop => pin_drop_span = Some(fn_sig.span), | ||
| _ => {} | ||
| } | ||
| } | ||
| } | ||
|
|
||
| match (drop_span, pin_drop_span) { | ||
| (None, None) => { | ||
| if tcx.features().pin_ergonomics() { | ||
| return Err(tcx.dcx().emit_err(crate::errors::MissingOneOfTraitItem { | ||
| span: tcx.def_span(drop_impl_did), | ||
| note: None, | ||
| missing_items_msg: "drop`, `pin_drop".to_string(), | ||
| })); | ||
| } else { | ||
| return Err(tcx | ||
| .dcx() | ||
| .span_delayed_bug(tcx.def_span(drop_impl_did), "missing `Drop::drop`")); | ||
| } | ||
| } | ||
| (Some(span), None) => { | ||
| if tcx.adt_def(adt_def_id).is_pin_project() { | ||
| let pin_v2_span = tcx.get_attr(adt_def_id, sym::pin_v2).map(|attr| attr.span()); | ||
| let adt_name = tcx.item_name(adt_def_id); | ||
| return Err(tcx.dcx().emit_err(crate::errors::PinV2WithoutPinDrop { | ||
| span, | ||
| pin_v2_span, | ||
| adt_name, | ||
| })); | ||
| } | ||
| } | ||
| (None, Some(span)) => { | ||
| if !tcx.features().pin_ergonomics() { | ||
| return Err(tcx.dcx().span_delayed_bug( | ||
| span, | ||
| "`Drop::pin_drop` should be guarded by the library feature gate", | ||
| )); | ||
| } | ||
| } | ||
| (Some(drop_span), Some(pin_drop_span)) => { | ||
| return Err(tcx.dcx().emit_err(crate::errors::ConflictImplDropAndPinDrop { | ||
| span: tcx.def_span(drop_impl_did), | ||
| drop_span, | ||
| pin_drop_span, | ||
| })); | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -37,9 +37,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) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Body id should have a parent, but if it somehow doesn't, we certainly shouldn't skip the code below.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, calling the destructor explicitly should probably be unsafe? |
||||||
| { | ||||||
| let sugg = if let Some(receiver) = receiver.filter(|s| !s.is_empty()) { | ||||||
| errors::ExplicitDestructorCallSugg::Snippet { | ||||||
| lo: expr_span.shrink_to_lo(), | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1682,6 +1682,7 @@ symbols! { | |
| pic, | ||
| pie, | ||
| pin, | ||
| pin_drop, | ||
| pin_ergonomics, | ||
| pin_macro, | ||
| pin_v2, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,13 @@ error[E0046]: not all trait items implemented, missing: `drop` | |
| --> $DIR/missing-drop-method.rs:2:1 | ||
| | | ||
| LL | impl Drop for DropNoMethod {} | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `drop` in implementation | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can report the old error if |
||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = help: implement the missing item: `fn drop(&mut self) { todo!() }` | ||
| = note: default implementation of `drop` is unstable | ||
| = note: use of unstable library feature `pin_ergonomics` | ||
| = note: see issue #130494 <https://github.com/rust-lang/rust/issues/130494> for more information | ||
| = help: add `#![feature(pin_ergonomics)]` to the crate attributes to enable | ||
| = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.