Auto merge of #154327 - WaffleLapkin:drop_in_place_ref, r=RalfJung,scottmcm,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

Author: bors

compiler/rustc_codegen_cranelift/example/mini_core.rs

@@ -575,14 +575,10 @@ unsafe extern "C" {
     fn _Unwind_Resume(exc: *mut ()) -> !;
 }
 
-#[lang = "drop_in_place"]
-#[allow(unconditional_recursion)]
-pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
+#[lang = "drop_glue"]
+pub unsafe fn drop_glue<T: ?Sized>(_to_drop: &mut T) {
     // Code here does not matter - this is replaced by the
     // real drop glue by the compiler.
-    unsafe {
-        drop_in_place(to_drop);
-    }
 }
 
 #[lang = "unpin"]

compiler/rustc_codegen_cranelift/src/abi/mod.rs

@@ -701,7 +701,7 @@ pub(crate) fn codegen_drop<'tcx>(
     unwind: UnwindAction,
 ) {
     let ty = drop_place.layout().ty;
-    let drop_instance = Instance::resolve_drop_in_place(fx.tcx, ty);
+    let drop_instance = Instance::resolve_drop_glue(fx.tcx, ty);
     let ret_block = fx.get_block(target);
 
     // AsyncDropGlueCtorShim can't be here
@@ -734,7 +734,7 @@ pub(crate) fn codegen_drop<'tcx>(
                 fx.bcx.switch_to_block(continued);
 
                 // FIXME(eddyb) perhaps move some of this logic into
-                // `Instance::resolve_drop_in_place`?
+                // `Instance::resolve_drop_glue`?
                 let virtual_drop = Instance {
                     def: ty::InstanceKind::Virtual(drop_instance.def_id(), 0),
                     args: drop_instance.args,

compiler/rustc_codegen_gcc/example/mini_core.rs

@@ -577,12 +577,10 @@ fn eh_personality() -> ! {
     loop {}
 }
 
-#[lang = "drop_in_place"]
-#[allow(unconditional_recursion)]
-pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
+#[lang = "drop_glue"]
+pub unsafe fn drop_glue<T: ?Sized>(_to_drop: &mut T) {
     // Code here does not matter - this is replaced by the
     // real drop glue by the compiler.
-    drop_in_place(to_drop);
 }
 
 #[lang = "unpin"]

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -403,19 +403,18 @@ fn upstream_monomorphizations_provider(
 
     let mut instances: DefIdMap<UnordMap<_, _>> = Default::default();
 
-    let drop_in_place_fn_def_id = tcx.lang_items().drop_in_place_fn();
+    let drop_glue_fn_def_id = tcx.lang_items().drop_glue_fn();
     let async_drop_in_place_fn_def_id = tcx.lang_items().async_drop_in_place_fn();
 
     for &cnum in cnums.iter() {
         for (exported_symbol, _) in tcx.exported_generic_symbols(cnum).iter() {
             let (def_id, args) = match *exported_symbol {
                 ExportedSymbol::Generic(def_id, args) => (def_id, args),
                 ExportedSymbol::DropGlue(ty) => {
-                    if let Some(drop_in_place_fn_def_id) = drop_in_place_fn_def_id {
+                    if let Some(drop_in_place_fn_def_id) = drop_glue_fn_def_id {
                         (drop_in_place_fn_def_id, tcx.mk_args(&[ty.into()]))
                     } else {
-                        // `drop_in_place` in place does not exist, don't try
-                        // to use it.
+                        // `drop_glue` does not exist, don't try to use it.
                         continue;
                     }
                 }
@@ -465,7 +464,7 @@ fn upstream_drop_glue_for_provider<'tcx>(
     tcx: TyCtxt<'tcx>,
     args: GenericArgsRef<'tcx>,
 ) -> Option<CrateNum> {
-    let def_id = tcx.lang_items().drop_in_place_fn()?;
+    let def_id = tcx.lang_items().drop_glue_fn()?;
     tcx.upstream_monomorphizations_for(def_id)?.get(&args).cloned()
 }
 
@@ -587,7 +586,7 @@ pub(crate) fn symbol_name_for_instance_in_crate<'tcx>(
         }
         ExportedSymbol::DropGlue(ty) => rustc_symbol_mangling::symbol_name_for_instance_in_crate(
             tcx,
-            Instance::resolve_drop_in_place(tcx, ty),
+            Instance::resolve_drop_glue(tcx, ty),
             instantiating_crate,
         ),
         ExportedSymbol::AsyncDropGlueCtorShim(ty) => {

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -603,7 +603,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     ) -> MergingSucc {
         let ty = location.ty(self.mir, bx.tcx()).ty;
         let ty = self.monomorphize(ty);
-        let drop_fn = Instance::resolve_drop_in_place(bx.tcx(), ty);
+        let drop_fn = Instance::resolve_drop_glue(bx.tcx(), ty);
 
         if let ty::InstanceKind::DropGlue(_, None) = drop_fn.def {
             // we don't actually need to drop anything.
@@ -621,7 +621,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         };
         let (maybe_null, drop_fn, fn_abi, drop_instance) = match ty.kind() {
             // FIXME(eddyb) perhaps move some of this logic into
-            // `Instance::resolve_drop_in_place`?
+            // `Instance::resolve_drop_glue`?
             ty::Dynamic(_, _) => {
                 // IN THIS ARM, WE HAVE:
                 // ty = *mut (dyn Trait)

compiler/rustc_const_eval/src/interpret/call.rs

@@ -7,7 +7,7 @@ use std::borrow::Cow;
 use either::{Left, Right};
 use rustc_abi::{self as abi, ExternAbi, FieldIdx, Integer, VariantIdx};
 use rustc_hir::def_id::DefId;
-use rustc_hir::{LangItem, find_attr};
+use rustc_hir::find_attr;
 use rustc_middle::ty::layout::{IntegerExt, TyAndLayout};
 use rustc_middle::ty::{self, AdtDef, Instance, Ty, Unnormalized, VariantDef};
 use rustc_middle::{bug, mir, span_bug};
@@ -278,7 +278,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         callee_arg: &mir::Place<'tcx>,
         callee_ty: Ty<'tcx>,
         already_live: bool,
-        is_drop_in_place: bool,
     ) -> InterpResult<'tcx>
     where
         'tcx: 'x,
@@ -323,16 +322,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             self.storage_live_dyn(local, meta)?;
         }
         // Now we can finally actually evaluate the callee place.
-        let mut callee_arg = self.eval_place(*callee_arg)?;
-        // drop_in_place has a signature which says that the first argument is `*mut T`
-        // but really it's `&mut T`. This is where we handle that terrible hack in
-        // the MIR semantics.
-        // FIXME(#154274): remove this hack.
-        if is_drop_in_place && callee_arg_idx == 0 {
-            let pointee_ty = callee_arg.layout.ty.builtin_deref(true).unwrap();
-            let mutref_ty = Ty::new_mut_ref(*self.tcx, self.tcx.lifetimes.re_erased, pointee_ty);
-            callee_arg = callee_arg.transmute(self.layout_of(mutref_ty)?, self)?;
-        }
+        let callee_arg = self.eval_place(*callee_arg)?;
         // We allow some transmutes here.
         // FIXME: Depending on the PassMode, this should reset some padding to uninitialized. (This
         // is true for all `copy_op`, but there are a lot of special cases for argument passing
@@ -464,12 +454,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
         // Determine whether there is a special VaList argument. This is always the
         // last argument, and since arguments start at index 1 that's `arg_count`.
         let va_list_arg = callee_fn_abi.c_variadic.then(|| mir::Local::from_usize(body.arg_count));
-        // Part of the hack for #154274, see `pass_argument`.
-        let is_drop_in_place = {
-            let def_id = body.source.def_id();
-            self.tcx.is_lang_item(def_id, LangItem::DropInPlace)
-                || self.tcx.is_lang_item(def_id, LangItem::AsyncDropInPlace)
-        };
 
         // During argument passing, we want retagging with protectors.
         M::with_retag_mode(self, RetagMode::FnEntry, |ecx| {
@@ -532,7 +516,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                             &dest,
                             field_ty,
                             /* already_live */ true,
-                            is_drop_in_place,
                         )?;
                     }
                 } else {
@@ -545,7 +528,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                         &dest,
                         ty,
                         /* already_live */ false,
-                        is_drop_in_place,
                     )?;
                 }
             }
@@ -913,19 +895,21 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
             _ => {
                 debug_assert_eq!(
                     instance,
-                    ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty)
+                    ty::Instance::resolve_drop_glue(*self.tcx, place.layout.ty)
                 );
                 place
             }
         };
+
         let instance = {
-            let _trace =
-                enter_trace_span!(M, resolve::resolve_drop_in_place, ty = ?place.layout.ty);
-            ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty)
+            let _trace = enter_trace_span!(M, resolve::resolve_drop_glue, ty = ?place.layout.ty);
+            ty::Instance::resolve_drop_glue(*self.tcx, place.layout.ty)
         };
         let fn_abi = self.fn_abi_of_instance_no_deduced_attrs(instance, ty::List::empty())?;
 
-        let arg = self.mplace_to_imm_ptr(&place, None)?;
+        let ref_ty = Ty::new_mut_ref(self.tcx.tcx, self.tcx.lifetimes.re_erased, place.layout.ty);
+        let arg = self.mplace_to_imm_ptr(&place, Some(ref_ty))?;
+
         let ret = MPlaceTy::fake_alloc_zst(self.layout_of(self.tcx.types.unit)?);
 
         self.init_fn_call(

compiler/rustc_const_eval/src/interpret/step.rs

@@ -593,8 +593,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
                 let place = self.eval_place(place)?;
                 let instance = {
                     let _trace =
-                        enter_trace_span!(M, resolve::resolve_drop_in_place, ty = ?place.layout.ty);
-                    Instance::resolve_drop_in_place(*self.tcx, place.layout.ty)
+                        enter_trace_span!(M, resolve::resolve_drop_glue, ty = ?place.layout.ty);
+                    Instance::resolve_drop_glue(*self.tcx, place.layout.ty)
                 };
                 if let ty::InstanceKind::DropGlue(_, None) = instance.def {
                     // This is the branch we enter if and only if the dropped type has no drop glue

compiler/rustc_hir/src/lang_items.rs

@@ -321,7 +321,8 @@ language_item_table! {
     FormatArgument,          sym::format_argument,     format_argument,            Target::Struct,         GenericRequirement::None;
     FormatArguments,         sym::format_arguments,    format_arguments,           Target::Struct,         GenericRequirement::None;
 
-    DropInPlace,             sym::drop_in_place,       drop_in_place_fn,           Target::Fn,             GenericRequirement::Minimum(1);
+    // Compiler-generated drop glue function, aka `core::ptr::drop_glue`
+    DropGlue,                sym::drop_glue,           drop_glue_fn,               Target::Fn,             GenericRequirement::Exact(1);
     AllocLayout,             sym::alloc_layout,        alloc_layout,               Target::Struct,         GenericRequirement::None;
 
     /// For all binary crates without `#![no_main]`, Rust will generate a "main" function.

compiler/rustc_middle/src/hir/mod.rs

@@ -300,8 +300,8 @@ impl<'tcx> TyCtxt<'tcx> {
             Node::Expr(parent_expr) => {
                 match parent_expr.kind {
                     // Addr-of, field projections, and LHS of assignment don't constitute reads.
-                    // Assignment does call `drop_in_place`, though, but its safety
-                    // requirements are not the same.
+                    // Assignment does call `drop_glue`, though, but its safety requirements are
+                    // not the same.
                     ExprKind::AddrOf(..) | ExprKind::Field(..) => false,
 
                     // Place-preserving expressions only constitute reads if their

compiler/rustc_middle/src/middle/exported_symbols.rs

@@ -66,7 +66,7 @@ impl<'tcx> ExportedSymbol<'tcx> {
                 tcx.symbol_name(ty::Instance::new_raw(def_id, args))
             }
             ExportedSymbol::DropGlue(ty) => {
-                tcx.symbol_name(ty::Instance::resolve_drop_in_place(tcx, ty))
+                tcx.symbol_name(ty::Instance::resolve_drop_glue(tcx, ty))
             }
             ExportedSymbol::AsyncDropGlueCtorShim(ty) => {
                 tcx.symbol_name(ty::Instance::resolve_async_drop_in_place(tcx, ty))