Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 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
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_cranelift/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,13 @@ pub trait MetaSized: PointeeSized {}
pub trait Sized: MetaSized {}

#[lang = "destruct"]
pub trait Destruct {}
pub trait Destruct {
/// Entrypoint for drop
///
/// Generated by default if not implemented manually.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(_to_drop: *mut Self);
Copy link
Copy Markdown
Member

@Nadrieril Nadrieril May 6, 2026

Choose a reason for hiding this comment

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

To limit the amount of trait-related magic, I would rather do this:

Suggested change
unsafe fn drop_in_place(_to_drop: *mut Self);
/// Entrypoint for drop. Called when a value is still live at end of scope
/// if none of its fields have been moved out of.
///
/// The default implementation calls `Drop::drop` if implemented,
/// then recursively calls `drop_in_place` on each field in order.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(to_drop: *mut Self) {
core::intrinsics::drop_in_place_shim(to_drop);
}

and then have the intrinsic be the only thing that's auto-generated.

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

IMO there isn't much trait magic right now; the resolver simply checks if there are any user impls, and only assembles builtin ones if there aren't any. IMO this indirection would be rather confusing and also might clash with Specialization

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have the opposite intuition: having a method that magically has a body generated for it is not something that exists in rust; only intrinsics do that. And I don't see why this would interact with specialization in any way. Moreover this has the benefit that if someone wants to they can call the auto-generated drop glue.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The person can still invoke Destruct::drop_in_place manually, although it causes smth like a double drop issue.

Hey i was dropped
test
Hey i was dropped

I'll work on this. If you're talking about having both a custom dtor and the default one, i see very little usecase for that. Maybe that's just my intuition, but i prefer some less indirection, as this approach makes it more obvious how drop is being called (its just a function call, rather than what Drop::drop did, which called ptr::drop_in_place)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I mean if I want to override Destruct::drop_in_place in a way that reuses the default thing, e.g. because you want to call it conditionally. I agree it's probably rare.

its just a function call, rather than what Drop::drop did, which called ptr::drop_in_place

I'm confused: Drop::drop never called ptr::drop_in_place.

}

#[lang = "tuple_trait"]
pub trait Tuple {}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_codegen_gcc/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ pub trait MetaSized: PointeeSized {}
pub trait Sized: MetaSized {}

#[lang = "destruct"]
pub trait Destruct {}
pub trait Destruct {
/// Entrypoint for drop
///
/// Generated by default if not implemented manually.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(_to_drop: *mut Self);
}

#[lang = "tuple_trait"]
pub trait Tuple {}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ fn upstream_monomorphizations_provider(

let mut instances: DefIdMap<UnordMap<_, _>> = Default::default();

let drop_in_place_fn_def_id = tcx.lang_items().drop_in_place_fn();
let drop_in_place_fn_def_id = tcx.lang_items().destruct_drop_in_place();
let async_drop_in_place_fn_def_id = tcx.lang_items().async_drop_in_place_fn();

for &cnum in cnums.iter() {
Expand Down Expand Up @@ -465,7 +465,7 @@ fn upstream_drop_glue_for_provider<'tcx>(
tcx: TyCtxt<'tcx>,
args: GenericArgsRef<'tcx>,
) -> Option<CrateNum> {
let def_id = tcx.lang_items().drop_in_place_fn()?;
let def_id = tcx.lang_items().destruct_drop_in_place()?;
tcx.upstream_monomorphizations_for(def_id)?.get(&args).cloned()
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ language_item_table! {

Drop, sym::drop, drop_trait, Target::Trait, GenericRequirement::None;
Destruct, sym::destruct, destruct_trait, Target::Trait, GenericRequirement::None;
DestructDropInPlace, sym::destruct_drop_in_place, destruct_drop_in_place, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;
AsyncDrop, sym::async_drop, async_drop_trait, Target::Trait, GenericRequirement::None;
AsyncDropInPlace, sym::async_drop_in_place, async_drop_in_place_fn, Target::Fn, GenericRequirement::Exact(1);

Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ pub enum InstanceKind<'tcx> {
/// Proxy shim for async drop of future (def_id, proxy_cor_ty, impl_cor_ty)
FutureDropPollShim(DefId, Ty<'tcx>, Ty<'tcx>),

/// `core::ptr::drop_in_place::<T>`.
/// `Destruct::drop_in_place()`
///
/// The `DefId` is for `core::ptr::drop_in_place`.
/// The `Option<Ty<'tcx>>` is either `Some(T)`, or `None` for empty drop
Expand Down Expand Up @@ -717,7 +717,7 @@ impl<'tcx> Instance<'tcx> {
}

pub fn resolve_drop_in_place(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> ty::Instance<'tcx> {
let def_id = tcx.require_lang_item(LangItem::DropInPlace, DUMMY_SP);
let def_id = tcx.require_lang_item(LangItem::DestructDropInPlace, DUMMY_SP);
let args = tcx.mk_args(&[ty.into()]);
Instance::expect_resolve(
tcx,
Expand All @@ -728,6 +728,16 @@ impl<'tcx> Instance<'tcx> {
)
}

pub fn try_resolve_drop_in_place(
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
ty: Ty<'tcx>,
) -> Result<Option<Instance<'tcx>>, ErrorGuaranteed> {
let def_id = tcx.require_lang_item(LangItem::DestructDropInPlace, DUMMY_SP);
let args = tcx.mk_args(&[ty.into()]);
Instance::try_resolve(tcx, typing_env, def_id, args)
}

pub fn resolve_async_drop_in_place(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> ty::Instance<'tcx> {
let def_id = tcx.require_lang_item(LangItem::AsyncDropInPlace, DUMMY_SP);
let args = tcx.mk_args(&[ty.into()]);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,9 @@ fn visit_instance_use<'tcx>(
/// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we
/// can just link to the upstream crate and therefore don't need a mono item.
fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> bool {
if let ty::InstanceKind::DropGlue(_, Some(_)) = instance.def {
return instance.upstream_monomorphization(tcx).is_none();
}
let Some(def_id) = instance.def.def_id_if_not_guaranteed_local_codegen() else {
return true;
};
Expand Down
65 changes: 39 additions & 26 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,34 +454,47 @@ where

match assemble_from {
AssembleCandidatesFrom::All => {
self.assemble_builtin_impl_candidates(goal, &mut candidates);
// For performance we only assemble impls if there are no candidates
// which would shadow them. This is necessary to avoid hangs in rayon,
// see trait-system-refactor-initiative#109 for more details.
//
// We always assemble builtin impls as trivial builtin impls have a higher
// priority than where-clauses.
//
// We only do this if any such candidate applies without any constraints
// as we may want to weaken inference guidance in the future and don't want
// to worry about causing major performance regressions when doing so.
// See trait-system-refactor-initiative#226 for some ideas here.
let assemble_impls = match self.typing_mode() {
TypingMode::Coherence => true,
TypingMode::Analysis { .. }
| TypingMode::Borrowck { .. }
| TypingMode::PostBorrowckAnalysis { .. }
| TypingMode::PostAnalysis => !candidates.iter().any(|c| {
matches!(
c.source,
CandidateSource::ParamEnv(ParamEnvSource::NonGlobal)
| CandidateSource::AliasBound(_)
) && has_no_inference_or_external_constraints(c.result)
}),
};
if assemble_impls {
let trait_def_id = goal.predicate.trait_def_id(self.cx());
// Check if there are any user defined impls for Destruct. If there are,
// use those, and fallback to builtin drop glue impl if none are present
if self.cx().is_trait_lang_item(trait_def_id, SolverTraitLangItem::Destruct) {
self.assemble_impl_candidates(goal, &mut candidates);
let has_impl_candidate =
candidates.iter().any(|c| matches!(c.source, CandidateSource::Impl(_)));
if !has_impl_candidate {
self.assemble_builtin_impl_candidates(goal, &mut candidates);
}
self.assemble_object_bound_candidates(goal, &mut candidates);
} else {
self.assemble_builtin_impl_candidates(goal, &mut candidates);
// For performance we only assemble impls if there are no candidates
// which would shadow them. This is necessary to avoid hangs in rayon,
// see trait-system-refactor-initiative#109 for more details.
//
// We always assemble builtin impls as trivial builtin impls have a higher
// priority than where-clauses.
//
// We only do this if any such candidate applies without any constraints
// as we may want to weaken inference guidance in the future and don't want
// to worry about causing major performance regressions when doing so.
// See trait-system-refactor-initiative#226 for some ideas here.
let assemble_impls = match self.typing_mode() {
TypingMode::Coherence => true,
TypingMode::Analysis { .. }
| TypingMode::Borrowck { .. }
| TypingMode::PostBorrowckAnalysis { .. }
| TypingMode::PostAnalysis => !candidates.iter().any(|c| {
matches!(
c.source,
CandidateSource::ParamEnv(ParamEnvSource::NonGlobal)
| CandidateSource::AliasBound(_)
) && has_no_inference_or_external_constraints(c.result)
}),
};
if assemble_impls {
self.assemble_impl_candidates(goal, &mut candidates);
self.assemble_object_bound_candidates(goal, &mut candidates);
}
}
}
AssembleCandidatesFrom::EnvAndBounds => {
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 @@ -798,6 +798,7 @@ symbols! {
derive_from,
derive_smart_pointer,
destruct,
destruct_drop_in_place,
destructuring_assignment,
diagnostic,
diagnostic_namespace,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
self.assemble_candidates_for_unsizing(obligation, &mut candidates);
}
Some(LangItem::Destruct) => {
self.assemble_const_destruct_candidates(obligation, &mut candidates);
let before = candidates.vec.len();
self.assemble_candidates_from_impls(obligation, &mut candidates);
let added_impl =
candidates.vec[before..].iter().any(|c| matches!(c, ImplCandidate(_)));
if !added_impl {
self.assemble_const_destruct_candidates(obligation, &mut candidates);
}
}
Some(LangItem::TransmuteTrait) => {
// User-defined transmutability impls are permitted.
Expand Down
82 changes: 55 additions & 27 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,33 +38,7 @@ fn resolve_instance_raw<'tcx>(
} else if tcx.is_lang_item(def_id, LangItem::DropInPlace) {
let ty = args.type_at(0);

if ty.needs_drop(tcx, typing_env) {
debug!(" => nontrivial drop glue");
match *ty.kind() {
ty::Coroutine(coroutine_def_id, ..) => {
// FIXME: sync drop of coroutine with async drop (generate both versions?)
// Currently just ignored
if tcx.optimized_mir(coroutine_def_id).coroutine_drop_async().is_some() {
ty::InstanceKind::DropGlue(def_id, None)
} else {
ty::InstanceKind::DropGlue(def_id, Some(ty))
}
}
ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Tuple(..)
| ty::Adt(..)
| ty::Dynamic(..)
| ty::Array(..)
| ty::Slice(..)
| ty::UnsafeBinder(..) => ty::InstanceKind::DropGlue(def_id, Some(ty)),
// Drop shims can only be built from ADTs.
_ => return Ok(None),
}
} else {
debug!(" => trivial drop glue");
ty::InstanceKind::DropGlue(def_id, None)
}
Comment on lines -41 to -67
Copy link
Copy Markdown
Member

@Nadrieril Nadrieril May 6, 2026

Choose a reason for hiding this comment

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

For perf reasons I expect we should keep some of this shortcutting. Also whatever's happening with async drop feels important, why is it ok to remove it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll have a look soon, thanks for pointing this out

Copy link
Copy Markdown
Member Author

@JayanAXHF JayanAXHF May 6, 2026

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure, but why is that ok and why is that useful? Doesn't this return the wrong instance kind in some cases now?

return ty::Instance::try_resolve_drop_in_place(tcx, typing_env, ty);
} else if tcx.is_lang_item(def_id, LangItem::AsyncDropInPlace) {
let ty = args.type_at(0);

Expand Down Expand Up @@ -166,6 +140,20 @@ fn resolve_associated_item<'tcx>(
if !eligible {
return Ok(None);
}
if tcx.is_lang_item(trait_ref.def_id, LangItem::Destruct) {
if !tcx.is_lang_item(trait_item_id, LangItem::DestructDropInPlace) {
bug!(
"unexpected associated item for built-in `{trait_ref}`: {}",
tcx.item_name(trait_item_id)
);
}

debug!("Got user Destruct impl");
return Ok(Some(Instance {
def: ty::InstanceKind::Item(leaf_def.item.def_id),
args: rcvr_args,
}));
}
Comment on lines +143 to +156
Copy link
Copy Markdown
Member

@Nadrieril Nadrieril May 6, 2026

Choose a reason for hiding this comment

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

Why is this early return useful/necessary? Please add a comment explaning its purpose

View changes since the review


let typing_env = typing_env.with_post_analysis_normalized(tcx);
let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env);
Expand Down Expand Up @@ -403,6 +391,46 @@ fn resolve_associated_item<'tcx>(
} else {
bug!("unexpected associated associated item")
}
} else if tcx.is_lang_item(trait_ref.def_id, LangItem::Destruct) {
debug!(
"resolving Destruct for ImplSource::Builtin: {:?}, {:?}, {:?}",
typing_env, trait_item_id, rcvr_args
);
if !tcx.is_lang_item(trait_item_id, LangItem::DestructDropInPlace) {
bug!(
"unexpected associated item for built-in `{trait_ref}`: {}",
tcx.item_name(trait_item_id)
);
}

let self_ty = trait_ref.self_ty();

let def = if self_ty.needs_drop(tcx, typing_env) {
match *self_ty.kind() {
ty::Coroutine(coroutine_def_id, ..) => {
if tcx.optimized_mir(coroutine_def_id).coroutine_drop_async().is_some()
{
ty::InstanceKind::DropGlue(trait_item_id, None)
} else {
ty::InstanceKind::DropGlue(trait_item_id, Some(self_ty))
}
}
ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Tuple(..)
| ty::Adt(..)
| ty::Dynamic(..)
| ty::Array(..)
| ty::Slice(..)
| ty::UnsafeBinder(..) => {
ty::InstanceKind::DropGlue(trait_item_id, Some(self_ty))
}
_ => return Ok(None),
}
} else {
ty::InstanceKind::DropGlue(trait_item_id, None)
};
Some(ty::Instance { def, args: rcvr_args })
} else {
Instance::try_resolve_item_for_coroutine(tcx, trait_item_id, trait_id, rcvr_args)
}
Expand Down
11 changes: 8 additions & 3 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,10 +1056,15 @@ marker_impls! {
#[unstable(feature = "const_destruct", issue = "133214")]
#[rustc_const_unstable(feature = "const_destruct", issue = "133214")]
#[lang = "destruct"]
#[diagnostic::on_unimplemented(message = "can't drop `{Self}`")]
#[rustc_deny_explicit_impl]
#[rustc_on_unimplemented(message = "can't drop `{Self}`")]
#[rustc_dyn_incompatible_trait]
pub const trait Destruct: PointeeSized {}
pub const trait Destruct: PointeeSized {
/// Entrypoint for drop
///
/// Generated by default if not implemented manually.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(_to_drop: *mut Self);
}

/// A marker for tuple types.
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2404,7 +2404,13 @@ impl const MyClone for i32 {
}
}
#[lang = "destruct"]
pub trait Destruct {}
pub trait Destruct {
/// Entrypoint for drop
///
/// Generated by default if not implemented manually.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(_to_drop: *mut Self);
}
"#,
);
}
Expand Down
8 changes: 7 additions & 1 deletion tests/auxiliary/minicore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,13 @@ pub trait Sized: MetaSized {}

#[lang = "destruct"]
#[diagnostic::on_unimplemented(message = "can't drop `{Self}`")]
pub trait Destruct: PointeeSized {}
pub trait Destruct: PointeeSized {
/// Entrypoint for drop
///
/// Generated by default if not implemented manually.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(_to_drop: *mut Self);
}

#[lang = "legacy_receiver"]
pub trait LegacyReceiver {}
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/drop/custom_dtor_with_destruct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//@ run-pass
//@ check-stdout
//@ check-run-results

#![feature(const_destruct)]
use std::marker::Destruct;
struct A {
_a: String,
}

impl Destruct for A {
unsafe fn drop_in_place(_to_drop: *mut Self) {
println!("Hey i was dropped");
}
}

fn main() {
let _a = A { _a: String::new() };
}
1 change: 1 addition & 0 deletions tests/ui/drop/custom_dtor_with_destruct.run.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Hey i was dropped
10 changes: 8 additions & 2 deletions tests/ui/traits/const-traits/auxiliary/minicore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
fundamental,
marker_trait_attr,
const_trait_impl,
const_destruct,
const_destruct
)]
#![allow(internal_features, incomplete_features)]
#![no_std]
Expand Down Expand Up @@ -122,7 +122,13 @@ impl<T: Deref + MetaSized> Receiver for T {
}

#[lang = "destruct"]
pub const trait Destruct {}
pub const trait Destruct {
/// Entrypoint for drop
///
/// Generated by default if not implemented manually.
#[lang = "destruct_drop_in_place"]
unsafe fn drop_in_place(_to_drop: *mut Self);
}

#[lang = "freeze"]
pub unsafe auto trait Freeze {}
Expand Down
Loading