change the type of the argument of drop_in_place lang item to &mut _

[WaffleLapkin]

View all comments

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

rust/compiler/rustc_ty_utils/src/abi.rs

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]

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]

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+.

[rustbot]

r? @TaKO8Ki

rustbot has assigned @TaKO8Ki.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 69 candidates
• Random selection from 11 candidates

[rustbot]

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.

[WaffleLapkin]

(#155044 is merged, ci is passing, this PR is ready for review again :3)