Skip to content

Allow MIR-inlining Drop terminators too #144561

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

scottmcm
Copy link
Member

We've already been able to do this if drop(_1) was spelled _2 = &raw mut _1; drop_in_place(move _2), so this just needs to do the necessary updates to the query-cycle-avoidance check and to look for Drops in the normal (non-force) inliner.

This tries to avoid turning drops into drop_in_place calls if they're likely not going to be inlined, since the Call needs an extra local and an extra statement.

r? cjgillot

@rustbot 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 28, 2025
let source_info = terminator.source_info;
let drop_ty = place.ty(&caller_body.local_decls, tcx).ty;
if !drop_ty.needs_drop(tcx, inliner.typing_env()) {
//return None;
Copy link
Member Author

Choose a reason for hiding this comment

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

note to self: fix

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Jul 28, 2025

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

@cjgillot
Copy link
Contributor

You'll probably need to change the mir shim pass. The current implementation only reliably supports monomorphic types.

Zalathar added a commit to Zalathar/rust that referenced this pull request Jul 29, 2025
Simplify `align_of_val::<[T]>(…)` → `align_of::<T>()`

I spotted this while working on the inliner (rust-lang#144561).  In particular, if [`Layout::for_value`](https://doc.rust-lang.org/std/alloc/struct.Layout.html#method.for_value) inlines, then it can be pretty easy to end up with an `align_of_val::<[T]>` today (demo: <https://rust.godbolt.org/z/Tesnscj4a>) where we can save at least a block, if not more, by using the version that's an rvalue and not a call.
rust-timer added a commit that referenced this pull request Jul 29, 2025
Rollup merge of #144566 - scottmcm:align-of-slice, r=oli-obk

Simplify `align_of_val::<[T]>(…)` → `align_of::<T>()`

I spotted this while working on the inliner (#144561).  In particular, if [`Layout::for_value`](https://doc.rust-lang.org/std/alloc/struct.Layout.html#method.for_value) inlines, then it can be pretty easy to end up with an `align_of_val::<[T]>` today (demo: <https://rust.godbolt.org/z/Tesnscj4a>) where we can save at least a block, if not more, by using the version that's an rvalue and not a call.
@scottmcm
Copy link
Member Author

scottmcm commented Aug 1, 2025

Hmm, looks like you're right. Just trying to guard requesting the mir doesn't work since it's a concrete outer type, just with field issues. Tricky; I'll have to spend some time grokking the shim pass.

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
---- [ui] tests/ui/mir/issue-107691.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 101
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/mir/issue-107691.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" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/mir/issue-107691" "-A" "unused" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers" "-C" "opt-level=3"
stdout: none
--- stderr -------------------------------
##[error]error: internal compiler error: compiler/rustc_mir_transform/src/validate.rs:80:25: broken MIR in DropGlue(DefId(2:2933 ~ core[1fb6]::ptr::drop_in_place), Some(RecordResolver<'{erased}>)) (after phase change to runtime-optimized) at bb0[0]:
                                Field projection `(*_1).0` specified type `{type error}`, but actual type is `<&[u8] as Archive>::Resolver`
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/mod.rs:804:1


thread 'rustc' panicked at compiler/rustc_mir_transform/src/validate.rs:80:25:
Box<dyn Any>
stack backtrace:
   0: std::panicking::begin_panic::<rustc_errors::ExplicitBug>
   1: <rustc_errors::diagnostic::BugAbort as rustc_errors::diagnostic::EmissionGuarantee>::emit_producing_guarantee
   2: <rustc_errors::diagnostic::Diag<rustc_errors::diagnostic::BugAbort>>::emit
   3: <rustc_errors::DiagCtxtHandle>::span_bug::<rustc_span::span_encoding::Span, alloc::string::String>
   4: rustc_middle::util::bug::opt_span_bug_fmt::<rustc_span::span_encoding::Span>::{closure#0}
   5: rustc_middle::ty::context::tls::with_opt::<rustc_middle::util::bug::opt_span_bug_fmt<rustc_span::span_encoding::Span>::{closure#0}, !>::{closure#0}
   6: rustc_middle::ty::context::tls::with_context_opt::<rustc_middle::ty::context::tls::with_opt<rustc_middle::util::bug::opt_span_bug_fmt<rustc_span::span_encoding::Span>::{closure#0}, !>::{closure#0}, !>
   7: rustc_middle::util::bug::span_bug_fmt::<rustc_span::span_encoding::Span>
   8: <rustc_mir_transform::validate::CfgChecker>::fail::<alloc::string::String>
   9: <rustc_mir_transform::validate::Validator as rustc_mir_transform::pass_manager::MirPass>::run_pass
  10: rustc_mir_transform::pass_manager::run_passes_inner
  11: rustc_mir_transform::shim::make_shim
      [... omitted 3 frames ...]
  12: <rustc_middle::ty::context::TyCtxt>::instance_mir
  13: rustc_mir_transform::inline::try_instance_mir
  14: rustc_mir_transform::inline::process_blocks::<rustc_mir_transform::inline::NormalInliner>
  15: <rustc_mir_transform::inline::Inline as rustc_mir_transform::pass_manager::MirPass>::run_pass
  16: rustc_mir_transform::pass_manager::run_passes_inner
  17: rustc_mir_transform::run_optimization_passes
  18: rustc_mir_transform::optimized_mir
      [... omitted 3 frames ...]
  19: rustc_mir_transform::cross_crate_inline::cross_crate_inlinable
      [... omitted 3 frames ...]
  20: rustc_passes::reachable::recursively_reachable
  21: rustc_passes::reachable::reachable_set
      [... omitted 3 frames ...]
  22: <rustc_metadata::rmeta::encoder::EncodeContext>::encode_crate_root
  23: <rustc_metadata::rmeta::encoder::encode_metadata::{closure#3} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, &std::path::Path)>>::call_once
  24: rustc_metadata::rmeta::encoder::encode_metadata
  25: rustc_metadata::fs::encode_and_write_metadata
  26: rustc_interface::passes::start_codegen
  27: <rustc_interface::queries::Linker>::codegen_and_build_linker
  28: <std::thread::local::LocalKey<core::cell::Cell<*const ()>>>::with::<rustc_middle::ty::context::tls::enter_context<<rustc_middle::ty::context::GlobalCtxt>::enter<rustc_interface::passes::create_and_enter_global_ctxt<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>::{closure#2}::{closure#0}, core::option::Option<rustc_interface::queries::Linker>>::{closure#1}, core::option::Option<rustc_interface::queries::Linker>>::{closure#0}, core::option::Option<rustc_interface::queries::Linker>>
  29: <rustc_middle::ty::context::TyCtxt>::create_global_ctxt::<core::option::Option<rustc_interface::queries::Linker>, rustc_interface::passes::create_and_enter_global_ctxt<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>::{closure#2}::{closure#0}>
  30: <rustc_interface::passes::create_and_enter_global_ctxt<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>::{closure#2} as core::ops::function::FnOnce<(&rustc_session::session::Session, rustc_middle::ty::context::CurrentGcx, alloc::sync::Arc<rustc_data_structures::jobserver::Proxy>, &std::sync::once_lock::OnceLock<rustc_middle::ty::context::GlobalCtxt>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_middle::arena::Arena>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_hir::Arena>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2})>>::call_once::{shim:vtable#0}
  31: <alloc::boxed::Box<dyn for<'a> core::ops::function::FnOnce<(&'a rustc_session::session::Session, rustc_middle::ty::context::CurrentGcx, alloc::sync::Arc<rustc_data_structures::jobserver::Proxy>, &'a std::sync::once_lock::OnceLock<rustc_middle::ty::context::GlobalCtxt<'a>>, &'a rustc_data_structures::sync::worker_local::WorkerLocal<rustc_middle::arena::Arena<'a>>, &'a rustc_data_structures::sync::worker_local::WorkerLocal<rustc_hir::Arena<'a>>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}), Output = core::option::Option<rustc_interface::queries::Linker>>> as core::ops::function::FnOnce<(&rustc_session::session::Session, rustc_middle::ty::context::CurrentGcx, alloc::sync::Arc<rustc_data_structures::jobserver::Proxy>, &std::sync::once_lock::OnceLock<rustc_middle::ty::context::GlobalCtxt>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_middle::arena::Arena>, &rustc_data_structures::sync::worker_local::WorkerLocal<rustc_hir::Arena>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2})>>::call_once
  32: rustc_interface::passes::create_and_enter_global_ctxt::<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>
  33: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::util::run_in_thread_with_globals<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<(), rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#0}, ()>::{closure#0}::{closure#0}::{closure#0}, ()>
  34: rustc_span::create_session_globals_then::<(), rustc_interface::util::run_in_thread_with_globals<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<(), rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#0}, ()>::{closure#0}::{closure#0}::{closure#0}>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: please make sure that you have updated to the latest nightly

note: rustc 1.90.0-nightly (ed304d325 2025-08-01) running on aarch64-unknown-linux-gnu

note: compiler flags: -Z threads=1 -Z simulate-remapped-rust-src-base=/rustc/FAKE_PREFIX -Z translate-remapped-path-to-local-path=no -Z ignore-directory-in-diagnostics-source-blocks=/cargo -Z ignore-directory-in-diagnostics-source-blocks=/checkout/vendor -C codegen-units=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z write-long-types-to-disk=no -C strip=debuginfo -C prefer-dynamic -C rpath -C debuginfo=0 -C opt-level=3

query stack during panic:
#0 [mir_shims] generating MIR shim for `core::ptr::drop_in_place`, instance=DropGlue(DefId(2:2933 ~ core[1fb6]::ptr::drop_in_place), Some(RecordResolver<'{erased}>))
#1 [optimized_mir] optimizing MIR for `<impl at /checkout/tests/ui/mir/issue-107691.rs:34:1: 36:23>::resolve`
#2 [cross_crate_inlinable] whether the item should be made inlinable across crates
#3 [reachable_set] reachability
end of query stack
error: aborting due to 1 previous error
------------------------------------------


GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 1, 2025
Fortify RemoveUnneededDrops test.

Test tweak that is useful in preparation for rust-lang#144561
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 2, 2025
Fortify RemoveUnneededDrops test.

Test tweak that is useful in preparation for rust-lang#144561
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 2, 2025
Fortify RemoveUnneededDrops test.

Test tweak that is useful in preparation for rust-lang#144561
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants