Skip to content

Skip no-op drop glue #142508

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+60 −19
Open

Skip no-op drop glue #142508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e23b38
Skip collecting no-op DropGlue
Mark-Simulacrum Jun 14, 2025
9549f6b
Fix tests to drop now-skipped codegen
Mark-Simulacrum Jun 14, 2025
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
12 changes: 8 additions & 4 deletions compiler/rustc_monomorphize/src/collector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(..)
Expand Down
2 changes: 0 additions & 2 deletions tests/codegen-units/item-collection/instantiation-through-vtable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,13 @@ impl<T> Trait for Struct<T> {
pub fn start(_: isize, _: *const *const u8) -> isize {
let s1 = Struct { _a: 0u32 };

//~ MONO_ITEM fn std::ptr::drop_in_place::<Struct<u32>> - shim(None) @@ instantiation_through_vtable-cgu.0[External]
//~ MONO_ITEM fn <Struct<u32> as Trait>::foo
//~ MONO_ITEM fn <Struct<u32> as Trait>::bar
let r1 = &s1 as &Trait;
r1.foo();
r1.bar();

let s1 = Struct { _a: 0u64 };
//~ MONO_ITEM fn std::ptr::drop_in_place::<Struct<u64>> - shim(None) @@ instantiation_through_vtable-cgu.0[External]
//~ MONO_ITEM fn <Struct<u64> as Trait>::foo
//~ MONO_ITEM fn <Struct<u64> as Trait>::bar
let _ = &s1 as &Trait;
Copy link
Member Author

Choose a reason for hiding this comment

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

It seems to me this test is mostly a duplicate of the unsizing test, so I'm skipping updating it with its own drop glue variant -- seems like that's covered there. Maybe worth deleting one of these entirely in a follow-up PR though.

Expand Down
29 changes: 25 additions & 4 deletions tests/codegen-units/item-collection/non-generic-closures.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"]
Expand All @@ -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;
};
Expand All @@ -40,13 +39,28 @@ 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::<PresentDrop> - 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::<PresentDrop> @@ 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 {
temporary();
assigned_to_variable_but_not_executed();
assigned_to_variable_executed_directly();
assigned_to_variable_executed_indirectly();
with_drop(PresentDrop);

0
}
Expand All @@ -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 <PresentDrop as std::ops::Drop>::drop @@ non_generic_closures-cgu.0[External]
fn drop(&mut self) {}
}
22 changes: 18 additions & 4 deletions tests/codegen-units/item-collection/unsizing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,33 +42,47 @@ struct Wrapper<T: ?Sized>(#[allow(dead_code)] *const T);

impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Wrapper<U>> for Wrapper<T> {}

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::<bool> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <bool as Trait>::foo
let _bool_unsized = bool_sized as &Trait;

let char_sized = &'a';

//~ MONO_ITEM fn std::ptr::drop_in_place::<char> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <char as Trait>::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::<f64> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <f64 as Trait>::foo
let _struct_unsized = struct_sized as &Struct<Trait>;

// custom coercion
let wrapper_sized = Wrapper(&0u32);
//~ MONO_ITEM fn std::ptr::drop_in_place::<u32> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <u32 as Trait>::foo
let _wrapper_sized = wrapper_sized as Wrapper<Trait>;

// with drop
let droppable = &PresentDrop;
//~ MONO_ITEM fn <PresentDrop as std::ops::Drop>::drop @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn std::ptr::drop_in_place::<PresentDrop> - shim(Some(PresentDrop)) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <PresentDrop as Trait>::foo
let droppable = droppable as &dyn Trait;

false.foo();

0
Expand Down
14 changes: 9 additions & 5 deletions tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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<dyn Send>;
let _ = Box::new(NonEmptyDrop) as Box<dyn Send>;
let _ = Box::new(PresentDrop) as Box<dyn Send>;
}

// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE"}
Loading