From 6e23b38348507aaa75bb0b1d4f79e6a178e40d84 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Jun 2025 10:22:49 -0400 Subject: [PATCH 1/2] Skip collecting no-op DropGlue Since 122662 this no longer gets used in vtables, so we're safe to fully drop generating these empty functions. Those are eventually cleaned up by LLVM, but it's wasteful to produce them in the first place. --- compiler/rustc_monomorphize/src/collector.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 173030e0326e8..6e9130549e3f9 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -948,10 +948,14 @@ fn visit_instance_use<'tcx>( bug!("{:?} being reified", instance); } ty::InstanceKind::DropGlue(_, None) => { - // Don't need to emit noop drop glue if we are calling directly. - if !is_direct_call { - output.push(create_fn_mono_item(tcx, instance, source)); - } + // No-op drop glue never needs to be emitted. + // In direct calls, we skip it in codegen. In indirect calls (vtables) we place a null + // pointer rather than codegen'ing a pointer to the empty drop. + // + // Note that if user code casts drop_in_place to a fn(...) that's not a DropGlue, so + // it won't hit this branch. + // + // If we get this wrong we'll see linker errors. } ty::InstanceKind::DropGlue(_, Some(_)) | ty::InstanceKind::FutureDropPollShim(..) From 9549f6b5863e101fd63631181340f8384180ad38 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Jun 2025 10:36:53 -0400 Subject: [PATCH 2/2] Fix tests to drop now-skipped codegen --- .../instantiation-through-vtable.rs | 2 -- .../item-collection/non-generic-closures.rs | 29 ++++++++++++++++--- .../codegen-units/item-collection/unsizing.rs | 22 +++++++++++--- ...tadata-id-itanium-cxx-abi-drop-in-place.rs | 14 +++++---- 4 files changed, 52 insertions(+), 15 deletions(-) diff --git a/tests/codegen-units/item-collection/instantiation-through-vtable.rs b/tests/codegen-units/item-collection/instantiation-through-vtable.rs index 8f13fd558083c..7882a526b6829 100644 --- a/tests/codegen-units/item-collection/instantiation-through-vtable.rs +++ b/tests/codegen-units/item-collection/instantiation-through-vtable.rs @@ -24,7 +24,6 @@ impl Trait for Struct { pub fn start(_: isize, _: *const *const u8) -> isize { let s1 = Struct { _a: 0u32 }; - //~ MONO_ITEM fn std::ptr::drop_in_place::> - shim(None) @@ instantiation_through_vtable-cgu.0[External] //~ MONO_ITEM fn as Trait>::foo //~ MONO_ITEM fn as Trait>::bar let r1 = &s1 as &Trait; @@ -32,7 +31,6 @@ pub fn start(_: isize, _: *const *const u8) -> isize { r1.bar(); let s1 = Struct { _a: 0u64 }; - //~ MONO_ITEM fn std::ptr::drop_in_place::> - shim(None) @@ instantiation_through_vtable-cgu.0[External] //~ MONO_ITEM fn as Trait>::foo //~ MONO_ITEM fn as Trait>::bar let _ = &s1 as &Trait; diff --git a/tests/codegen-units/item-collection/non-generic-closures.rs b/tests/codegen-units/item-collection/non-generic-closures.rs index 124fe7e3b69a0..740b963c07c92 100644 --- a/tests/codegen-units/item-collection/non-generic-closures.rs +++ b/tests/codegen-units/item-collection/non-generic-closures.rs @@ -1,4 +1,4 @@ -//@ compile-flags:-Clink-dead-code -Zinline-mir=no +//@ compile-flags:-Clink-dead-code -Zinline-mir=no -Zhuman-readable-cgu-names #![deny(dead_code)] #![crate_type = "lib"] @@ -22,9 +22,8 @@ fn assigned_to_variable_but_not_executed() { //~ MONO_ITEM fn assigned_to_variable_executed_indirectly @@ non_generic_closures-cgu.0[External] fn assigned_to_variable_executed_indirectly() { //~ MONO_ITEM fn assigned_to_variable_executed_indirectly::{closure#0} @@ non_generic_closures-cgu.0[External] - //~ MONO_ITEM fn <{closure@TEST_PATH:28:13: 28:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[External] - //~ MONO_ITEM fn <{closure@TEST_PATH:28:13: 28:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[External] - //~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:28:13: 28:21}> - shim(None) @@ non_generic_closures-cgu.0[External] + //~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[External] + //~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[External] let f = |a: i32| { let _ = a + 2; }; @@ -40,6 +39,20 @@ fn assigned_to_variable_executed_directly() { f(4); } +// Make sure we generate mono items for stateful closures that need dropping +//~ MONO_ITEM fn with_drop @@ non_generic_closures-cgu.0[External] +fn with_drop(v: PresentDrop) { + //~ MONO_ITEM fn with_drop::{closure#0} @@ non_generic_closures-cgu.0[External] + //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(Some(PresentDrop)) @@ non_generic_closures-cgu.0[Internal] + //~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:49:14: 49:24}> - shim(Some({closure@TEST_PATH:49:14: 49:24})) @@ non_generic_closures-cgu.0[Internal] + + let _f = |a: usize| { + let _ = a + 2; + //~ MONO_ITEM fn std::mem::drop:: @@ non_generic_closures-cgu.0[External] + drop(v); + }; +} + //~ MONO_ITEM fn start @@ non_generic_closures-cgu.0[External] #[no_mangle] pub fn start(_: isize, _: *const *const u8) -> isize { @@ -47,6 +60,7 @@ pub fn start(_: isize, _: *const *const u8) -> isize { assigned_to_variable_but_not_executed(); assigned_to_variable_executed_directly(); assigned_to_variable_executed_indirectly(); + with_drop(PresentDrop); 0 } @@ -55,3 +69,10 @@ pub fn start(_: isize, _: *const *const u8) -> isize { fn run_closure(f: &Fn(i32)) { f(3); } + +struct PresentDrop; + +impl Drop for PresentDrop { + //~ MONO_ITEM fn ::drop @@ non_generic_closures-cgu.0[External] + fn drop(&mut self) {} +} diff --git a/tests/codegen-units/item-collection/unsizing.rs b/tests/codegen-units/item-collection/unsizing.rs index 15e42bce2495d..d008d9d6898af 100644 --- a/tests/codegen-units/item-collection/unsizing.rs +++ b/tests/codegen-units/item-collection/unsizing.rs @@ -42,33 +42,47 @@ struct Wrapper(#[allow(dead_code)] *const T); impl, U: ?Sized> CoerceUnsized> for Wrapper {} +struct PresentDrop; + +impl Drop for PresentDrop { + fn drop(&mut self) {} +} + +// Custom Coercion Case +impl Trait for PresentDrop { + fn foo(&self) {} +} + //~ MONO_ITEM fn start #[no_mangle] pub fn start(_: isize, _: *const *const u8) -> isize { // simple case let bool_sized = &true; - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _bool_unsized = bool_sized as &Trait; let char_sized = &'a'; - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _char_unsized = char_sized as &Trait; // struct field let struct_sized = &Struct { _a: 1, _b: 2, _c: 3.0f64 }; - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _struct_unsized = struct_sized as &Struct; // custom coercion let wrapper_sized = Wrapper(&0u32); - //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(None) @@ unsizing-cgu.0[Internal] //~ MONO_ITEM fn ::foo let _wrapper_sized = wrapper_sized as Wrapper; + // with drop + let droppable = &PresentDrop; + //~ MONO_ITEM fn ::drop @@ unsizing-cgu.0[Internal] + //~ MONO_ITEM fn std::ptr::drop_in_place:: - shim(Some(PresentDrop)) @@ unsizing-cgu.0[Internal] + //~ MONO_ITEM fn ::foo + let droppable = droppable as &dyn Trait; + false.foo(); 0 diff --git a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs index 2a7eca6fc1963..8fec275fd0646 100644 --- a/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs +++ b/tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs @@ -1,5 +1,9 @@ // Verifies that type metadata identifiers for drop functions are emitted correctly. // +// Non needs_drop drop glue isn't codegen'd at all, so we don't try to check the IDs there. But we +// do check it's not emitted which should help catch bugs if we do start generating it again in the +// future. +// //@ needs-sanitizer-cfi //@ compile-flags: -Clto -Cno-prepopulate-passes -Copt-level=0 -Zsanitizer=cfi -Ctarget-feature=-crt-static @@ -10,18 +14,18 @@ // CHECK: call i1 @llvm.type.test(ptr {{%.+}}, metadata !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE") struct EmptyDrop; -// CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}EmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} +// CHECK-NOT: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}EmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} -struct NonEmptyDrop; +struct PresentDrop; -impl Drop for NonEmptyDrop { +impl Drop for PresentDrop { fn drop(&mut self) {} - // CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}NonEmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} + // CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}PresentDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}} } pub fn foo() { let _ = Box::new(EmptyDrop) as Box; - let _ = Box::new(NonEmptyDrop) as Box; + let _ = Box::new(PresentDrop) as Box; } // CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE"}