Skip to content

Make async drop code more consistent with regular drop code #126594

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

Merged
merged 2 commits into from
Jun 19, 2024
Merged
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
23 changes: 13 additions & 10 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ fn exported_symbols_provider_local(
}) => {
// A little sanity-check
debug_assert_eq!(
args.non_erasable_generics(tcx, def_id).skip(1).next(),
args.non_erasable_generics(tcx, def_id).next(),
Some(GenericArgKind::Type(ty))
);
symbols.push((
Expand Down Expand Up @@ -422,10 +422,7 @@ fn upstream_monomorphizations_provider(
}
ExportedSymbol::AsyncDropGlueCtorShim(ty) => {
if let Some(async_drop_in_place_fn_def_id) = async_drop_in_place_fn_def_id {
(
async_drop_in_place_fn_def_id,
tcx.mk_args(&[tcx.lifetimes.re_erased.into(), ty.into()]),
)
(async_drop_in_place_fn_def_id, tcx.mk_args(&[ty.into()]))
} else {
// `drop_in_place` in place does not exist, don't try
// to use it.
Expand Down Expand Up @@ -473,11 +470,16 @@ fn upstream_drop_glue_for_provider<'tcx>(
tcx: TyCtxt<'tcx>,
args: GenericArgsRef<'tcx>,
) -> Option<CrateNum> {
if let Some(def_id) = tcx.lang_items().drop_in_place_fn() {
tcx.upstream_monomorphizations_for(def_id).and_then(|monos| monos.get(&args).cloned())
} else {
None
}
let def_id = tcx.lang_items().drop_in_place_fn()?;
tcx.upstream_monomorphizations_for(def_id)?.get(&args).cloned()
}

fn upstream_async_drop_glue_for_provider<'tcx>(
tcx: TyCtxt<'tcx>,
args: GenericArgsRef<'tcx>,
) -> Option<CrateNum> {
let def_id = tcx.lang_items().async_drop_in_place_fn()?;
tcx.upstream_monomorphizations_for(def_id)?.get(&args).cloned()
}

fn is_unreachable_local_definition_provider(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
Expand All @@ -491,6 +493,7 @@ pub fn provide(providers: &mut Providers) {
providers.upstream_monomorphizations = upstream_monomorphizations_provider;
providers.is_unreachable_local_definition = is_unreachable_local_definition_provider;
providers.upstream_drop_glue_for = upstream_drop_glue_for_provider;
providers.upstream_async_drop_glue_for = upstream_async_drop_glue_for_provider;
providers.wasm_import_module_map = wasm_import_module_map;
providers.extern_queries.is_reachable_non_generic = is_reachable_non_generic_provider_extern;
providers.extern_queries.upstream_monomorphizations_for =
Expand Down
33 changes: 27 additions & 6 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1554,12 +1554,13 @@ rustc_queries! {
}
}

/// The entire set of monomorphizations the local crate can safely link
/// to because they are exported from upstream crates. Do not depend on
/// this directly, as its value changes anytime a monomorphization gets
/// added or removed in any upstream crate. Instead use the narrower
/// `upstream_monomorphizations_for`, `upstream_drop_glue_for`, or, even
/// better, `Instance::upstream_monomorphization()`.
/// The entire set of monomorphizations the local crate can safely
/// link to because they are exported from upstream crates. Do
/// not depend on this directly, as its value changes anytime
/// a monomorphization gets added or removed in any upstream
/// crate. Instead use the narrower `upstream_monomorphizations_for`,
/// `upstream_drop_glue_for`, `upstream_async_drop_glue_for`, or,
/// even better, `Instance::upstream_monomorphization()`.
query upstream_monomorphizations(_: ()) -> &'tcx DefIdMap<UnordMap<GenericArgsRef<'tcx>, CrateNum>> {
arena_cache
desc { "collecting available upstream monomorphizations" }
Expand Down Expand Up @@ -1601,6 +1602,26 @@ rustc_queries! {
desc { "available upstream drop-glue for `{:?}`", args }
}

/// Returns the upstream crate that exports async-drop-glue for
/// the given type (`args` is expected to be a single-item list
/// containing the type one wants async-drop-glue for).
///
/// This is a subset of `upstream_monomorphizations_for` in order
/// to increase dep-tracking granularity. Otherwise adding or
/// removing any type with async-drop-glue in any upstream crate
/// would invalidate all functions calling async-drop-glue of an
/// upstream type.
///
/// You likely want to call `Instance::upstream_monomorphization()`
/// instead of invoking this query directly.
///
/// NOTE: This query could easily be extended to also support other
/// common functions that have are large set of monomorphizations
/// (like `Clone::clone` for example).
query upstream_async_drop_glue_for(args: GenericArgsRef<'tcx>) -> Option<CrateNum> {
desc { "available upstream async-drop-glue for `{:?}`", args }
}

/// Returns a list of all `extern` blocks of a crate.
query foreign_modules(_: CrateNum) -> &'tcx FxIndexMap<DefId, ForeignModule> {
arena_cache
Expand Down
23 changes: 16 additions & 7 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,9 @@ impl<'tcx> Instance<'tcx> {
InstanceKind::Item(def) => tcx
.upstream_monomorphizations_for(def)
.and_then(|monos| monos.get(&self.args).cloned()),
InstanceKind::DropGlue(_, Some(_)) | InstanceKind::AsyncDropGlueCtorShim(_, _) => {
tcx.upstream_drop_glue_for(self.args)
InstanceKind::DropGlue(_, Some(_)) => tcx.upstream_drop_glue_for(self.args),
InstanceKind::AsyncDropGlueCtorShim(_, Some(_)) => {
tcx.upstream_async_drop_glue_for(self.args)
}
_ => None,
}
Expand Down Expand Up @@ -256,7 +257,7 @@ impl<'tcx> InstanceKind<'tcx> {
match self {
ty::InstanceKind::Item(def) => Some(def),
ty::InstanceKind::DropGlue(def_id, Some(_))
| InstanceKind::AsyncDropGlueCtorShim(def_id, _)
| InstanceKind::AsyncDropGlueCtorShim(def_id, Some(_))
| InstanceKind::ThreadLocalShim(def_id) => Some(def_id),
InstanceKind::VTableShim(..)
| InstanceKind::ReifyShim(..)
Expand All @@ -267,6 +268,7 @@ impl<'tcx> InstanceKind<'tcx> {
| ty::InstanceKind::ConstructCoroutineInClosureShim { .. }
| ty::InstanceKind::CoroutineKindShim { .. }
| InstanceKind::DropGlue(..)
| InstanceKind::AsyncDropGlueCtorShim(..)
| InstanceKind::CloneShim(..)
| InstanceKind::FnPtrAddrShim(..) => None,
}
Expand Down Expand Up @@ -312,7 +314,9 @@ impl<'tcx> InstanceKind<'tcx> {
if self.requires_inline(tcx) {
return true;
}
if let ty::InstanceKind::DropGlue(.., Some(ty)) = *self {
if let ty::InstanceKind::DropGlue(.., Some(ty))
| ty::InstanceKind::AsyncDropGlueCtorShim(.., Some(ty)) = *self
{
// Drop glue generally wants to be instantiated at every codegen
// unit, but without an #[inline] hint. We should make this
// available to normal end-users.
Expand All @@ -327,9 +331,14 @@ impl<'tcx> InstanceKind<'tcx> {
// drops of `Option::None` before LTO. We also respect the intent of
// `#[inline]` on `Drop::drop` implementations.
return ty.ty_adt_def().map_or(true, |adt_def| {
adt_def
.destructor(tcx)
.map_or_else(|| adt_def.is_enum(), |dtor| tcx.cross_crate_inlinable(dtor.did))
match *self {
ty::InstanceKind::DropGlue(..) => adt_def.destructor(tcx).map(|dtor| dtor.did),
ty::InstanceKind::AsyncDropGlueCtorShim(..) => {
adt_def.async_destructor(tcx).map(|dtor| dtor.ctor)
}
_ => unreachable!(),
}
.map_or_else(|| adt_def.is_enum(), |did| tcx.cross_crate_inlinable(did))
});
}
if let ty::InstanceKind::ThreadLocalShim(..) = *self {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ pub fn transform_instance<'tcx>(
mut instance: Instance<'tcx>,
options: TransformTyOptions,
) -> Instance<'tcx> {
// FIXME: account for async-drop-glue
if (matches!(instance.def, ty::InstanceKind::Virtual(..))
&& tcx.is_lang_item(instance.def_id(), LangItem::DropInPlace))
|| matches!(instance.def, ty::InstanceKind::DropGlue(..))
Expand Down
Loading