Fix improper disabling of async drop on usage by nanai10a · Pull Request #163953 · rust-lang/rust

nanai10a · 2026-10-07T16:38:32Z

Fix a condition on branch to generate async-drop shim. In summary, if async_drop feature is enabled, rustc emits a async-drop shim for many of types. But if async_drop feature is disabled, rustc doesn't emit a async-drop shim, and nobody do "async drop" (excludes dependent crates).
But currently rustc emits async-drop shim for each compilation, and if dependent crate has nested async-drop shim, that shim is regenerate as no shim inside. That causes unsoundness bug for memory access; and rustc doesn't describe this situation.

If dependent crate does "async drop" and disabled async_drop feature on usage, rustc warns that situation (introduced by #141253 or 6105928). But rustc doesn't care if dependent crate has only shims that does "async drop", that implicitly declared. This fix doesn't include about that.

This behavior was reported in #163506 unconcisely, but this introduces some tests about this behavior, so that refuses regression.

This behavior introduced on #123948 (initial implementation) and fix landed on #153274 but missed a situation and this fix solves it. async_drop feature itself tracked on #126482.

And, currently this PR is draft, because some tests are working improperly, caused by compiletest behavior. I know ways of reproducing this issue but I'm not familar with rustc and its testing system, so want to take advice about testing; for example, these tests about MIR might not needed in this PR. I changed status to ready for review, because after reverting false positive tests, CIs are passed all. ... if needed these low-level tests, I'll add them again.

rustbot · 2026-10-09T16:48:22Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

rustbot · 2026-10-09T16:48:26Z

Thanks for the pull request, and welcome! The Rust Project has assigned @oli-obk (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 77 candidates
Random selection from 18 candidates

oli-obk · 2026-10-23T11:44:02Z

+// FIXME: This test is a variant of asynd-drop-shim-usage on MIR,
+// but compiletest seems it cannot produce EMIR_MIR in revisions.
+// This test corresponds to `with_feature` revision,
+// and other `without_feature` revision could found at
+// `tests/mir-opt/coroutine/async_drop_shim_usage_without_feature.rs`.


instead of using EMIT_MIR, could you use the CHECK flags to instead check that the specific things you're looking for are there?

if that works you can also get rid of the ui test, just having the MIR opt test ensure things are the way you expect should suffice

View changes since the review

i'd tried to use CHECK flags (on 52d7c0d and it's replaced later) but --emit=mir produced doesn't contain expected parts, so i'd used a bit hacky way. and also, this regression is not focused to user-provided programs; only for shim parts, that generated by rustc, in every compilation. so usual unit test is not confortable.

rustbot · 2026-10-23T11:44:11Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

…coroutine" This reverts commit ebd392f. I've tried to add tests for MIR-level but couldn't doing well so draw back these tests.

`ty::ShimKind::AsyncDropGlue(..)` is constructed on: - `rustc_codegen_ssa::back::symbol_export::exported_generic_symbols_provider_local` ... it's for codegen - `rustc_middle::ty::TyCtxt::async_drop_coroutine_layout` ... returns only layout of coroutine - `rustc_mir_transform::shim::async_destructor_ctor::build_async_drop_shim` ... reconstructs from args which're destructed from itself - `rustc_ty_utils::instance::resolve_instance_raw` ... reconstructs if `TyCtxt` is in `async_drop_in_place` ... so it's fine to imply, that type is contained on async-drop shim.

… coroutine" This reverts commit 76bd55d.

… of EMIT_MIR (incomplete) consider: `filecheck-flags` usage seems hacky and/or improper. note: `no-prefer-dynamic` is needed because it causes failing compilation of test. note: on `has_drop_inside_dep.rs`, setted `dump-mir=none`, for clearly observe dumps which came from compilation, for usages side, not aux-crate itself. consider: `without_feature` revision of `async_drop_shim_usage.rs` emits no `core::future::async_drop_in_place` instances. this behavior is expected, but in some reasons it is not good: (1.) i want to focus to regrect miscalculation of layout. but if check only `async_drop_in_place` item, implies no check for layout of `AsyncDropGlue`; it's indirectional test. (2.) fundamentally, that behavior causes failing execution of filecheck, because MIR dump is not fully generated so indicates ENOENT. it's also indirectional test too.

consider: types order on text alignment is not important. but it might imply, these types are processed in definitely same order or procedures

rustbot · 2026-10-24T17:41:02Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

nanai10a · 2026-10-25T04:38:34Z

thanks to @oli-obk for reviewing. since that review, changed:

replace mir-opt test to ui test (with considerations; see commit details). regression for this behavior basically is check layout of Coroutine, which's desugar of async fn. print-size-types emits layout of various types, includes async_drop_in_place<...>, is not user-defined.
introduced condition became more strict. ty::ShimKind::AsyncDropGlue(...) is constracted on not so many places and limited, and expected.

@rustbot ready

oli-obk · 2026-10-26T11:45:30Z

I am extremely confused by some of the commit messages, the changes, and in general about the change, so: what parts of this PR, finding the problem, writing tests, commits, and sent messages are being generated by LLMs?

nanai10a · 2026-10-26T13:23:05Z

I apologize to have confused you. In this PR, only "finding the problem" part is helped by LLM. The initial report of this problem (#163506) contains generated contents, but current issue (#163506 too) had been reorganized later by myself. Therefore this PR is based on the report generated by LLM, it's on a "finding the problem" part, but not used for other parts: writing patch, test and commit messages.

I wrote these commit messages to note decisions for me, of course, and reviewers if couldn't understand why made this changes. But if these messages make confused you (and others), ignore them. Or if these messages are bad and/or inarticulate English sentences, ignore them too.

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 7, 2026