From e0904cd6a96e76f89112ea83b45e4c507e3e31df Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 24 May 2024 17:20:49 +0300 Subject: [PATCH 1/7] Add size check inside of the async drop tests --- src/tools/miri/tests/pass/async-drop.rs | 79 +++++++++++++-------- tests/ui/async-await/async-drop.rs | 91 +++++++++++++++---------- 2 files changed, 105 insertions(+), 65 deletions(-) diff --git a/src/tools/miri/tests/pass/async-drop.rs b/src/tools/miri/tests/pass/async-drop.rs index f16206f3db62d..744ffdeebdaf7 100644 --- a/src/tools/miri/tests/pass/async-drop.rs +++ b/src/tools/miri/tests/pass/async-drop.rs @@ -11,9 +11,21 @@ use core::mem::{self, ManuallyDrop}; use core::pin::{pin, Pin}; use core::task::{Context, Poll, Waker}; -async fn test_async_drop(x: T) { +async fn test_async_drop(x: T, _size: usize) { let mut x = mem::MaybeUninit::new(x); let dtor = pin!(unsafe { async_drop_in_place(x.as_mut_ptr()) }); + + // FIXME(zetanumbers): This check fully depends on the layout of + // the coroutine state, since async destructor combinators are just + // async functions. + #[cfg(target_pointer_width = "64")] + assert_eq!( + mem::size_of_val(&*dtor), + _size, + "sizes did not match for async destructor of type {}", + core::any::type_name::(), + ); + test_idempotency(dtor).await; } @@ -34,49 +46,58 @@ fn main() { let i = 13; let fut = pin!(async { - test_async_drop(Int(0)).await; - test_async_drop(AsyncInt(0)).await; - test_async_drop([AsyncInt(1), AsyncInt(2)]).await; - test_async_drop((AsyncInt(3), AsyncInt(4))).await; - test_async_drop(5).await; + test_async_drop(Int(0), 16).await; + test_async_drop(AsyncInt(0), 104).await; + test_async_drop([AsyncInt(1), AsyncInt(2)], 152).await; + test_async_drop((AsyncInt(3), AsyncInt(4)), 488).await; + test_async_drop(5, 0).await; let j = 42; - test_async_drop(&i).await; - test_async_drop(&j).await; - test_async_drop(AsyncStruct { b: AsyncInt(8), a: AsyncInt(7), i: 6 }).await; - test_async_drop(ManuallyDrop::new(AsyncInt(9))).await; + test_async_drop(&i, 0).await; + test_async_drop(&j, 0).await; + test_async_drop(AsyncStruct { b: AsyncInt(8), a: AsyncInt(7), i: 6 }, 1688).await; + test_async_drop(ManuallyDrop::new(AsyncInt(9)), 0).await; let foo = AsyncInt(10); - test_async_drop(AsyncReference { foo: &foo }).await; + test_async_drop(AsyncReference { foo: &foo }, 104).await; let foo = AsyncInt(11); - test_async_drop(|| { - black_box(foo); - let foo = AsyncInt(10); - foo - }) + test_async_drop( + || { + black_box(foo); + let foo = AsyncInt(10); + foo + }, + 120, + ) .await; - test_async_drop(AsyncEnum::A(AsyncInt(12))).await; - test_async_drop(AsyncEnum::B(SyncInt(13))).await; + test_async_drop(AsyncEnum::A(AsyncInt(12)), 792).await; + test_async_drop(AsyncEnum::B(SyncInt(13)), 792).await; - test_async_drop(SyncInt(14)).await; - test_async_drop(SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }) - .await; + test_async_drop(SyncInt(14), 72).await; + test_async_drop( + SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }, + 3512, + ) + .await; let async_drop_fut = pin!(core::future::async_drop(AsyncInt(19))); test_idempotency(async_drop_fut).await; let foo = AsyncInt(20); - test_async_drop(async || { - black_box(foo); - let foo = AsyncInt(19); - // Await point there, but this is async closure so it's fine - black_box(core::future::ready(())).await; - foo - }) + test_async_drop( + async || { + black_box(foo); + let foo = AsyncInt(19); + // Await point there, but this is async closure so it's fine + black_box(core::future::ready(())).await; + foo + }, + 120, + ) .await; - test_async_drop(AsyncUnion { signed: 21 }).await; + test_async_drop(AsyncUnion { signed: 21 }, 32).await; }); let res = fut.poll(&mut cx); assert_eq!(res, Poll::Ready(())); diff --git a/tests/ui/async-await/async-drop.rs b/tests/ui/async-await/async-drop.rs index 6d02dcebc0b65..d1222cd740c32 100644 --- a/tests/ui/async-await/async-drop.rs +++ b/tests/ui/async-await/async-drop.rs @@ -13,9 +13,21 @@ use core::mem::{self, ManuallyDrop}; use core::pin::{pin, Pin}; use core::task::{Context, Poll, Waker}; -async fn test_async_drop(x: T) { +async fn test_async_drop(x: T, _size: usize) { let mut x = mem::MaybeUninit::new(x); let dtor = pin!(unsafe { async_drop_in_place(x.as_mut_ptr()) }); + + // FIXME(zetanumbers): This check fully depends on the layout of + // the coroutine state, since async destructor combinators are just + // async functions. + #[cfg(target_pointer_width = "64")] + assert_eq!( + mem::size_of_val(&*dtor), + _size, + "sizes did not match for async destructor of type {}", + core::any::type_name::(), + ); + test_idempotency(dtor).await; } @@ -36,51 +48,58 @@ fn main() { let i = 13; let fut = pin!(async { - test_async_drop(Int(0)).await; - test_async_drop(AsyncInt(0)).await; - test_async_drop([AsyncInt(1), AsyncInt(2)]).await; - test_async_drop((AsyncInt(3), AsyncInt(4))).await; - test_async_drop(5).await; + test_async_drop(Int(0), 16).await; + test_async_drop(AsyncInt(0), 104).await; + test_async_drop([AsyncInt(1), AsyncInt(2)], 152).await; + test_async_drop((AsyncInt(3), AsyncInt(4)), 488).await; + test_async_drop(5, 0).await; let j = 42; - test_async_drop(&i).await; - test_async_drop(&j).await; - test_async_drop(AsyncStruct { b: AsyncInt(8), a: AsyncInt(7), i: 6 }).await; - test_async_drop(ManuallyDrop::new(AsyncInt(9))).await; + test_async_drop(&i, 0).await; + test_async_drop(&j, 0).await; + test_async_drop(AsyncStruct { b: AsyncInt(8), a: AsyncInt(7), i: 6 }, 1688).await; + test_async_drop(ManuallyDrop::new(AsyncInt(9)), 0).await; let foo = AsyncInt(10); - test_async_drop(AsyncReference { foo: &foo }).await; + test_async_drop(AsyncReference { foo: &foo }, 104).await; let foo = AsyncInt(11); - test_async_drop(|| { - black_box(foo); - let foo = AsyncInt(10); - foo - }).await; - - test_async_drop(AsyncEnum::A(AsyncInt(12))).await; - test_async_drop(AsyncEnum::B(SyncInt(13))).await; - - test_async_drop(SyncInt(14)).await; - test_async_drop(SyncThenAsync { - i: 15, - a: AsyncInt(16), - b: SyncInt(17), - c: AsyncInt(18), - }).await; + test_async_drop( + || { + black_box(foo); + let foo = AsyncInt(10); + foo + }, + 120, + ) + .await; + + test_async_drop(AsyncEnum::A(AsyncInt(12)), 792).await; + test_async_drop(AsyncEnum::B(SyncInt(13)), 792).await; + + test_async_drop(SyncInt(14), 72).await; + test_async_drop( + SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }, + 3512, + ) + .await; let async_drop_fut = pin!(core::future::async_drop(AsyncInt(19))); test_idempotency(async_drop_fut).await; let foo = AsyncInt(20); - test_async_drop(async || { - black_box(foo); - let foo = AsyncInt(19); - // Await point there, but this is async closure so it's fine - black_box(core::future::ready(())).await; - foo - }).await; - - test_async_drop(AsyncUnion { signed: 21 }).await; + test_async_drop( + async || { + black_box(foo); + let foo = AsyncInt(19); + // Await point there, but this is async closure so it's fine + black_box(core::future::ready(())).await; + foo + }, + 120, + ) + .await; + + test_async_drop(AsyncUnion { signed: 21 }, 32).await; }); let res = fut.poll(&mut cx); assert_eq!(res, Poll::Ready(())); From a47173c4f77b5e7c960ffe178eba2bc00bb91e31 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Thu, 2 May 2024 17:59:02 +0300 Subject: [PATCH 2/7] Start implementing needs_async_drop and related --- compiler/rustc_hir_analysis/src/check/mod.rs | 5 + .../src/rmeta/decoder/cstore_impl.rs | 4 + compiler/rustc_middle/src/query/erase.rs | 2 + compiler/rustc_middle/src/query/mod.rs | 17 +- compiler/rustc_middle/src/ty/adt.rs | 10 +- compiler/rustc_middle/src/ty/mod.rs | 9 + compiler/rustc_middle/src/ty/sty.rs | 26 +-- compiler/rustc_middle/src/ty/util.rs | 196 ++++++++++++------ compiler/rustc_ty_utils/src/common_traits.rs | 21 +- compiler/rustc_ty_utils/src/needs_drop.rs | 16 ++ 10 files changed, 196 insertions(+), 110 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/mod.rs b/compiler/rustc_hir_analysis/src/check/mod.rs index 0083da2a1e44f..149e7737e3090 100644 --- a/compiler/rustc_hir_analysis/src/check/mod.rs +++ b/compiler/rustc_hir_analysis/src/check/mod.rs @@ -112,6 +112,7 @@ pub fn provide(providers: &mut Providers) { wfcheck::provide(providers); *providers = Providers { adt_destructor, + adt_async_destructor, region_scope_tree, collect_return_position_impl_trait_in_trait_tys, compare_impl_const: compare_impl_item::compare_impl_const_raw, @@ -124,6 +125,10 @@ fn adt_destructor(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option tcx.calculate_dtor(def_id.to_def_id(), dropck::check_drop_impl) } +fn adt_async_destructor(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option { + tcx.calculate_async_dtor(def_id.to_def_id(), dropck::check_drop_impl) +} + /// Given a `DefId` for an opaque type in return position, find its parent item's return /// expressions. fn get_owner_return_paths( diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 4cad317ce80cd..f6b9c7ed99265 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -287,6 +287,10 @@ provide! { tcx, def_id, other, cdata, let _ = cdata; tcx.calculate_dtor(def_id, |_,_| Ok(())) } + adt_async_destructor => { + let _ = cdata; + tcx.calculate_async_dtor(def_id, |_,_| Ok(())) + } associated_item_def_ids => { tcx.arena.alloc_from_iter(cdata.get_associated_item_or_field_def_ids(def_id.index)) } diff --git a/compiler/rustc_middle/src/query/erase.rs b/compiler/rustc_middle/src/query/erase.rs index 1e36f034cc220..f98dbf8a0bd42 100644 --- a/compiler/rustc_middle/src/query/erase.rs +++ b/compiler/rustc_middle/src/query/erase.rs @@ -238,6 +238,7 @@ trivial! { Option, Option, Option, + Option, Option, Option, Option, @@ -295,6 +296,7 @@ trivial! { rustc_middle::ty::AssocItem, rustc_middle::ty::AssocItemContainer, rustc_middle::ty::Asyncness, + rustc_middle::ty::AsyncDestructor, rustc_middle::ty::BoundVariableKind, rustc_middle::ty::DeducedParamAttrs, rustc_middle::ty::Destructor, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 6ad4b7c40fb2a..d72ad09f95484 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -703,6 +703,11 @@ rustc_queries! { cache_on_disk_if { key.is_local() } separate_provide_extern } + query adt_async_destructor(key: DefId) -> Option { + desc { |tcx| "computing `AsyncDrop` impl for `{}`", tcx.def_path_str(key) } + cache_on_disk_if { key.is_local() } + separate_provide_extern + } query adt_sized_constraint(key: DefId) -> Option>> { desc { |tcx| "computing the `Sized` constraint for `{}`", tcx.def_path_str(key) } @@ -1343,18 +1348,14 @@ rustc_queries! { query is_unpin_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { desc { "computing whether `{}` is `Unpin`", env.value } } - /// Query backing `Ty::has_surface_async_drop`. - query has_surface_async_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { - desc { "computing whether `{}` has `AsyncDrop` implementation", env.value } - } - /// Query backing `Ty::has_surface_drop`. - query has_surface_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { - desc { "computing whether `{}` has `Drop` implementation", env.value } - } /// Query backing `Ty::needs_drop`. query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { desc { "computing whether `{}` needs drop", env.value } } + /// Query backing `Ty::needs_async_drop`. + query needs_async_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + desc { "computing whether `{}` needs async drop", env.value } + } /// Query backing `Ty::has_significant_drop_raw`. query has_significant_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { desc { "computing whether `{}` has a significant drop", env.value } diff --git a/compiler/rustc_middle/src/ty/adt.rs b/compiler/rustc_middle/src/ty/adt.rs index 8e946bc8b314d..988ce35484d5b 100644 --- a/compiler/rustc_middle/src/ty/adt.rs +++ b/compiler/rustc_middle/src/ty/adt.rs @@ -24,7 +24,9 @@ use std::hash::{Hash, Hasher}; use std::ops::Range; use std::str; -use super::{Destructor, FieldDef, GenericPredicates, Ty, TyCtxt, VariantDef, VariantDiscr}; +use super::{ + AsyncDestructor, Destructor, FieldDef, GenericPredicates, Ty, TyCtxt, VariantDef, VariantDiscr, +}; #[derive(Clone, Copy, PartialEq, Eq, Hash, HashStable, TyEncodable, TyDecodable)] pub struct AdtFlags(u16); @@ -577,6 +579,12 @@ impl<'tcx> AdtDef<'tcx> { tcx.adt_destructor(self.did()) } + // FIXME(zetanumbers): consider supporting this method in same places where + // `destructor` is referenced + pub fn async_destructor(self, tcx: TyCtxt<'tcx>) -> Option { + tcx.adt_async_destructor(self.did()) + } + /// Returns a type such that `Self: Sized` if and only if that type is `Sized`, /// or `None` if the type is always sized. pub fn sized_constraint(self, tcx: TyCtxt<'tcx>) -> Option>> { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index f24074cb472d9..90c154233dabb 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1172,6 +1172,15 @@ pub struct Destructor { pub constness: hir::Constness, } +// FIXME: consider combining this definition with regular `Destructor` +#[derive(Copy, Clone, Debug, HashStable, Encodable, Decodable)] +pub struct AsyncDestructor { + /// The `DefId` of the async destructor future constructor + pub ctor: DefId, + /// The `DefId` of the async destructor future type + pub future: DefId, +} + #[derive(Clone, Copy, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] pub struct VariantFlags(u8); bitflags::bitflags! { diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 5cf96d298375b..00f603a881899 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1991,7 +1991,7 @@ impl<'tcx> Ty<'tcx> { ty::Adt(adt_def, _) => { assert!(adt_def.is_union()); - let surface_drop = self.surface_async_dropper_ty(tcx, param_env).unwrap(); + let surface_drop = self.surface_async_dropper_ty(tcx).unwrap(); Ty::async_destructor_combinator(tcx, LangItem::AsyncDropFuse) .instantiate(tcx, &[surface_drop.into()]) @@ -2041,7 +2041,7 @@ impl<'tcx> Ty<'tcx> { }) .unwrap(); - let dtor = if let Some(dropper_ty) = self.surface_async_dropper_ty(tcx, param_env) { + let dtor = if let Some(dropper_ty) = self.surface_async_dropper_ty(tcx) { Ty::async_destructor_combinator(tcx, LangItem::AsyncDropChain) .instantiate(tcx, &[dropper_ty.into(), variants_dtor.into()]) } else { @@ -2052,21 +2052,13 @@ impl<'tcx> Ty<'tcx> { .instantiate(tcx, &[dtor.into()]) } - fn surface_async_dropper_ty( - self, - tcx: TyCtxt<'tcx>, - param_env: ParamEnv<'tcx>, - ) -> Option> { - if self.has_surface_async_drop(tcx, param_env) { - Some(LangItem::SurfaceAsyncDropInPlace) - } else if self.has_surface_drop(tcx, param_env) { - Some(LangItem::AsyncDropSurfaceDropInPlace) - } else { - None - } - .map(|dropper| { - Ty::async_destructor_combinator(tcx, dropper).instantiate(tcx, &[self.into()]) - }) + fn surface_async_dropper_ty(self, tcx: TyCtxt<'tcx>) -> Option> { + let adt_def = self.ty_adt_def()?; + let dropper = adt_def + .async_destructor(tcx) + .map(|_| LangItem::SurfaceAsyncDropInPlace) + .or_else(|| adt_def.destructor(tcx).map(|_| LangItem::AsyncDropSurfaceDropInPlace))?; + Some(Ty::async_destructor_combinator(tcx, dropper).instantiate(tcx, &[self.into()])) } fn async_destructor_combinator( diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index cacaa859d526d..905ada3832d5e 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -4,8 +4,8 @@ use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags; use crate::query::{IntoQueryParam, Providers}; use crate::ty::layout::{FloatExt, IntegerExt}; use crate::ty::{ - self, FallibleTypeFolder, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, - TypeVisitableExt, Upcast, + self, Asyncness, FallibleTypeFolder, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, + TypeSuperFoldable, TypeVisitableExt, Upcast, }; use crate::ty::{GenericArgKind, GenericArgsRef}; use rustc_apfloat::Float as _; @@ -382,6 +382,45 @@ impl<'tcx> TyCtxt<'tcx> { Some(ty::Destructor { did, constness }) } + /// Calculate the async destructor of a given type. + pub fn calculate_async_dtor( + self, + adt_did: DefId, + validate: impl Fn(Self, DefId) -> Result<(), ErrorGuaranteed>, + ) -> Option { + let async_drop_trait = self.lang_items().async_drop_trait()?; + self.ensure().coherent_trait(async_drop_trait).ok()?; + + let ty = self.type_of(adt_did).instantiate_identity(); + let mut dtor_candidate = None; + self.for_each_relevant_impl(async_drop_trait, ty, |impl_did| { + if validate(self, impl_did).is_err() { + // Already `ErrorGuaranteed`, no need to delay a span bug here. + return; + } + + let [future, ctor] = self.associated_item_def_ids(impl_did) else { + self.dcx().span_delayed_bug( + self.def_span(impl_did), + "AsyncDrop impl without async_drop function or Dropper type", + ); + return; + }; + + if let Some((_, _, old_impl_did)) = dtor_candidate { + self.dcx() + .struct_span_err(self.def_span(impl_did), "multiple async drop impls found") + .with_span_note(self.def_span(old_impl_did), "other impl here") + .delay_as_bug(); + } + + dtor_candidate = Some((*future, *ctor, impl_did)); + }); + + let (future, ctor, _) = dtor_candidate?; + Some(ty::AsyncDestructor { future, ctor }) + } + /// Returns the set of types that are required to be alive in /// order to run the destructor of `def` (see RFCs 769 and /// 1238). @@ -1303,72 +1342,19 @@ impl<'tcx> Ty<'tcx> { } } - /// Checks whether values of this type `T` implements the `AsyncDrop` - /// trait. - pub fn has_surface_async_drop(self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { - self.could_have_surface_async_drop() && tcx.has_surface_async_drop_raw(param_env.and(self)) - } - - /// Fast path helper for testing if a type has `AsyncDrop` - /// implementation. - /// - /// Returning `false` means the type is known to not have `AsyncDrop` - /// implementation. Returning `true` means nothing -- could be - /// `AsyncDrop`, might not be. - fn could_have_surface_async_drop(self) -> bool { - !self.is_async_destructor_trivially_noop() - && !matches!( - self.kind(), - ty::Tuple(_) - | ty::Slice(_) - | ty::Array(_, _) - | ty::Closure(..) - | ty::CoroutineClosure(..) - | ty::Coroutine(..) - ) - } - - /// Checks whether values of this type `T` implements the `Drop` - /// trait. - pub fn has_surface_drop(self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { - self.could_have_surface_drop() && tcx.has_surface_drop_raw(param_env.and(self)) - } - - /// Fast path helper for testing if a type has `Drop` implementation. - /// - /// Returning `false` means the type is known to not have `Drop` - /// implementation. Returning `true` means nothing -- could be - /// `Drop`, might not be. - fn could_have_surface_drop(self) -> bool { - !self.is_async_destructor_trivially_noop() - && !matches!( - self.kind(), - ty::Tuple(_) - | ty::Slice(_) - | ty::Array(_, _) - | ty::Closure(..) - | ty::CoroutineClosure(..) - | ty::Coroutine(..) - ) - } - /// Checks whether values of this type `T` implement has noop async destructor. // // FIXME: implement optimization to make ADTs, which do not need drop, - // to skip fields or to have noop async destructor. + // to skip fields or to have noop async destructor, use `needs_(async_)drop` pub fn is_async_destructor_noop( self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, ) -> bool { + // TODO: check on the most generic version of your type self.is_async_destructor_trivially_noop() - || if let ty::Adt(adt_def, _) = self.kind() { - (adt_def.is_union() || adt_def.is_payloadfree()) - && !self.has_surface_async_drop(tcx, param_env) - && !self.has_surface_drop(tcx, param_env) - } else { - false - } + || self.needs_async_drop(tcx, param_env) + || self.needs_drop(tcx, param_env) } /// Fast path helper for testing if a type has noop async destructor. @@ -1391,7 +1377,34 @@ impl<'tcx> Ty<'tcx> { | ty::FnPtr(_) => true, ty::Tuple(tys) => tys.is_empty(), ty::Adt(adt_def, _) => adt_def.is_manually_drop(), - _ => false, + ty::Bool => todo!(), + ty::Char => todo!(), + ty::Int(_) => todo!(), + ty::Uint(_) => todo!(), + ty::Float(_) => todo!(), + ty::Adt(_, _) => todo!(), + ty::Foreign(_) => todo!(), + ty::Str => todo!(), + ty::Array(_, _) => todo!(), + ty::Pat(_, _) => todo!(), + ty::Slice(_) => todo!(), + ty::RawPtr(_, _) => todo!(), + ty::Ref(_, _, _) => todo!(), + ty::FnDef(_, _) => todo!(), + ty::FnPtr(_) => todo!(), + ty::Dynamic(_, _, _) => todo!(), + ty::Closure(_, _) => todo!(), + ty::CoroutineClosure(_, _) => todo!(), + ty::Coroutine(_, _) => todo!(), + ty::CoroutineWitness(_, _) => todo!(), + ty::Never => todo!(), + ty::Tuple(_) => todo!(), + ty::Alias(_, _) => todo!(), + ty::Param(_) => todo!(), + ty::Bound(_, _) => todo!(), + ty::Placeholder(_) => todo!(), + ty::Infer(_) => todo!(), + ty::Error(_) => todo!(), } } @@ -1430,6 +1443,42 @@ impl<'tcx> Ty<'tcx> { } } + /// If `ty.needs_async_drop(...)` returns `true`, then `ty` is definitely + /// non-copy and *might* have a async destructor attached; if it returns + /// `false`, then `ty` definitely has no async destructor (i.e., no async + /// drop glue). + /// + /// (Note that this implies that if `ty` has an async destructor attached, + /// then `needs_async_drop` will definitely return `true` for `ty`.) + /// + /// Note that this method is used to check eligible types in unions. + #[inline] + pub fn needs_async_drop(self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { + // Avoid querying in simple cases. + match needs_drop_components(tcx, self) { + Err(AlwaysRequiresDrop) => true, + Ok(components) => { + let query_ty = match *components { + [] => return false, + // If we've got a single component, call the query with that + // to increase the chance that we hit the query cache. + [component_ty] => component_ty, + _ => self, + }; + + // This doesn't depend on regions, so try to minimize distinct + // query keys used. + // If normalization fails, we just use `query_ty`. + debug_assert!(!param_env.has_infer()); + let query_ty = tcx + .try_normalize_erasing_regions(param_env, query_ty) + .unwrap_or_else(|_| tcx.erase_regions(query_ty)); + + tcx.needs_async_drop_raw(param_env.and(query_ty)) + } + } + } + /// Checks if `ty` has a significant drop. /// /// Note that this method can return false even if `ty` has a destructor @@ -1598,12 +1647,24 @@ impl<'tcx> ExplicitSelf<'tcx> { } } +// FIXME(zetanumbers): make specifying asyncness explicit /// Returns a list of types such that the given type needs drop if and only if /// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if /// this type always needs drop. pub fn needs_drop_components<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, +) -> Result; 2]>, AlwaysRequiresDrop> { + needs_drop_components_with_async(tcx, ty, Asyncness::No) +} + +/// Returns a list of types such that the given type needs drop if and only if +/// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if +/// this type always needs drop. +pub fn needs_drop_components_with_async<'tcx>( + tcx: TyCtxt<'tcx>, + ty: Ty<'tcx>, + asyncness: Asyncness, ) -> Result; 2]>, AlwaysRequiresDrop> { match *ty.kind() { ty::Infer(ty::FreshIntTy(_)) @@ -1623,11 +1684,18 @@ pub fn needs_drop_components<'tcx>( // Foreign types can never have destructors. ty::Foreign(..) => Ok(SmallVec::new()), - ty::Dynamic(..) | ty::Error(_) => Err(AlwaysRequiresDrop), + // FIXME(zetanumbers): Temporary workaround for async drop of dynamic types + ty::Dynamic(..) | ty::Error(_) => { + if asyncness.is_async() { + Ok(SmallVec::new()) + } else { + Err(AlwaysRequiresDrop) + } + } - ty::Pat(ty, _) | ty::Slice(ty) => needs_drop_components(tcx, ty), + ty::Pat(ty, _) | ty::Slice(ty) => needs_drop_components_with_async(tcx, ty, asyncness), ty::Array(elem_ty, size) => { - match needs_drop_components(tcx, elem_ty) { + match needs_drop_components_with_async(tcx, elem_ty, asyncness) { Ok(v) if v.is_empty() => Ok(v), res => match size.try_to_target_usize(tcx) { // Arrays of size zero don't need drop, even if their element @@ -1643,7 +1711,7 @@ pub fn needs_drop_components<'tcx>( } // If any field needs drop, then the whole tuple does. ty::Tuple(fields) => fields.iter().try_fold(SmallVec::new(), move |mut acc, elem| { - acc.extend(needs_drop_components(tcx, elem)?); + acc.extend(needs_drop_components_with_async(tcx, elem, asyncness)?); Ok(acc) }), diff --git a/compiler/rustc_ty_utils/src/common_traits.rs b/compiler/rustc_ty_utils/src/common_traits.rs index cb95239e99192..51b908881eb49 100644 --- a/compiler/rustc_ty_utils/src/common_traits.rs +++ b/compiler/rustc_ty_utils/src/common_traits.rs @@ -22,17 +22,6 @@ fn is_unpin_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) is_item_raw(tcx, query, LangItem::Unpin) } -fn has_surface_async_drop_raw<'tcx>( - tcx: TyCtxt<'tcx>, - query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, -) -> bool { - is_item_raw(tcx, query, LangItem::AsyncDrop) -} - -fn has_surface_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { - is_item_raw(tcx, query, LangItem::Drop) -} - fn is_item_raw<'tcx>( tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>, @@ -45,13 +34,5 @@ fn is_item_raw<'tcx>( } pub(crate) fn provide(providers: &mut Providers) { - *providers = Providers { - is_copy_raw, - is_sized_raw, - is_freeze_raw, - is_unpin_raw, - has_surface_async_drop_raw, - has_surface_drop_raw, - ..*providers - }; + *providers = Providers { is_copy_raw, is_sized_raw, is_freeze_raw, is_unpin_raw, ..*providers }; } diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs index 72ee1a4824953..7fc3543ff66a7 100644 --- a/compiler/rustc_ty_utils/src/needs_drop.rs +++ b/compiler/rustc_ty_utils/src/needs_drop.rs @@ -30,6 +30,21 @@ fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx> res } +fn needs_async_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + // If we don't know a type doesn't need async drop, for example if it's a + // type parameter without a `Copy` bound, then we conservatively return that + // it needs async drop. + let adt_has_async_dtor = + |adt_def: ty::AdtDef<'tcx>| adt_def.async_destructor(tcx).map(|_| DtorType::Significant); + let res = drop_tys_helper(tcx, query.value, query.param_env, adt_has_dtor, false) + .filter(filter_array_elements(tcx, query.param_env)) + .next() + .is_some(); + + debug!("needs_drop_raw({:?}) = {:?}", query, res); + res +} + /// HACK: in order to not mistakenly assume that `[PhantomData; N]` requires drop glue /// we check the element type for drop glue. The correct fix would be looking at the /// entirety of the code around `needs_drop_components` and this file and come up with @@ -389,6 +404,7 @@ fn adt_significant_drop_tys( pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { needs_drop_raw, + needs_async_drop_raw, has_significant_drop_raw, adt_drop_tys, adt_significant_drop_tys, From 7cdd95e1a683c89baff762a6c7755951bcd32717 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 3 May 2024 15:47:23 +0300 Subject: [PATCH 3/7] Optimize async drop glue for some old types --- compiler/rustc_hir/src/lang_items.rs | 1 + compiler/rustc_middle/src/ty/adt.rs | 4 +- compiler/rustc_middle/src/ty/sty.rs | 48 +++--- compiler/rustc_middle/src/ty/util.rs | 139 +++++++++++------- .../src/shim/async_destructor_ctor.rs | 37 ++++- compiler/rustc_span/src/symbol.rs | 1 + .../src/solve/normalizes_to/mod.rs | 2 +- .../src/traits/project.rs | 2 +- compiler/rustc_ty_utils/src/instance.rs | 3 +- compiler/rustc_ty_utils/src/needs_drop.rs | 2 +- library/core/src/future/async_drop.rs | 11 ++ 11 files changed, 161 insertions(+), 89 deletions(-) diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index c4be67cdd887f..4c9d143a63b90 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -176,6 +176,7 @@ language_item_table! { AsyncDropSlice, sym::async_drop_slice, async_drop_slice_fn, Target::Fn, GenericRequirement::Exact(1); AsyncDropChain, sym::async_drop_chain, async_drop_chain_fn, Target::Fn, GenericRequirement::Exact(2); AsyncDropNoop, sym::async_drop_noop, async_drop_noop_fn, Target::Fn, GenericRequirement::Exact(0); + AsyncDropDeferredDropInPlace, sym::async_drop_deferred_drop_in_place, async_drop_deferred_drop_in_place_fn, Target::Fn, GenericRequirement::Exact(1); AsyncDropFuse, sym::async_drop_fuse, async_drop_fuse_fn, Target::Fn, GenericRequirement::Exact(1); AsyncDropDefer, sym::async_drop_defer, async_drop_defer_fn, Target::Fn, GenericRequirement::Exact(1); AsyncDropEither, sym::async_drop_either, async_drop_either_fn, Target::Fn, GenericRequirement::Exact(3); diff --git a/compiler/rustc_middle/src/ty/adt.rs b/compiler/rustc_middle/src/ty/adt.rs index 988ce35484d5b..5f9b870331c9a 100644 --- a/compiler/rustc_middle/src/ty/adt.rs +++ b/compiler/rustc_middle/src/ty/adt.rs @@ -579,8 +579,8 @@ impl<'tcx> AdtDef<'tcx> { tcx.adt_destructor(self.did()) } - // FIXME(zetanumbers): consider supporting this method in same places where - // `destructor` is referenced + // FIXME: consider combining this method with `AdtDef::destructor` and removing + // this version pub fn async_destructor(self, tcx: TyCtxt<'tcx>) -> Option { tcx.adt_async_destructor(self.did()) } diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 00f603a881899..5f7385fccc989 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -25,7 +25,7 @@ use std::assert_matches::debug_assert_matches; use std::borrow::Cow; use std::iter; use std::ops::{ControlFlow, Range}; -use ty::util::IntTypeExt; +use ty::util::{AsyncDropGlueMorphology, IntTypeExt}; use rustc_type_ir::TyKind::*; use rustc_type_ir::{self as ir, BoundVar, CollectAndApply, DynKind}; @@ -1951,11 +1951,22 @@ impl<'tcx> Ty<'tcx> { } /// Returns the type of the async destructor of this type. - pub fn async_destructor_ty(self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> Ty<'tcx> { - if self.is_async_destructor_noop(tcx, param_env) || matches!(self.kind(), ty::Error(_)) { - return Ty::async_destructor_combinator(tcx, LangItem::AsyncDropNoop) - .instantiate_identity(); + pub fn async_destructor_ty(self, tcx: TyCtxt<'tcx>) -> Ty<'tcx> { + match self.async_drop_glue_morphology(tcx) { + AsyncDropGlueMorphology::Noop => { + return Ty::async_destructor_combinator(tcx, LangItem::AsyncDropNoop) + .instantiate_identity(); + } + AsyncDropGlueMorphology::DeferredDropInPlace => { + let drop_in_place = + Ty::async_destructor_combinator(tcx, LangItem::AsyncDropDeferredDropInPlace) + .instantiate(tcx, &[self.into()]); + return Ty::async_destructor_combinator(tcx, LangItem::AsyncDropFuse) + .instantiate(tcx, &[drop_in_place.into()]); + } + AsyncDropGlueMorphology::Custom => (), } + match *self.kind() { ty::Param(_) | ty::Alias(..) | ty::Infer(ty::TyVar(_)) => { let assoc_items = tcx @@ -1974,19 +1985,13 @@ impl<'tcx> Ty<'tcx> { .adt_async_destructor_ty( tcx, adt_def.variants().iter().map(|v| v.fields.iter().map(|f| f.ty(tcx, args))), - param_env, ), - ty::Tuple(tys) => self.adt_async_destructor_ty(tcx, iter::once(tys), param_env), - ty::Closure(_, args) => self.adt_async_destructor_ty( - tcx, - iter::once(args.as_closure().upvar_tys()), - param_env, - ), - ty::CoroutineClosure(_, args) => self.adt_async_destructor_ty( - tcx, - iter::once(args.as_coroutine_closure().upvar_tys()), - param_env, - ), + ty::Tuple(tys) => self.adt_async_destructor_ty(tcx, iter::once(tys)), + ty::Closure(_, args) => { + self.adt_async_destructor_ty(tcx, iter::once(args.as_closure().upvar_tys())) + } + ty::CoroutineClosure(_, args) => self + .adt_async_destructor_ty(tcx, iter::once(args.as_coroutine_closure().upvar_tys())), ty::Adt(adt_def, _) => { assert!(adt_def.is_union()); @@ -2008,17 +2013,12 @@ impl<'tcx> Ty<'tcx> { } } - fn adt_async_destructor_ty( - self, - tcx: TyCtxt<'tcx>, - variants: I, - param_env: ParamEnv<'tcx>, - ) -> Ty<'tcx> + fn adt_async_destructor_ty(self, tcx: TyCtxt<'tcx>, variants: I) -> Ty<'tcx> where I: Iterator + ExactSizeIterator, I::Item: IntoIterator>, { - debug_assert!(!self.is_async_destructor_noop(tcx, param_env)); + debug_assert_eq!(self.async_drop_glue_morphology(tcx), AsyncDropGlueMorphology::Custom); let defer = Ty::async_destructor_combinator(tcx, LangItem::AsyncDropDefer); let chain = Ty::async_destructor_combinator(tcx, LangItem::AsyncDropChain); diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 905ada3832d5e..2f2d03e9ec841 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -421,6 +421,25 @@ impl<'tcx> TyCtxt<'tcx> { Some(ty::AsyncDestructor { future, ctor }) } + /// Returns async drop glue morphology for a definition. To get async drop + /// glue morphology for a type see [`Ty::async_drop_glue_morphology`]. + // + // FIXME: consider making this a query + pub fn async_drop_glue_morphology(self, did: DefId) -> AsyncDropGlueMorphology { + let ty: Ty<'tcx> = self.type_of(did).instantiate_identity(); + + // Async drop glue morphology is an internal detail, so reveal_all probably + // should be fine + let param_env = ty::ParamEnv::reveal_all(); + if ty.needs_async_drop(self, param_env) { + AsyncDropGlueMorphology::Custom + } else if ty.needs_drop(self, param_env) { + AsyncDropGlueMorphology::DeferredDropInPlace + } else { + AsyncDropGlueMorphology::Noop + } + } + /// Returns the set of types that are required to be alive in /// order to run the destructor of `def` (see RFCs 769 and /// 1238). @@ -1177,6 +1196,18 @@ impl<'tcx> TypeFolder> for WeakAliasTypeExpander<'tcx> { } } +/// Indicates the form of `AsyncDestruct::Destructor`. Used to simplify async +/// drop glue for types not using async drop. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum AsyncDropGlueMorphology { + /// Async destructor simply does nothing + Noop, + /// Async destructor simply runs `drop_in_place` + DeferredDropInPlace, + /// Async destructor has custom logic + Custom, +} + impl<'tcx> Ty<'tcx> { /// Returns the `Size` for primitive types (bool, uint, int, char, float). pub fn primitive_size(self, tcx: TyCtxt<'tcx>) -> Size { @@ -1342,27 +1373,16 @@ impl<'tcx> Ty<'tcx> { } } - /// Checks whether values of this type `T` implement has noop async destructor. + /// Get morphology of the async drop glue, needed for types which do not + /// use async drop. To get async drop glue morphology for a definition see + /// [`TyCtxt::async_drop_glue_morphology`]. Used for `AsyncDestruct::Destructor` + /// type construction. // - // FIXME: implement optimization to make ADTs, which do not need drop, - // to skip fields or to have noop async destructor, use `needs_(async_)drop` - pub fn is_async_destructor_noop( - self, - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - ) -> bool { - // TODO: check on the most generic version of your type - self.is_async_destructor_trivially_noop() - || self.needs_async_drop(tcx, param_env) - || self.needs_drop(tcx, param_env) - } - - /// Fast path helper for testing if a type has noop async destructor. - /// - /// Returning `true` means the type is known to have noop async destructor - /// implementation. Returning `true` means nothing -- could be - /// `Drop`, might not be. - fn is_async_destructor_trivially_noop(self) -> bool { + // FIXME: implement optimization to not instantiate a certain morphology of + // async drop glue too soon to allow per type optimizations, see array case + // for more info. Perhaps then remove this method and use `needs_(async_)drop` + // instead. + pub fn async_drop_glue_morphology(self, tcx: TyCtxt<'tcx>) -> AsyncDropGlueMorphology { match self.kind() { ty::Int(_) | ty::Uint(_) @@ -1374,37 +1394,43 @@ impl<'tcx> Ty<'tcx> { | ty::Ref(..) | ty::RawPtr(..) | ty::FnDef(..) - | ty::FnPtr(_) => true, - ty::Tuple(tys) => tys.is_empty(), - ty::Adt(adt_def, _) => adt_def.is_manually_drop(), - ty::Bool => todo!(), - ty::Char => todo!(), - ty::Int(_) => todo!(), - ty::Uint(_) => todo!(), - ty::Float(_) => todo!(), - ty::Adt(_, _) => todo!(), - ty::Foreign(_) => todo!(), - ty::Str => todo!(), - ty::Array(_, _) => todo!(), - ty::Pat(_, _) => todo!(), - ty::Slice(_) => todo!(), - ty::RawPtr(_, _) => todo!(), - ty::Ref(_, _, _) => todo!(), - ty::FnDef(_, _) => todo!(), - ty::FnPtr(_) => todo!(), - ty::Dynamic(_, _, _) => todo!(), - ty::Closure(_, _) => todo!(), - ty::CoroutineClosure(_, _) => todo!(), - ty::Coroutine(_, _) => todo!(), - ty::CoroutineWitness(_, _) => todo!(), - ty::Never => todo!(), - ty::Tuple(_) => todo!(), - ty::Alias(_, _) => todo!(), - ty::Param(_) => todo!(), - ty::Bound(_, _) => todo!(), - ty::Placeholder(_) => todo!(), - ty::Infer(_) => todo!(), - ty::Error(_) => todo!(), + | ty::FnPtr(_) + | ty::Infer(ty::FreshIntTy(_)) + | ty::Infer(ty::FreshFloatTy(_)) => AsyncDropGlueMorphology::Noop, + + ty::Tuple(tys) if tys.is_empty() => AsyncDropGlueMorphology::Noop, + ty::Adt(adt_def, _) if adt_def.is_manually_drop() => AsyncDropGlueMorphology::Noop, + + // Foreign types can never have destructors. + ty::Foreign(_) => AsyncDropGlueMorphology::Noop, + + // FIXME: implement dynamic types async drops + ty::Error(_) | ty::Dynamic(..) => AsyncDropGlueMorphology::DeferredDropInPlace, + + ty::Tuple(_) | ty::Array(_, _) | ty::Slice(_) => { + // Assume worst-case scenario, because we can instantiate async + // destructors in different orders: + // + // 1. Instantiate [T; N] with T = String and N = 0 + // 2. Instantiate <[String; 0] as AsyncDestruct>::Destructor + // + // And viceversa, thus we cannot rely on String not using async + // drop or array having zero (0) elements + AsyncDropGlueMorphology::Custom + } + ty::Pat(ty, _) => ty.async_drop_glue_morphology(tcx), + + ty::Adt(adt_def, _) => tcx.async_drop_glue_morphology(adt_def.did()), + + ty::Closure(did, _) + | ty::CoroutineClosure(did, _) + | ty::Coroutine(did, _) + | ty::CoroutineWitness(did, _) => tcx.async_drop_glue_morphology(*did), + + ty::Alias(..) | ty::Param(_) | ty::Bound(..) | ty::Placeholder(..) | ty::Infer(_) => { + // No specifics, but would usually mean forwarding async drop glue + AsyncDropGlueMorphology::Custom + } } } @@ -1451,7 +1477,11 @@ impl<'tcx> Ty<'tcx> { /// (Note that this implies that if `ty` has an async destructor attached, /// then `needs_async_drop` will definitely return `true` for `ty`.) /// - /// Note that this method is used to check eligible types in unions. + /// When constructing `AsyncDestruct::Destructor` type, use + /// [`Ty::async_drop_glue_morphology`] instead. + // + // FIXME(zetanumbers): Note that this method is used to check eligible types + // in unions. #[inline] pub fn needs_async_drop(self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { // Avoid querying in simple cases. @@ -1647,10 +1677,13 @@ impl<'tcx> ExplicitSelf<'tcx> { } } -// FIXME(zetanumbers): make specifying asyncness explicit /// Returns a list of types such that the given type needs drop if and only if /// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if /// this type always needs drop. +// +// FIXME(zetanumbers): consider replacing this with only +// `needs_drop_components_with_async` +#[inline] pub fn needs_drop_components<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, diff --git a/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs b/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs index f4481c22fc1fe..aa9c87d8f80d9 100644 --- a/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs +++ b/compiler/rustc_mir_transform/src/shim/async_destructor_ctor.rs @@ -12,7 +12,7 @@ use rustc_middle::mir::{ Terminator, TerminatorKind, UnwindAction, UnwindTerminateReason, RETURN_PLACE, }; use rustc_middle::ty::adjustment::PointerCoercion; -use rustc_middle::ty::util::Discr; +use rustc_middle::ty::util::{AsyncDropGlueMorphology, Discr}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::{bug, span_bug}; use rustc_span::source_map::respan; @@ -116,15 +116,25 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { } fn build(self) -> Body<'tcx> { - let (tcx, def_id, Some(self_ty)) = (self.tcx, self.def_id, self.self_ty) else { + let (tcx, Some(self_ty)) = (self.tcx, self.self_ty) else { return self.build_zst_output(); }; + match self_ty.async_drop_glue_morphology(tcx) { + AsyncDropGlueMorphology::Noop => span_bug!( + self.span, + "async drop glue shim generator encountered type with noop async drop glue morphology" + ), + AsyncDropGlueMorphology::DeferredDropInPlace => { + return self.build_deferred_drop_in_place(); + } + AsyncDropGlueMorphology::Custom => (), + } let surface_drop_kind = || { - let param_env = tcx.param_env_reveal_all_normalized(def_id); - if self_ty.has_surface_async_drop(tcx, param_env) { + let adt_def = self_ty.ty_adt_def()?; + if adt_def.async_destructor(tcx).is_some() { Some(SurfaceDropKind::Async) - } else if self_ty.has_surface_drop(tcx, param_env) { + } else if adt_def.destructor(tcx).is_some() { Some(SurfaceDropKind::Sync) } else { None @@ -267,6 +277,13 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { self.return_() } + fn build_deferred_drop_in_place(mut self) -> Body<'tcx> { + self.put_self(); + let deferred = self.combine_deferred_drop_in_place(); + self.combine_fuse(deferred); + self.return_() + } + fn build_fused_async_surface(mut self) -> Body<'tcx> { self.put_self(); let surface = self.combine_async_surface(); @@ -441,6 +458,14 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { ) } + fn combine_deferred_drop_in_place(&mut self) -> Ty<'tcx> { + self.apply_combinator( + 1, + LangItem::AsyncDropDeferredDropInPlace, + &[self.self_ty.unwrap().into()], + ) + } + fn combine_fuse(&mut self, inner_future_ty: Ty<'tcx>) -> Ty<'tcx> { self.apply_combinator(1, LangItem::AsyncDropFuse, &[inner_future_ty.into()]) } @@ -481,7 +506,7 @@ impl<'tcx> AsyncDestructorCtorShimBuilder<'tcx> { if let Some(ty) = self.self_ty { debug_assert_eq!( output.ty(&self.locals, self.tcx), - ty.async_destructor_ty(self.tcx, self.param_env), + ty.async_destructor_ty(self.tcx), "output async destructor types did not match for type: {ty:?}", ); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 90da220b3f549..f771b4686902e 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -430,6 +430,7 @@ symbols! { async_drop, async_drop_chain, async_drop_defer, + async_drop_deferred_drop_in_place, async_drop_either, async_drop_fuse, async_drop_in_place, diff --git a/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs b/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs index 7fd2a3801cc4c..9d4c03375fe81 100644 --- a/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs @@ -839,7 +839,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> { | ty::Str | ty::Slice(_) | ty::Tuple(_) - | ty::Error(_) => self_ty.async_destructor_ty(ecx.interner(), goal.param_env), + | ty::Error(_) => self_ty.async_destructor_ty(ecx.interner()), // We do not call `Ty::async_destructor_ty` on alias, param, or placeholder // types, which return `::AsyncDestructor` diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs index 87c8b1cda5041..0f8b003c07786 100644 --- a/compiler/rustc_trait_selection/src/traits/project.rs +++ b/compiler/rustc_trait_selection/src/traits/project.rs @@ -1560,7 +1560,7 @@ fn confirm_builtin_candidate<'cx, 'tcx>( let destructor_def_id = tcx.associated_item_def_ids(trait_def_id)[0]; assert_eq!(destructor_def_id, item_def_id); - (self_ty.async_destructor_ty(tcx, obligation.param_env).into(), Vec::new()) + (self_ty.async_destructor_ty(tcx).into(), Vec::new()) } else if lang_items.pointee_trait() == Some(trait_def_id) { let metadata_def_id = tcx.require_lang_item(LangItem::Metadata, None); assert_eq!(metadata_def_id, item_def_id); diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 0d089205c1ec9..e4dcea785d411 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -4,6 +4,7 @@ use rustc_infer::infer::TyCtxtInferExt; use rustc_middle::bug; use rustc_middle::query::Providers; use rustc_middle::traits::{BuiltinImplSource, CodegenObligationError}; +use rustc_middle::ty::util::AsyncDropGlueMorphology; use rustc_middle::ty::GenericArgsRef; use rustc_middle::ty::{self, Instance, TyCtxt, TypeVisitableExt}; use rustc_span::sym; @@ -59,7 +60,7 @@ fn resolve_instance<'tcx>( } else if Some(def_id) == tcx.lang_items().async_drop_in_place_fn() { let ty = args.type_at(0); - if !ty.is_async_destructor_noop(tcx, param_env) { + if ty.async_drop_glue_morphology(tcx) != AsyncDropGlueMorphology::Noop { match *ty.kind() { ty::Closure(..) | ty::CoroutineClosure(..) diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs index 7fc3543ff66a7..205b3f2760f36 100644 --- a/compiler/rustc_ty_utils/src/needs_drop.rs +++ b/compiler/rustc_ty_utils/src/needs_drop.rs @@ -36,7 +36,7 @@ fn needs_async_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty // it needs async drop. let adt_has_async_dtor = |adt_def: ty::AdtDef<'tcx>| adt_def.async_destructor(tcx).map(|_| DtorType::Significant); - let res = drop_tys_helper(tcx, query.value, query.param_env, adt_has_dtor, false) + let res = drop_tys_helper(tcx, query.value, query.param_env, adt_has_async_dtor, false) .filter(filter_array_elements(tcx, query.param_env)) .next() .is_some(); diff --git a/library/core/src/future/async_drop.rs b/library/core/src/future/async_drop.rs index 0eb8d7bb32899..ea7e2f52ba1db 100644 --- a/library/core/src/future/async_drop.rs +++ b/library/core/src/future/async_drop.rs @@ -161,6 +161,11 @@ async unsafe fn surface_drop_in_place(ptr: *mut T) { /// wrapped future completes by returning `Poll::Ready(())` on poll. This /// is useful for constructing async destructors to guarantee this /// "fuse" property +// +// FIXME: Consider optimizing combinators to not have to use fuse in majority +// of cases, perhaps by adding `#[(rustc_)idempotent(_future)]` attribute for +// async functions and blocks with the unit return type. However current layout +// optimizations currently encode `None` case into the async block's discriminant. struct Fuse { inner: Option, } @@ -251,6 +256,12 @@ async unsafe fn either, M: IntoFuture, T } } +#[cfg(not(bootstrap))] +#[lang = "async_drop_deferred_drop_in_place"] +async unsafe fn deferred_drop_in_place(to_drop: *mut T) { + unsafe { crate::ptr::drop_in_place(to_drop) } +} + /// Used for noop async destructors. We don't use [`core::future::Ready`] /// because it panics after its second poll, which could be potentially /// bad if that would happen during the cleanup. From 2892302aef73715678f604ccbbf8e371bdf0ab4e Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 3 May 2024 17:02:26 +0300 Subject: [PATCH 4/7] Add safety comment to fix tidy --- library/core/src/future/async_drop.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/core/src/future/async_drop.rs b/library/core/src/future/async_drop.rs index ea7e2f52ba1db..25138c445c437 100644 --- a/library/core/src/future/async_drop.rs +++ b/library/core/src/future/async_drop.rs @@ -259,6 +259,8 @@ async unsafe fn either, M: IntoFuture, T #[cfg(not(bootstrap))] #[lang = "async_drop_deferred_drop_in_place"] async unsafe fn deferred_drop_in_place(to_drop: *mut T) { + // SAFETY: same safety requirements as with drop_in_place (implied by + // function's name) unsafe { crate::ptr::drop_in_place(to_drop) } } From 39e193964e6b35e57b1e04f81831c625bd9e0c4c Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 20 May 2024 16:22:02 +0300 Subject: [PATCH 5/7] fix non-existing ToPredicate trait error --- compiler/rustc_middle/src/ty/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 2f2d03e9ec841..52a0e72e17e28 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -4,8 +4,8 @@ use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags; use crate::query::{IntoQueryParam, Providers}; use crate::ty::layout::{FloatExt, IntegerExt}; use crate::ty::{ - self, Asyncness, FallibleTypeFolder, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, - TypeSuperFoldable, TypeVisitableExt, Upcast, + self, Asyncness, FallibleTypeFolder, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, + TypeVisitableExt, Upcast, }; use crate::ty::{GenericArgKind, GenericArgsRef}; use rustc_apfloat::Float as _; From 57a44b21197a7156608ec2d15cf6967e64f04db9 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 24 May 2024 18:03:36 +0300 Subject: [PATCH 6/7] Bless new async destructor sizes in async drop tests --- src/tools/miri/tests/pass/async-drop.rs | 10 +++++----- tests/ui/async-await/async-drop.rs | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/tests/pass/async-drop.rs b/src/tools/miri/tests/pass/async-drop.rs index 744ffdeebdaf7..c289c9410b258 100644 --- a/src/tools/miri/tests/pass/async-drop.rs +++ b/src/tools/miri/tests/pass/async-drop.rs @@ -46,7 +46,7 @@ fn main() { let i = 13; let fut = pin!(async { - test_async_drop(Int(0), 16).await; + test_async_drop(Int(0), 0).await; test_async_drop(AsyncInt(0), 104).await; test_async_drop([AsyncInt(1), AsyncInt(2)], 152).await; test_async_drop((AsyncInt(3), AsyncInt(4)), 488).await; @@ -71,13 +71,13 @@ fn main() { ) .await; - test_async_drop(AsyncEnum::A(AsyncInt(12)), 792).await; - test_async_drop(AsyncEnum::B(SyncInt(13)), 792).await; + test_async_drop(AsyncEnum::A(AsyncInt(12)), 680).await; + test_async_drop(AsyncEnum::B(SyncInt(13)), 680).await; - test_async_drop(SyncInt(14), 72).await; + test_async_drop(SyncInt(14), 16).await; test_async_drop( SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }, - 3512, + 3064, ) .await; diff --git a/tests/ui/async-await/async-drop.rs b/tests/ui/async-await/async-drop.rs index d1222cd740c32..97b5a602bc377 100644 --- a/tests/ui/async-await/async-drop.rs +++ b/tests/ui/async-await/async-drop.rs @@ -48,7 +48,7 @@ fn main() { let i = 13; let fut = pin!(async { - test_async_drop(Int(0), 16).await; + test_async_drop(Int(0), 0).await; test_async_drop(AsyncInt(0), 104).await; test_async_drop([AsyncInt(1), AsyncInt(2)], 152).await; test_async_drop((AsyncInt(3), AsyncInt(4)), 488).await; @@ -73,13 +73,13 @@ fn main() { ) .await; - test_async_drop(AsyncEnum::A(AsyncInt(12)), 792).await; - test_async_drop(AsyncEnum::B(SyncInt(13)), 792).await; + test_async_drop(AsyncEnum::A(AsyncInt(12)), 680).await; + test_async_drop(AsyncEnum::B(SyncInt(13)), 680).await; - test_async_drop(SyncInt(14), 72).await; + test_async_drop(SyncInt(14), 16).await; test_async_drop( SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }, - 3512, + 3064, ) .await; From 4903cf4df6f6647177f1645ab17ebab933564949 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 27 May 2024 15:53:25 +0300 Subject: [PATCH 7/7] Revert miri async drop test but add warnings to each async drop test --- src/tools/miri/tests/pass/async-drop.rs | 84 ++++++++++--------------- tests/ui/async-await/async-drop.rs | 4 ++ 2 files changed, 38 insertions(+), 50 deletions(-) diff --git a/src/tools/miri/tests/pass/async-drop.rs b/src/tools/miri/tests/pass/async-drop.rs index c289c9410b258..92ecbdd29fdb9 100644 --- a/src/tools/miri/tests/pass/async-drop.rs +++ b/src/tools/miri/tests/pass/async-drop.rs @@ -1,6 +1,11 @@ //@revisions: stack tree //@compile-flags: -Zmiri-strict-provenance //@[tree]compile-flags: -Zmiri-tree-borrows + +// WARNING: If you would ever want to modify this test, +// please consider modifying rustc's async drop test at +// `tests/ui/async-await/async-drop.rs`. + #![feature(async_drop, impl_trait_in_assoc_type, noop_waker, async_closure)] #![allow(incomplete_features, dead_code)] @@ -11,21 +16,9 @@ use core::mem::{self, ManuallyDrop}; use core::pin::{pin, Pin}; use core::task::{Context, Poll, Waker}; -async fn test_async_drop(x: T, _size: usize) { +async fn test_async_drop(x: T) { let mut x = mem::MaybeUninit::new(x); let dtor = pin!(unsafe { async_drop_in_place(x.as_mut_ptr()) }); - - // FIXME(zetanumbers): This check fully depends on the layout of - // the coroutine state, since async destructor combinators are just - // async functions. - #[cfg(target_pointer_width = "64")] - assert_eq!( - mem::size_of_val(&*dtor), - _size, - "sizes did not match for async destructor of type {}", - core::any::type_name::(), - ); - test_idempotency(dtor).await; } @@ -46,58 +39,49 @@ fn main() { let i = 13; let fut = pin!(async { - test_async_drop(Int(0), 0).await; - test_async_drop(AsyncInt(0), 104).await; - test_async_drop([AsyncInt(1), AsyncInt(2)], 152).await; - test_async_drop((AsyncInt(3), AsyncInt(4)), 488).await; - test_async_drop(5, 0).await; + test_async_drop(Int(0)).await; + test_async_drop(AsyncInt(0)).await; + test_async_drop([AsyncInt(1), AsyncInt(2)]).await; + test_async_drop((AsyncInt(3), AsyncInt(4))).await; + test_async_drop(5).await; let j = 42; - test_async_drop(&i, 0).await; - test_async_drop(&j, 0).await; - test_async_drop(AsyncStruct { b: AsyncInt(8), a: AsyncInt(7), i: 6 }, 1688).await; - test_async_drop(ManuallyDrop::new(AsyncInt(9)), 0).await; + test_async_drop(&i).await; + test_async_drop(&j).await; + test_async_drop(AsyncStruct { b: AsyncInt(8), a: AsyncInt(7), i: 6 }).await; + test_async_drop(ManuallyDrop::new(AsyncInt(9))).await; let foo = AsyncInt(10); - test_async_drop(AsyncReference { foo: &foo }, 104).await; + test_async_drop(AsyncReference { foo: &foo }).await; let foo = AsyncInt(11); - test_async_drop( - || { - black_box(foo); - let foo = AsyncInt(10); - foo - }, - 120, - ) + test_async_drop(|| { + black_box(foo); + let foo = AsyncInt(10); + foo + }) .await; - test_async_drop(AsyncEnum::A(AsyncInt(12)), 680).await; - test_async_drop(AsyncEnum::B(SyncInt(13)), 680).await; + test_async_drop(AsyncEnum::A(AsyncInt(12))).await; + test_async_drop(AsyncEnum::B(SyncInt(13))).await; - test_async_drop(SyncInt(14), 16).await; - test_async_drop( - SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }, - 3064, - ) - .await; + test_async_drop(SyncInt(14)).await; + test_async_drop(SyncThenAsync { i: 15, a: AsyncInt(16), b: SyncInt(17), c: AsyncInt(18) }) + .await; let async_drop_fut = pin!(core::future::async_drop(AsyncInt(19))); test_idempotency(async_drop_fut).await; let foo = AsyncInt(20); - test_async_drop( - async || { - black_box(foo); - let foo = AsyncInt(19); - // Await point there, but this is async closure so it's fine - black_box(core::future::ready(())).await; - foo - }, - 120, - ) + test_async_drop(async || { + black_box(foo); + let foo = AsyncInt(19); + // Await point there, but this is async closure so it's fine + black_box(core::future::ready(())).await; + foo + }) .await; - test_async_drop(AsyncUnion { signed: 21 }, 32).await; + test_async_drop(AsyncUnion { signed: 21 }).await; }); let res = fut.poll(&mut cx); assert_eq!(res, Poll::Ready(())); diff --git a/tests/ui/async-await/async-drop.rs b/tests/ui/async-await/async-drop.rs index 97b5a602bc377..12f120a0b1216 100644 --- a/tests/ui/async-await/async-drop.rs +++ b/tests/ui/async-await/async-drop.rs @@ -1,6 +1,10 @@ //@ run-pass //@ check-run-results +// WARNING: If you would ever want to modify this test, +// please consider modifying miri's async drop test at +// `src/tools/miri/tests/pass/async-drop.rs`. + #![feature(async_drop, impl_trait_in_assoc_type, noop_waker, async_closure)] #![allow(incomplete_features, dead_code)]