change the type of the argument of `drop_in_place` lang item to `&mut _` by WaffleLapkin · Pull Request #154327 · rust-lang/rust

WaffleLapkin · 2026-03-24T17:44:25Z

We used to special case core::ptr::drop_in_place when computing LLVM argument attributes with this hack:

Lines 383 to 392 in db5e2dc

    
           let drop_target_pointee_info = drop_target_pointee.and_then(|pointee| { 
        
               assert_eq!(pointee, layout.ty.builtin_deref(true).unwrap()); 
        
               assert_eq!(offset, Size::ZERO); 
        
               // The argument to `drop_in_place` is semantically equivalent to a mutable reference. 
        
               let mutref = Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, pointee); 
        
               let layout = cx.layout_of(mutref).unwrap(); 
        
               layout.pointee_info_at(&cx, offset) 
        
           }); 
        
           if let Some(pointee) = drop_target_pointee_info.or_else(|| layout.pointee_info_at(&cx, offset))

This is because even though drop_in_place takes a *mut T it is semantically a &mut T (remember how &mut Self is passed to Drop::drop). This is apparently relevant for perf.

This PR replaces this hack with a simpler solution -- it makes drop_in_place a thin wrapper around newly added core::ptr::drop_glue, which is the actual lang item and takes a &mut T:

rust/library/core/src/ptr/mod.rs

Lines 810 to 833 in d2563d5

    
           pub const unsafe fn drop_in_place<T: PointeeSized>(to_drop: *mut T) 
        
           where 
        
               T: [const] Destruct, 
        
           { 
        
               // Due to historic reasons, `drop_in_place` takes a pointer rather than a reference, 
        
               // which results in worse codegen since we don't apply noalias/dereferenceable llvm 
        
               // attributes to pointer arguments. To workaround this without breaking public 
        
               // interface, `drop_in_place` calls the lang item, rather than being one directly. 
        
               // SAFETY: 
        
               // - compiler glue has the same safety requirements as this function 
        
               // - the pointer must be valid as per the safety requirement of this function 
        
               unsafe { drop_glue(&mut *to_drop) } 
        
           } 
        
           /// Helper function for `drop_in_place`. 
        
           #[lang = "drop_glue"] 
        
           const unsafe fn drop_glue<T: PointeeSized>(_: &mut T) 
        
           where 
        
               T: [const] Destruct, 
        
           { 
        
               // Code here does not matter - this is replaced by the 
        
               // real drop glue by the compiler. 
        
           }

The rest of the PR is blessing tests and cleaning up things which are not necessary after this change.

One thing that is a bit awkward is that now that drop_glue is the actual lang item, a lot of the comments referring to drop_in_place are outdated. Should I try fixing that?

I've also changed async_drop_in_place to take a &mut T, and it simplified the code handling it a bit. (since it's unstable we don't need to introduce a wrapper)

cc @RalfJung
Closes #154274

RalfJung · 2026-03-24T17:47:02Z

Oh wow I hadn't expected my wish to be fulfilled before I could even finish my PR that motivated me to express the wish in the first place. :-) ❤️

RalfJung · 2026-04-06T13:52:26Z

I like the interpreter changes and renames. Not sure what the status of the rest of the PR is, but if you submit just those as a separate PR I'll r+.

…s, r=RalfJung Slightly refactor mplace<->ptr conversions split off of rust-lang#154327 r? RalfJung

@RalfJung

…ottmcm,saethlin change the type of the argument of `drop_in_place` lang item to `&mut _` We used to special case `core::ptr::drop_in_place` when computing LLVM argument attributes with this hack: https://github.com/rust-lang/rust/blob/db5e2dc248fe5bb26f70d7baec46a3bca9fa3e1d/compiler/rustc_ty_utils/src/abi.rs#L383-L392 This is because even though `drop_in_place` takes a `*mut T` it is semantically a `&mut T` (remember how `&mut Self` is passed to `Drop::drop`). This is apparently relevant for perf. This PR replaces this hack with a simpler solution -- it makes `drop_in_place` a thin wrapper around newly added `core::ptr::drop_glue`, which is the actual lang item and takes a `&mut T`: https://github.com/rust-lang/rust/blob/d2563d5003bbecff1efc40c1f5673ceec603825b/library/core/src/ptr/mod.rs#L810-L833 ------ The rest of the PR is blessing tests and cleaning up things which are not necessary after this change. One thing that is a bit awkward is that now that `drop_glue` is the actual lang item, a lot of the comments referring to `drop_in_place` are outdated. Should I try fixing that? I've also changed `async_drop_in_place` to take a `&mut T`, and it simplified the code handling it a bit. (since it's unstable we don't need to introduce a wrapper) ------- cc @RalfJung Closes #154274

rust-bors · 2026-05-05T00:51:47Z

💔 Test for 2e5d870 failed: CI. Failed job:

auto - dist-x86_64-mingw (web logs, enhanced plaintext logs)

WaffleLapkin · 2026-05-05T10:38:47Z

Had to cfg(bootstrap) the lang item rename on library/rtstartup.

@bors try dist-x86_64-mingw

rust-bors · 2026-05-05T10:38:49Z

Unknown argument "dist-x86_64-mingw". Did you mean to use @bors jobs=<jobs>|parent=<parent>? Run @bors help to see available commands.

WaffleLapkin · 2026-05-05T10:39:01Z

@bors try jobs=dist-x86_64-mingw

change the type of the argument of `drop_in_place` lang item to `&mut _` try-job: dist-x86_64-mingw

rust-bors · 2026-05-05T12:50:34Z

☀️ Try build successful (CI)
Build commit: c04492d (c04492d91a204261d6c4f2e6bc89590fec806b36, parent: 83adf7e080d0fe4f0970c00eac2976a767dbd042)

WaffleLapkin · 2026-05-05T12:51:46Z

@bors r=RalfJung,scottmcm,saethlin

rust-bors · 2026-05-05T12:51:49Z

📌 Commit 29d32c7 has been approved by RalfJung,scottmcm,saethlin

It is now in the queue for this repository.

@RalfJung

…ottmcm,saethlin change the type of the argument of `drop_in_place` lang item to `&mut _` We used to special case `core::ptr::drop_in_place` when computing LLVM argument attributes with this hack: https://github.com/rust-lang/rust/blob/db5e2dc248fe5bb26f70d7baec46a3bca9fa3e1d/compiler/rustc_ty_utils/src/abi.rs#L383-L392 This is because even though `drop_in_place` takes a `*mut T` it is semantically a `&mut T` (remember how `&mut Self` is passed to `Drop::drop`). This is apparently relevant for perf. This PR replaces this hack with a simpler solution -- it makes `drop_in_place` a thin wrapper around newly added `core::ptr::drop_glue`, which is the actual lang item and takes a `&mut T`: https://github.com/rust-lang/rust/blob/d2563d5003bbecff1efc40c1f5673ceec603825b/library/core/src/ptr/mod.rs#L810-L833 ------ The rest of the PR is blessing tests and cleaning up things which are not necessary after this change. One thing that is a bit awkward is that now that `drop_glue` is the actual lang item, a lot of the comments referring to `drop_in_place` are outdated. Should I try fixing that? I've also changed `async_drop_in_place` to take a `&mut T`, and it simplified the code handling it a bit. (since it's unstable we don't need to introduce a wrapper) ------- cc @RalfJung Closes #154274

rust-log-analyzer · 2026-05-05T15:13:49Z

The job test-various failed! Check out the build log: (web) (plain enhanced) (plain)

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

73 +         StorageLive(_11);
+ +         StorageLive(_9);
74 +         StorageLive(_10);
75 +         StorageLive(_12);
76 +         _12 = copy (((((*_7).0: alloc::raw_vec::RawVec<A>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);

109 +         StorageDead(_15);
110 +         StorageDead(_14);
111 +         StorageDead(_13);
- +         StorageDead(_11);
113 +         StorageDead(_9);
+ +         StorageDead(_11);
114 +         drop(((*_6).0: alloc::raw_vec::RawVec<A>)) -> [return: bb1, unwind unreachable];
115 +     }
116 + 


thread '[mir-opt] tests/mir-opt/inline/inline_shims.rs' panicked at src/tools/compiletest/src/runtest/mir_opt.rs:73:21:
Actual MIR output differs from expected MIR output /checkout/tests/mir-opt/inline/inline_shims.drop.Inline.panic-abort.diff
stack backtrace:
   5: __rustc::rust_begin_unwind
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/panicking.rs:689:5
   6: core::panicking::panic_fmt
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/core/src/panicking.rs:80:14

rust-bors · 2026-05-05T15:14:07Z

💔 Test for 783a3e6 failed: CI. Failed job:

auto - test-various (web logs, enhanced plaintext logs)

RalfJung reviewed Mar 24, 2026

View reviewed changes

Comment thread library/core/src/ptr/mod.rs Outdated

This comment has been minimized.

Sign in to view

RalfJung reviewed Mar 25, 2026

View reviewed changes

Comment thread compiler/rustc_const_eval/src/interpret/place.rs

Comment thread library/core/src/ptr/mod.rs Outdated

WaffleLapkin force-pushed the drop_in_place_ref branch from 281786e to 4842194 Compare April 2, 2026 11:19

This comment has been minimized.

Sign in to view

RalfJung reviewed Apr 4, 2026

View reviewed changes

Comment thread compiler/rustc_const_eval/src/interpret/place.rs

WaffleLapkin force-pushed the drop_in_place_ref branch 2 times, most recently from 794fcf5 to ea0c913 Compare April 5, 2026 18:31

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Apr 5, 2026

This comment has been minimized.

Sign in to view

WaffleLapkin force-pushed the drop_in_place_ref branch from ea0c913 to 3dda404 Compare April 5, 2026 18:47

This comment has been minimized.

Sign in to view

WaffleLapkin force-pushed the drop_in_place_ref branch from 3dda404 to 32510c2 Compare April 6, 2026 12:22

This comment has been minimized.

Sign in to view

RalfJung reviewed Apr 6, 2026

View reviewed changes

Comment thread compiler/rustc_const_eval/src/interpret/place.rs Outdated

WaffleLapkin force-pushed the drop_in_place_ref branch from 32510c2 to 487fcc4 Compare April 6, 2026 14:00

This comment has been minimized.

Sign in to view

WaffleLapkin mentioned this pull request Apr 6, 2026

Slightly refactor mplace<->ptr conversions #154894

Merged

WaffleLapkin force-pushed the drop_in_place_ref branch from 487fcc4 to 6e43096 Compare April 6, 2026 15:56

This comment has been minimized.

Sign in to view

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Apr 6, 2026

Rollup merge of rust-lang#154894 - WaffleLapkin:mplace_ptr_conversion…

d1e186f

…s, r=RalfJung Slightly refactor mplace<->ptr conversions split off of rust-lang#154327 r? RalfJung

This comment has been minimized.

Sign in to view

rust-bors Bot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 5, 2026

WaffleLapkin force-pushed the drop_in_place_ref branch from e0ecfbb to 14a4b09 Compare May 5, 2026 10:35

rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2026

WaffleLapkin added 5 commits May 5, 2026 12:36

rename drop_in_place lang item to drop_glue

810eae9

change type of async_drop_in_place to accept &mut _

2051cb8

bless mir-opt tests

93aca7e

bless codegen-units tests

df8f82b

make NonNull::drop_in_place call drop_glue directly

29d32c7

WaffleLapkin force-pushed the drop_in_place_ref branch from 14a4b09 to 29d32c7 Compare May 5, 2026 10:38

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request May 5, 2026

Auto merge of #154327 - WaffleLapkin:drop_in_place_ref, r=<try>

c04492d

change the type of the argument of `drop_in_place` lang item to `&mut _` try-job: dist-x86_64-mingw

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 5, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 5, 2026

	let drop_target_pointee_info = drop_target_pointee.and_then(\|pointee\| {
	assert_eq!(pointee, layout.ty.builtin_deref(true).unwrap());
	assert_eq!(offset, Size::ZERO);
	// The argument to `drop_in_place` is semantically equivalent to a mutable reference.
	let mutref = Ty::new_mut_ref(tcx, tcx.lifetimes.re_erased, pointee);
	let layout = cx.layout_of(mutref).unwrap();
	layout.pointee_info_at(&cx, offset)
	});

	if let Some(pointee) = drop_target_pointee_info.or_else(\|\| layout.pointee_info_at(&cx, offset))

	pub const unsafe fn drop_in_place<T: PointeeSized>(to_drop: *mut T)
	where
	T: [const] Destruct,
	{
	// Due to historic reasons, `drop_in_place` takes a pointer rather than a reference,
	// which results in worse codegen since we don't apply noalias/dereferenceable llvm
	// attributes to pointer arguments. To workaround this without breaking public
	// interface, `drop_in_place` calls the lang item, rather than being one directly.

	// SAFETY:
	// - compiler glue has the same safety requirements as this function
	// - the pointer must be valid as per the safety requirement of this function
	unsafe { drop_glue(&mut *to_drop) }
	}

	/// Helper function for `drop_in_place`.
	#[lang = "drop_glue"]
	const unsafe fn drop_glue<T: PointeeSized>(_: &mut T)
	where
	T: [const] Destruct,
	{
	// Code here does not matter - this is replaced by the
	// real drop glue by the compiler.
	}

Uh oh!

Conversation

WaffleLapkin commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Mar 24, 2026

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

RalfJung commented Apr 6, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented May 5, 2026

Uh oh!

WaffleLapkin commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

WaffleLapkin commented May 5, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented May 5, 2026

Uh oh!

WaffleLapkin commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

This comment has been minimized.

rust-log-analyzer commented May 5, 2026

Uh oh!

rust-bors Bot commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

WaffleLapkin commented Mar 24, 2026 •

edited

Loading