[POC] create an MVP for using `Destruct` for custom dtors by JayanAXHF · Pull Request #156090 · rust-lang/rust

JayanAXHF · 2026-05-02T14:49:47Z

We redirect drop glue to Destruct::drop_in_place instead of mem::ptr::drop_in_place.
Theres a new lang item for DestructDropInPlace
The old solver checks for user candidates for impl in candidate_assembly, and only checks for builtin impl when it doesnt find one
The new trait solver does something similar, and checks for user impls before calling assemble_builtin_impl_candidates

cc: @Nadrieril and @oli-obk (you were interested in reviewing this from the PG)

Note that this is a POC and i've not ironed out the comments and im not 100% confident that this is the best way to do this.

r? ghost

JayanAXHF · 2026-05-03T07:42:40Z

welp i have no idea what this is. Is this due to smth with cg_clift? do i need to make changes there

JayanAXHF · 2026-05-03T08:08:58Z

fixed it i think, it was related to the minicores not being updated

JayanAXHF · 2026-05-06T13:51:01Z

It was another mini_core smh

Nadrieril · 2026-05-06T14:18:12Z

+    ///
+    /// Generated by default if not implemented manually.
+    #[lang = "destruct_drop_in_place"]
+    unsafe fn drop_in_place(_to_drop: *mut Self);


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

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

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.

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)

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.

Nadrieril · 2026-05-06T14:22:01Z

-            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)
-            }


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?

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

https://github.com/JayanAXHF/rust/blob/08f7d565937ac22308cb180cdefb9d59f88d95e8/compiler/rustc_ty_utils/src/instance.rs#L408-L433

IIRC that part is moved to here

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

fix: bless tests

Nadrieril

(github decided to not send my coments and make them review comments so I have to make a top-level review)

View changes since this review

Nadrieril · 2026-05-06T16:10:35Z

-            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)
-            }


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

rust-log-analyzer · 2026-05-06T16:33:18Z

The job aarch64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
PR_CI_JOB set; skipping tidy
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
downloading https://static.rust-lang.org/dist/2026-04-14/rustfmt-nightly-aarch64-unknown-linux-gnu.tar.xz
---
failures:

---- [ui] tests/ui/consts/miri_unleashed/drop.rs stdout ----

error: 1 diagnostics reported in JSON output but not expected in test file
tests/ui/consts/miri_unleashed/drop.rs:16:1: NOTE: inside `<Vec<i32> as Destruct>::drop_in_place - shim(Some(Vec<i32>))`
  expected with different message: inside `drop_in_place::<Vec<i32>> - shim(Some(Vec<i32>))`

error: 1 diagnostics expected in test file but not reported in JSON output
tests/ui/consts/miri_unleashed/drop.rs:16:4: NOTE: inside `drop_in_place::<Vec<i32>> - shim(Some(Vec<i32>))`
  reported with different message: inside `<Vec<i32> as Destruct>::drop_in_place - shim(Some(Vec<i32>))`

thread '[ui] tests/ui/consts/miri_unleashed/drop.rs' panicked at src/tools/compiletest/src/runtest.rs:902:13:
errors differ from expected
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/consts/miri_unleashed/drop.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/consts/miri_unleashed/drop" "-A" "unused" "-W" "unused_attributes" "-A" "internal_features" "-A" "incomplete_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "-Zunleash-the-miri-inside-of-you"

stack backtrace:
   8: __rustc::rust_begin_unwind
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/panicking.rs:689:5
   9: core::panicking::panic_fmt
---
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
---- [ui] tests/ui/consts/miri_unleashed/drop.rs stdout end ----
---- [ui] tests/ui/sanitizer/cfi/drop-in-place.rs stdout ----

error: test did not exit with success! code=None so test would pass with `run-crash`
status: signal: 5 (SIGTRAP) (core dumped)
command: cd "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/sanitizer/cfi/drop-in-place" && RUSTC="/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" RUST_TEST_THREADS="4" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/sanitizer/cfi/drop-in-place/a"
stdout: none
stderr: none

---- [ui] tests/ui/sanitizer/cfi/drop-in-place.rs stdout end ----
---- [ui] tests/ui/sanitizer/cfi/coroutine.rs#cfi stdout ----

error in revision `cfi`: test did not exit with success! code=None so test would pass with `run-crash`
status: signal: 5 (SIGTRAP) (core dumped)
command: cd "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/sanitizer/cfi/coroutine.cfi" && RUSTC="/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" RUST_TEST_THREADS="4" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/sanitizer/cfi/coroutine.cfi/a"
--- stdout -------------------------------

running 4 tests
------------------------------------------
stderr: none

rust-bors · 2026-05-06T17:25:39Z

☔ The latest upstream changes (presumably #155443) made this pull request unmergeable. Please resolve the merge conflicts.

Nadrieril · 2026-05-06T19:09:31Z

+            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,
+                }));
+            }


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

View changes since the review