Skip to content

Commit

Permalink
Auto merge of #50097 - glandium:box_free, r=nikomatsakis
Browse files Browse the repository at this point in the history
Partial future-proofing for Box<T, A>

In some ways, this is similar to @eddyb's PR #47043 that went stale, but doesn't cover everything. Notably, this still leaves Box internalized as a pointer in places, so practically speaking, only ZSTs can be practically added to the Box type with the changes here (the compiler ICEs otherwise).

The Box type is not changed here, that's left for the future because I want to test that further first, but this puts things in place in a way that hopefully will make things easier.
  • Loading branch information
bors committed Apr 27, 2018
2 parents ada45fd + bd8c177 commit 71d3dac
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 154 deletions.
14 changes: 11 additions & 3 deletions src/liballoc/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
issue = "32838")]

use core::intrinsics::{min_align_of_val, size_of_val};
use core::ptr::NonNull;
use core::ptr::{NonNull, Unique};
use core::usize;

#[doc(inline)]
Expand Down Expand Up @@ -152,9 +152,17 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
}
}

#[cfg_attr(not(test), lang = "box_free")]
#[cfg(stage0)]
#[lang = "box_free"]
#[inline]
unsafe fn old_box_free<T: ?Sized>(ptr: *mut T) {
box_free(Unique::new_unchecked(ptr))
}

#[cfg_attr(not(any(test, stage0)), lang = "box_free")]
#[inline]
pub(crate) unsafe fn box_free<T: ?Sized>(ptr: *mut T) {
pub(crate) unsafe fn box_free<T: ?Sized>(ptr: Unique<T>) {
let ptr = ptr.as_ptr();
let size = size_of_val(&*ptr);
let align = min_align_of_val(&*ptr);
// We do not allocate for Box<T> when T is ZST, so deallocation is also not necessary.
Expand Down
5 changes: 3 additions & 2 deletions src/liballoc/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,8 @@ impl<T: ?Sized> Arc<T> {

fn from_box(v: Box<T>) -> Arc<T> {
unsafe {
let bptr = Box::into_raw(v);
let box_unique = Box::into_unique(v);
let bptr = box_unique.as_ptr();

let value_size = size_of_val(&*bptr);
let ptr = Self::allocate_for_ptr(bptr);
Expand All @@ -578,7 +579,7 @@ impl<T: ?Sized> Arc<T> {
value_size);

// Free the allocation without dropping its contents
box_free(bptr);
box_free(box_unique);

Arc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData }
}
Expand Down
5 changes: 3 additions & 2 deletions src/liballoc/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,8 @@ impl<T: ?Sized> Rc<T> {

fn from_box(v: Box<T>) -> Rc<T> {
unsafe {
let bptr = Box::into_raw(v);
let box_unique = Box::into_unique(v);
let bptr = box_unique.as_ptr();

let value_size = size_of_val(&*bptr);
let ptr = Self::allocate_for_ptr(bptr);
Expand All @@ -693,7 +694,7 @@ impl<T: ?Sized> Rc<T> {
value_size);

// Free the allocation without dropping its contents
box_free(bptr);
box_free(box_unique);

Rc { ptr: NonNull::new_unchecked(ptr), phantom: PhantomData }
}
Expand Down
12 changes: 10 additions & 2 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use middle::resolve_lifetime::{self, ObjectLifetimeDefault};
use middle::stability;
use mir::{self, Mir, interpret};
use mir::interpret::{Value, PrimVal};
use ty::subst::{Kind, Substs};
use ty::subst::{Kind, Substs, Subst};
use ty::ReprOptions;
use ty::Instance;
use traits;
Expand Down Expand Up @@ -2328,7 +2328,15 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
pub fn mk_box(self, ty: Ty<'tcx>) -> Ty<'tcx> {
let def_id = self.require_lang_item(lang_items::OwnedBoxLangItem);
let adt_def = self.adt_def(def_id);
let substs = self.mk_substs(iter::once(Kind::from(ty)));
let generics = self.generics_of(def_id);
let mut substs = vec![Kind::from(ty)];
// Add defaults for other generic params if there are some.
for def in generics.types.iter().skip(1) {
assert!(def.has_default);
let ty = self.type_of(def.def_id).subst(self, &substs);
substs.push(ty.into());
}
let substs = self.mk_substs(substs.into_iter());
self.mk_ty(TyAdt(adt_def, substs))
}

Expand Down
70 changes: 1 addition & 69 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,11 +918,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
));
}

if self.is_box_free(func) {
self.check_box_free_inputs(mir, term, &sig, args, term_location);
} else {
self.check_call_inputs(mir, term, &sig, args, term_location);
}
self.check_call_inputs(mir, term, &sig, args, term_location);
}
TerminatorKind::Assert {
ref cond, ref msg, ..
Expand Down Expand Up @@ -1026,70 +1022,6 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
}
}

fn is_box_free(&self, operand: &Operand<'tcx>) -> bool {
match *operand {
Operand::Constant(ref c) => match c.ty.sty {
ty::TyFnDef(ty_def_id, _) => {
Some(ty_def_id) == self.tcx().lang_items().box_free_fn()
}
_ => false,
},
_ => false,
}
}

fn check_box_free_inputs(
&mut self,
mir: &Mir<'tcx>,
term: &Terminator<'tcx>,
sig: &ty::FnSig<'tcx>,
args: &[Operand<'tcx>],
term_location: Location,
) {
debug!("check_box_free_inputs");

// box_free takes a Box as a pointer. Allow for that.

if sig.inputs().len() != 1 {
span_mirbug!(self, term, "box_free should take 1 argument");
return;
}

let pointee_ty = match sig.inputs()[0].sty {
ty::TyRawPtr(mt) => mt.ty,
_ => {
span_mirbug!(self, term, "box_free should take a raw ptr");
return;
}
};

if args.len() != 1 {
span_mirbug!(self, term, "box_free called with wrong # of args");
return;
}

let ty = args[0].ty(mir, self.tcx());
let arg_ty = match ty.sty {
ty::TyRawPtr(mt) => mt.ty,
ty::TyAdt(def, _) if def.is_box() => ty.boxed_ty(),
_ => {
span_mirbug!(self, term, "box_free called with bad arg ty");
return;
}
};

if let Err(terr) = self.sub_types(arg_ty, pointee_ty, term_location.at_self()) {
span_mirbug!(
self,
term,
"bad box_free arg ({:?} <- {:?}): {:?}",
pointee_ty,
arg_ty,
terr
);
}
}

fn check_iscleanup(&mut self, mir: &Mir<'tcx>, block_data: &BasicBlockData<'tcx>) {
let is_cleanup = block_data.is_cleanup;
self.last_span = block_data.terminator().source_info.span;
Expand Down
65 changes: 2 additions & 63 deletions src/librustc_mir/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,6 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> {
TerminatorKind::Call { args, destination: Some(destination), cleanup, .. } => {
debug!("Inlined {:?} into {:?}", callsite.callee, self.source);

let is_box_free = Some(callsite.callee) == self.tcx.lang_items().box_free_fn();

let mut local_map = IndexVec::with_capacity(callee_mir.local_decls.len());
let mut scope_map = IndexVec::with_capacity(callee_mir.visibility_scopes.len());
let mut promoted_map = IndexVec::with_capacity(callee_mir.promoted.len());
Expand Down Expand Up @@ -460,24 +458,8 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> {

let return_block = destination.1;

let args : Vec<_> = if is_box_free {
assert!(args.len() == 1);
// box_free takes a Box, but is defined with a *mut T, inlining
// needs to generate the cast.
// FIXME: we should probably just generate correct MIR in the first place...

let arg = if let Operand::Move(ref place) = args[0] {
place.clone()
} else {
bug!("Constant arg to \"box_free\"");
};

let ptr_ty = args[0].ty(caller_mir, self.tcx);
vec![self.cast_box_free_arg(arg, ptr_ty, &callsite, caller_mir)]
} else {
// Copy the arguments if needed.
self.make_call_args(args, &callsite, caller_mir)
};
// Copy the arguments if needed.
let args: Vec<_> = self.make_call_args(args, &callsite, caller_mir);

let bb_len = caller_mir.basic_blocks().len();
let mut integrator = Integrator {
Expand Down Expand Up @@ -518,49 +500,6 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> {
}
}

fn cast_box_free_arg(&self, arg: Place<'tcx>, ptr_ty: Ty<'tcx>,
callsite: &CallSite<'tcx>, caller_mir: &mut Mir<'tcx>) -> Local {
let arg = Rvalue::Ref(
self.tcx.types.re_erased,
BorrowKind::Mut { allow_two_phase_borrow: false },
arg.deref());

let ty = arg.ty(caller_mir, self.tcx);
let ref_tmp = LocalDecl::new_temp(ty, callsite.location.span);
let ref_tmp = caller_mir.local_decls.push(ref_tmp);
let ref_tmp = Place::Local(ref_tmp);

let ref_stmt = Statement {
source_info: callsite.location,
kind: StatementKind::Assign(ref_tmp.clone(), arg)
};

caller_mir[callsite.bb]
.statements.push(ref_stmt);

let pointee_ty = match ptr_ty.sty {
ty::TyRawPtr(tm) | ty::TyRef(_, tm) => tm.ty,
_ if ptr_ty.is_box() => ptr_ty.boxed_ty(),
_ => bug!("Invalid type `{:?}` for call to box_free", ptr_ty)
};
let ptr_ty = self.tcx.mk_mut_ptr(pointee_ty);

let raw_ptr = Rvalue::Cast(CastKind::Misc, Operand::Move(ref_tmp), ptr_ty);

let cast_tmp = LocalDecl::new_temp(ptr_ty, callsite.location.span);
let cast_tmp = caller_mir.local_decls.push(cast_tmp);

let cast_stmt = Statement {
source_info: callsite.location,
kind: StatementKind::Assign(Place::Local(cast_tmp), raw_ptr)
};

caller_mir[callsite.bb]
.statements.push(cast_stmt);

cast_tmp
}

fn make_call_args(
&self,
args: Vec<Operand<'tcx>>,
Expand Down
34 changes: 21 additions & 13 deletions src/librustc_mir/util/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,18 +339,19 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
self.drop_ladder(fields, succ, unwind).0
}

fn open_drop_for_box<'a>(&mut self, ty: Ty<'tcx>) -> BasicBlock
fn open_drop_for_box<'a>(&mut self, adt: &'tcx ty::AdtDef, substs: &'tcx Substs<'tcx>)
-> BasicBlock
{
debug!("open_drop_for_box({:?}, {:?})", self, ty);
debug!("open_drop_for_box({:?}, {:?}, {:?})", self, adt, substs);

let interior = self.place.clone().deref();
let interior_path = self.elaborator.deref_subpath(self.path);

let succ = self.succ; // FIXME(#43234)
let unwind = self.unwind;
let succ = self.box_free_block(ty, succ, unwind);
let succ = self.box_free_block(adt, substs, succ, unwind);
let unwind_succ = self.unwind.map(|unwind| {
self.box_free_block(ty, unwind, Unwind::InCleanup)
self.box_free_block(adt, substs, unwind, Unwind::InCleanup)
});

self.drop_subpath(&interior, interior_path, succ, unwind_succ)
Expand Down Expand Up @@ -793,11 +794,12 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
ty::TyTuple(tys) => {
self.open_drop_for_tuple(tys)
}
ty::TyAdt(def, _) if def.is_box() => {
self.open_drop_for_box(ty.boxed_ty())
}
ty::TyAdt(def, substs) => {
self.open_drop_for_adt(def, substs)
if def.is_box() {
self.open_drop_for_box(def, substs)
} else {
self.open_drop_for_adt(def, substs)
}
}
ty::TyDynamic(..) => {
let unwind = self.unwind; // FIXME(#43234)
Expand Down Expand Up @@ -860,28 +862,34 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>

fn box_free_block<'a>(
&mut self,
ty: Ty<'tcx>,
adt: &'tcx ty::AdtDef,
substs: &'tcx Substs<'tcx>,
target: BasicBlock,
unwind: Unwind,
) -> BasicBlock {
let block = self.unelaborated_free_block(ty, target, unwind);
let block = self.unelaborated_free_block(adt, substs, target, unwind);
self.drop_flag_test_block(block, target, unwind)
}

fn unelaborated_free_block<'a>(
&mut self,
ty: Ty<'tcx>,
adt: &'tcx ty::AdtDef,
substs: &'tcx Substs<'tcx>,
target: BasicBlock,
unwind: Unwind
) -> BasicBlock {
let tcx = self.tcx();
let unit_temp = Place::Local(self.new_temp(tcx.mk_nil()));
let free_func = tcx.require_lang_item(lang_items::BoxFreeFnLangItem);
let substs = tcx.mk_substs(iter::once(Kind::from(ty)));
let args = adt.variants[0].fields.iter().enumerate().map(|(i, f)| {
let field = Field::new(i);
let field_ty = f.ty(self.tcx(), substs);
Operand::Move(self.place.clone().field(field, field_ty))
}).collect();

let call = TerminatorKind::Call {
func: Operand::function_handle(tcx, free_func, substs, self.source_info.span),
args: vec![Operand::Move(self.place.clone())],
args: args,
destination: Some((unit_temp, target)),
cleanup: None
}; // FIXME(#43234)
Expand Down

0 comments on commit 71d3dac

Please sign in to comment.