Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial future-proofing for Box<T, A> #50097

Merged
merged 5 commits into from
Apr 27, 2018
Merged

Partial future-proofing for Box<T, A> #50097

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
43b24c6
Pass the right type to box_free() in MIR
glandium Apr 18, 2018
dfa6111
Remove the explicit box_free type check
glandium Apr 19, 2018
6614fa0
Support an alternative form for box_free
glandium Apr 19, 2018
64f5233
Adapt the owned_box lang item to allow a Box type with defaulted para…
glandium Apr 19, 2018
bd8c177
Switch box_free to take the destructured contents of Box
glandium Apr 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/librustc/ty/context.rs
View file
Open in desktop
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 @@ -2319,7 +2319,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
View file
Open in desktop
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
View file
Open in desktop
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
83 changes: 64 additions & 19 deletions src/librustc_mir/util/elaborate_drops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,18 +337,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 @@ -791,11 +792,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 @@ -858,32 +860,75 @@ 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 free_sig = tcx.fn_sig(free_func).skip_binder().clone();
let free_inputs = free_sig.inputs();
// If the box_free function takes a *mut T, transform the Box into
// such a pointer before calling box_free. Otherwise, pass it all
// the fields in the Box as individual arguments.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think supporting the *mut T form adds unnecessary clutter to the compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only temporary clutter, and really makes the transition easier. For example, this could land now in 1.27, and the liballoc changes could be done in 1.28 without having a cfg(stage0)-specific Box implementation. /That/ would be clutter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[cfg(stage0)] box_free is tiny compared to this, since all it needs to do is call the real implementation, which it can do in much less code. (See my patch for more details)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other problem is that a #[cfg(stage0)] box_free with the old signature can't be compiled by the new code, which means if, say, version 1.27 has the new code, you can't bootstrap 1.27.1 (if there ends up being one) with 1.27, which, typically, linux distros would be doing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehm, we don't support that - or rather, you set a flag in config.toml that turns off stage0. cc @alexcrichton

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(we pretty much always use #[cfg(stage0)] like this)

let (stmts, args) = if free_inputs.len() == 1 && free_inputs[0].is_mutable_pointer() {
let ty = substs.type_at(0);
let ref_ty = tcx.mk_ref(tcx.types.re_erased, ty::TypeAndMut {
ty: ty,
mutbl: hir::Mutability::MutMutable
});
let ptr_ty = tcx.mk_mut_ptr(ty);
let ref_tmp = Place::Local(self.new_temp(ref_ty));
let ptr_tmp = Place::Local(self.new_temp(ptr_ty));
let stmts = vec![
self.assign(&ref_tmp, Rvalue::Ref(
tcx.types.re_erased,
BorrowKind::Mut { allow_two_phase_borrow: false },
self.place.clone().deref()
)),
self.assign(&ptr_tmp, Rvalue::Cast(
CastKind::Misc,
Operand::Move(ref_tmp),
ptr_ty,
)),
];
(stmts, vec![Operand::Move(ptr_tmp)])
} else {
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();
(vec![], args)
};

let call = TerminatorKind::Call {
func: Operand::function_handle(tcx, free_func, substs, self.source_info.span),
args: vec![Operand::Move(self.place.clone())],
destination: Some((unit_temp, target)),
cleanup: None
}; // FIXME(#43234)
let free_block = self.new_block(unwind, call);
let free_block = BasicBlockData {
statements: stmts,
terminator: Some(Terminator {
kind: TerminatorKind::Call {
func: Operand::function_handle(tcx, free_func, substs, self.source_info.span),
args: args,
destination: Some((unit_temp, target)),
cleanup: None
}, // FIXME(#43234)
source_info: self.source_info,
}),
is_cleanup: unwind.is_cleanup()
};
let free_block = self.elaborator.patch().new_block(free_block);

let block_start = Location { block: free_block, statement_index: 0 };
self.elaborator.clear_drop_flag(block_start, self.path, DropFlagMode::Shallow);
Expand Down