Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ fn exported_symbols_provider_local(
));
}
MonoItem::Fn(Instance {
def: InstanceDef::AsyncDropGlueCtorShim(def_id, ty),
def: InstanceDef::AsyncDropGlueCtorShim(def_id, Some(ty)),
args,
}) => {
// A little sanity-check
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ fn dump_path<'tcx>(
}));
s
}
ty::InstanceDef::AsyncDropGlueCtorShim(_, ty) => {
ty::InstanceDef::AsyncDropGlueCtorShim(_, Some(ty)) => {
// Unfortunately, pretty-printed typed are not very filename-friendly.
// We dome some filtering.
let mut s = ".".to_owned();
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,14 @@ macro_rules! make_mir_visitor {
receiver_by_ref: _,
} |
ty::InstanceDef::CoroutineKindShim { coroutine_def_id: _def_id } |
ty::InstanceDef::AsyncDropGlueCtorShim(_def_id, None) |
ty::InstanceDef::DropGlue(_def_id, None) => {}

ty::InstanceDef::FnPtrShim(_def_id, ty) |
ty::InstanceDef::DropGlue(_def_id, Some(ty)) |
ty::InstanceDef::CloneShim(_def_id, ty) |
ty::InstanceDef::FnPtrAddrShim(_def_id, ty) |
ty::InstanceDef::AsyncDropGlueCtorShim(_def_id, ty) => {
ty::InstanceDef::AsyncDropGlueCtorShim(_def_id, Some(ty)) => {
// FIXME(eddyb) use a better `TyContext` here.
self.visit_ty($(& $mutability)? *ty, TyContext::Location(location));
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub enum InstanceDef<'tcx> {
///
/// The `DefId` is for `core::future::async_drop::async_drop_in_place`, the `Ty`
/// is the type `T`.
AsyncDropGlueCtorShim(DefId, Ty<'tcx>),
AsyncDropGlueCtorShim(DefId, Option<Ty<'tcx>>),
}

impl<'tcx> Instance<'tcx> {
Expand Down Expand Up @@ -406,7 +406,8 @@ fn fmt_instance(
InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({ty}))"),
InstanceDef::CloneShim(_, ty) => write!(f, " - shim({ty})"),
InstanceDef::FnPtrAddrShim(_, ty) => write!(f, " - shim({ty})"),
InstanceDef::AsyncDropGlueCtorShim(_, ty) => write!(f, " - shim({ty})"),
InstanceDef::AsyncDropGlueCtorShim(_, None) => write!(f, " - shim(None)"),
InstanceDef::AsyncDropGlueCtorShim(_, Some(ty)) => write!(f, " - shim(Some({ty}))"),
}
}

Expand Down
43 changes: 12 additions & 31 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,10 @@ 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(_)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer self.references_error() over matching for ty::Error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither drop glue nor discriminant kind do this. I will have to switch every match on ty::Error otherwise compiler may emit type errors.

Copy link
Contributor Author

@zetanumbers zetanumbers Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I cannot add this case to be a part of is_async_destructor_noop because of this line, which differs from other similar types like ints:

But I'm not sure, maybe I should include it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I guess we can look if there's a general thing to improve here everywhere

return Ty::async_destructor_combinator(tcx, LangItem::AsyncDropNoop)
.instantiate_identity();
}
match *self.kind() {
ty::Param(_) | ty::Alias(..) | ty::Infer(ty::TyVar(_)) => {
let assoc_items = tcx
Expand All @@ -2333,9 +2337,6 @@ impl<'tcx> Ty<'tcx> {
.instantiate(tcx, &[dtor.into()])
}

ty::Adt(adt_def, _) if adt_def.is_manually_drop() => {
Ty::async_destructor_combinator(tcx, LangItem::AsyncDropNoop).instantiate_identity()
}
ty::Adt(adt_def, args) if adt_def.is_enum() || adt_def.is_struct() => self
.adt_async_destructor_ty(
tcx,
Expand All @@ -2357,34 +2358,10 @@ impl<'tcx> Ty<'tcx> {
ty::Adt(adt_def, _) => {
assert!(adt_def.is_union());

match self.surface_async_dropper_ty(tcx, param_env) {
None => Ty::async_destructor_combinator(tcx, LangItem::AsyncDropNoop)
.instantiate_identity(),
Some(surface_drop) => {
Ty::async_destructor_combinator(tcx, LangItem::AsyncDropFuse)
.instantiate(tcx, &[surface_drop.into()])
}
}
}

ty::Never
| ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Str
| ty::RawPtr(_, _)
| ty::Ref(..)
| ty::FnDef(..)
| ty::FnPtr(..)
| ty::Infer(ty::IntVar(_) | ty::FloatVar(_))
| ty::Error(_) => {
Ty::async_destructor_combinator(tcx, LangItem::AsyncDropNoop).instantiate_identity()
}
let surface_drop = self.surface_async_dropper_ty(tcx, param_env).unwrap();

ty::Dynamic(..) | ty::CoroutineWitness(..) | ty::Coroutine(..) | ty::Pat(..) => {
bug!("`async_destructor_ty` is not yet implemented for type: {self:?}")
Ty::async_destructor_combinator(tcx, LangItem::AsyncDropFuse)
.instantiate(tcx, &[surface_drop.into()])
}

ty::Bound(..)
Expand All @@ -2393,6 +2370,8 @@ impl<'tcx> Ty<'tcx> {
| ty::Infer(ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_)) => {
bug!("`async_destructor_ty` applied to unexpected type: {self:?}")
}

_ => bug!("`async_destructor_ty` is not yet implemented for type: {self:?}"),
}
}

Expand All @@ -2406,6 +2385,8 @@ impl<'tcx> Ty<'tcx> {
I: Iterator + ExactSizeIterator,
I::Item: IntoIterator<Item = Ty<'tcx>>,
{
debug_assert!(!self.is_async_destructor_noop(tcx, param_env));

let defer = Ty::async_destructor_combinator(tcx, LangItem::AsyncDropDefer);
let chain = Ty::async_destructor_combinator(tcx, LangItem::AsyncDropChain);

Expand All @@ -2425,7 +2406,7 @@ impl<'tcx> Ty<'tcx> {
.reduce(|other, matched| {
either.instantiate(tcx, &[other.into(), matched.into(), self.into()])
})
.unwrap_or(noop);
.unwrap();

let dtor = if let Some(dropper_ty) = self.surface_async_dropper_ty(tcx, param_env) {
Ty::async_destructor_combinator(tcx, LangItem::AsyncDropChain)
Expand Down
117 changes: 59 additions & 58 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1306,8 +1306,7 @@ 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.trivially_has_surface_async_drop()
&& tcx.has_surface_async_drop_raw(param_env.and(self))
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`
Expand All @@ -1316,52 +1315,68 @@ impl<'tcx> Ty<'tcx> {
/// Returning `false` means the type is known to not have `AsyncDrop`
/// implementation. Returning `true` means nothing -- could be
/// `AsyncDrop`, might not be.
fn trivially_has_surface_async_drop(self) -> bool {
match self.kind() {
ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Bool
| ty::Char
| ty::Str
| ty::Never
| ty::Ref(..)
| ty::RawPtr(_, _)
| ty::FnDef(..)
| ty::FnPtr(_)
| ty::Error(_)
| ty::Tuple(_)
| ty::Slice(_)
| ty::Array(_, _)
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(..)
| ty::CoroutineWitness(..)
| ty::Pat(..) => false,
ty::Adt(..)
| ty::Bound(..)
| ty::Dynamic(..)
| ty::Foreign(_)
| ty::Infer(_)
| ty::Alias(..)
| ty::Param(_)
| ty::Placeholder(_) => true,
}
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 `AsyncDrop`
/// trait.
pub fn has_surface_drop(self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool {
self.trivially_has_surface_drop() && tcx.has_surface_drop_raw(param_env.and(self))
self.could_have_surface_drop() && tcx.has_surface_drop_raw(param_env.and(self))
}

/// Fast path helper for testing if a type has `AsyncDrop`
/// implementation.
/// Fast path helper for testing if a type has `Drop` implementation.
///
/// Returning `false` means the type is known to not have `AsyncDrop`
/// Returning `false` means the type is known to not have `Drop`
/// implementation. Returning `true` means nothing -- could be
/// `AsyncDrop`, might not be.
fn trivially_has_surface_drop(self) -> bool {
/// `Drop`, might not be.
fn could_have_surface_drop(self) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this relate to is_trivially_const_drop and can we deduplicate some code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_trivially_const_drop recurses into array and tuple elements to determine if type can be trivially destructed, while could_have_surface_drop only checks if there's possible impl Drop for T. This function would return false for a tuple with any collection of types, while !is_trivially_const_drop for the same tuple may be true because of some element type could be an ADT. I could use could_have_surface_drop from is_trivially_const_drop since the former's negative implies the latter if it's ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in contrast is_trivially_const_drop does not check/intend to check ManuallyDrop, so those could mean a bit different things after all.

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.
pub fn is_async_destructor_noop(
self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> bool {
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
}
}

/// 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 {
match self.kind() {
ty::Int(_)
| ty::Uint(_)
Expand All @@ -1371,26 +1386,12 @@ impl<'tcx> Ty<'tcx> {
| ty::Str
| ty::Never
| ty::Ref(..)
| ty::RawPtr(_, _)
| ty::RawPtr(..)
| ty::FnDef(..)
| ty::FnPtr(_)
| ty::Error(_)
| ty::Tuple(_)
| ty::Slice(_)
| ty::Array(_, _)
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(..)
| ty::CoroutineWitness(..)
| ty::Pat(..) => false,
ty::Adt(..)
| ty::Bound(..)
| ty::Dynamic(..)
| ty::Foreign(_)
| ty::Infer(_)
| ty::Alias(..)
| ty::Param(_)
| ty::Placeholder(_) => true,
| ty::FnPtr(_) => true,
ty::Tuple(tys) => tys.is_empty(),
ty::Adt(adt_def, _) => adt_def.is_manually_drop(),
_ => false,
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,8 +1072,8 @@ fn try_instance_mir<'tcx>(
tcx: TyCtxt<'tcx>,
instance: InstanceDef<'tcx>,
) -> Result<&'tcx Body<'tcx>, &'static str> {
if let ty::InstanceDef::DropGlue(_, Some(ty)) | ty::InstanceDef::AsyncDropGlueCtorShim(_, ty) =
instance
if let ty::InstanceDef::DropGlue(_, Some(ty))
| ty::InstanceDef::AsyncDropGlueCtorShim(_, Some(ty)) = instance
&& let ty::Adt(def, args) = ty.kind()
{
let fields = def.all_fields();
Expand Down
Loading