Skip to content

Commit

Permalink
Auto merge of rust-lang#134326 - scottmcm:slice-drop-shim-ptrmetadata…
Browse files Browse the repository at this point in the history
…, r=saethlin

Use `PtrMetadata` instead of `Len` in slice drop shims

I tried to do a bigger change in rust-lang#134297 which didn't work, so here's the part I really wanted: Removing another use of `Len`, in favour of `PtrMetadata`.

Split into two commits where the first just adds a test, so you can look at the second commit to see how the drop shim for an array changes with this PR.

Reusing the same reviewer from the last one:
r? BoxyUwU
  • Loading branch information
bors committed Dec 22, 2024
2 parents bace306 + 8acccdc commit b22856d
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 10 deletions.
75 changes: 66 additions & 9 deletions compiler/rustc_mir_dataflow/src/elaborate_drops.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::{fmt, iter};
use std::{fmt, iter, mem};

use rustc_abi::{FIRST_VARIANT, FieldIdx, VariantIdx};
use rustc_hir::lang_items::LangItem;
use rustc_index::Idx;
use rustc_middle::mir::patch::MirPatch;
use rustc_middle::mir::*;
use rustc_middle::span_bug;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt};
use rustc_span::DUMMY_SP;
Expand Down Expand Up @@ -738,8 +739,13 @@ where
loop_block
}

fn open_drop_for_array(&mut self, ety: Ty<'tcx>, opt_size: Option<u64>) -> BasicBlock {
debug!("open_drop_for_array({:?}, {:?})", ety, opt_size);
fn open_drop_for_array(
&mut self,
array_ty: Ty<'tcx>,
ety: Ty<'tcx>,
opt_size: Option<u64>,
) -> BasicBlock {
debug!("open_drop_for_array({:?}, {:?}, {:?})", array_ty, ety, opt_size);
let tcx = self.tcx();

if let Some(size) = opt_size {
Expand Down Expand Up @@ -801,13 +807,50 @@ where
}
}

self.drop_loop_pair(ety)
let array_ptr_ty = Ty::new_mut_ptr(tcx, array_ty);
let array_ptr = self.new_temp(array_ptr_ty);

let slice_ty = Ty::new_slice(tcx, ety);
let slice_ptr_ty = Ty::new_mut_ptr(tcx, slice_ty);
let slice_ptr = self.new_temp(slice_ptr_ty);

let mut delegate_block = BasicBlockData {
statements: vec![
self.assign(Place::from(array_ptr), Rvalue::RawPtr(Mutability::Mut, self.place)),
self.assign(
Place::from(slice_ptr),
Rvalue::Cast(
CastKind::PointerCoercion(
PointerCoercion::Unsize,
CoercionSource::Implicit,
),
Operand::Move(Place::from(array_ptr)),
slice_ptr_ty,
),
),
],
is_cleanup: self.unwind.is_cleanup(),
terminator: None,
};

let array_place = mem::replace(
&mut self.place,
Place::from(slice_ptr).project_deeper(&[PlaceElem::Deref], tcx),
);
let slice_block = self.drop_loop_pair_for_slice(ety);
self.place = array_place;

delegate_block.terminator = Some(Terminator {
source_info: self.source_info,
kind: TerminatorKind::Goto { target: slice_block },
});
self.elaborator.patch().new_block(delegate_block)
}

/// Creates a pair of drop-loops of `place`, which drops its contents, even
/// in the case of 1 panic.
fn drop_loop_pair(&mut self, ety: Ty<'tcx>) -> BasicBlock {
debug!("drop_loop_pair({:?})", ety);
fn drop_loop_pair_for_slice(&mut self, ety: Ty<'tcx>) -> BasicBlock {
debug!("drop_loop_pair_for_slice({:?})", ety);
let tcx = self.tcx();
let len = self.new_temp(tcx.types.usize);
let cur = self.new_temp(tcx.types.usize);
Expand All @@ -817,10 +860,24 @@ where

let loop_block = self.drop_loop(self.succ, cur, len, ety, unwind);

let [PlaceElem::Deref] = self.place.projection.as_slice() else {
span_bug!(
self.source_info.span,
"Expected place for slice drop shim to be *_n, but it's {:?}",
self.place,
);
};

let zero = self.constant_usize(0);
let block = BasicBlockData {
statements: vec![
self.assign(len.into(), Rvalue::Len(self.place)),
self.assign(
len.into(),
Rvalue::UnaryOp(
UnOp::PtrMetadata,
Operand::Copy(Place::from(self.place.local)),
),
),
self.assign(cur.into(), Rvalue::Use(zero)),
],
is_cleanup: unwind.is_cleanup(),
Expand Down Expand Up @@ -863,9 +920,9 @@ where
ty::Dynamic(..) => self.complete_drop(self.succ, self.unwind),
ty::Array(ety, size) => {
let size = size.try_to_target_usize(self.tcx());
self.open_drop_for_array(*ety, size)
self.open_drop_for_array(ty, *ety, size)
}
ty::Slice(ety) => self.drop_loop_pair(*ety),
ty::Slice(ety) => self.drop_loop_pair_for_slice(*ety),

_ => span_bug!(self.source_info.span, "open drop from non-ADT `{:?}`", ty),
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// MIR for `std::ptr::drop_in_place` before AddMovesForPackedDrops

fn std::ptr::drop_in_place(_1: *mut [String; 42]) -> () {
let mut _0: ();
let mut _2: *mut [std::string::String; 42];
let mut _3: *mut [std::string::String];
let mut _4: usize;
let mut _5: usize;
let mut _6: *mut std::string::String;
let mut _7: bool;
let mut _8: *mut std::string::String;
let mut _9: bool;

bb0: {
goto -> bb9;
}

bb1: {
return;
}

bb2 (cleanup): {
resume;
}

bb3 (cleanup): {
_6 = &raw mut (*_3)[_5];
_5 = Add(move _5, const 1_usize);
drop((*_6)) -> [return: bb4, unwind terminate(cleanup)];
}

bb4 (cleanup): {
_7 = Eq(copy _5, copy _4);
switchInt(move _7) -> [0: bb3, otherwise: bb2];
}

bb5: {
_8 = &raw mut (*_3)[_5];
_5 = Add(move _5, const 1_usize);
drop((*_8)) -> [return: bb6, unwind: bb4];
}

bb6: {
_9 = Eq(copy _5, copy _4);
switchInt(move _9) -> [0: bb5, otherwise: bb1];
}

bb7: {
_4 = PtrMetadata(copy _3);
_5 = const 0_usize;
goto -> bb6;
}

bb8: {
goto -> bb7;
}

bb9: {
_2 = &raw mut (*_1);
_3 = move _2 as *mut [std::string::String] (PointerCoercion(Unsize, Implicit));
goto -> bb8;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ fn std::ptr::drop_in_place(_1: *mut [String]) -> () {
}

bb7: {
_2 = Len((*_1));
_2 = PtrMetadata(copy _1);
_3 = const 0_usize;
goto -> bb6;
}
Expand Down
2 changes: 2 additions & 0 deletions tests/mir-opt/slice_drop_shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
// if we use -Clink-dead-code.

// EMIT_MIR core.ptr-drop_in_place.[String].AddMovesForPackedDrops.before.mir
// EMIT_MIR core.ptr-drop_in_place.[String;42].AddMovesForPackedDrops.before.mir
fn main() {
let _fn = std::ptr::drop_in_place::<[String]> as unsafe fn(_);
let _fn = std::ptr::drop_in_place::<[String; 42]> as unsafe fn(_);
}

0 comments on commit b22856d

Please sign in to comment.