Skip to content

Commit

Permalink
Fix SwitchInt building in ElaborateDrops pass
Browse files Browse the repository at this point in the history
Previously it used to build a switch in a way that didn’t preserve the invariat of SwitchInt. Now
it builds it in an optimal way too, where otherwise branch becomes all the branches which did not
have partial variant drops.
  • Loading branch information
nagisa committed Feb 10, 2017
1 parent c993986 commit 4be1848
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 12 deletions.
15 changes: 11 additions & 4 deletions src/librustc/mir/mod.rs
Expand Up @@ -469,10 +469,17 @@ pub enum TerminatorKind<'tcx> {
/// are found in the corresponding indices from the `targets` vector.
values: Cow<'tcx, [ConstInt]>,

/// Possible branch sites. The length of this vector should be
/// equal to the length of the `values` vector plus 1 -- the
/// extra item is the block to branch to if none of the values
/// fit.
/// Possible branch sites. The last element of this vector is used
/// for the otherwise branch, so values.len() == targets.len() + 1
/// should hold.
// This invariant is quite non-obvious and also could be improved.
// One way to make this invariant is to have something like this instead:
//
// branches: Vec<(ConstInt, BasicBlock)>,
// otherwise: Option<BasicBlock> // exhaustive if None
//
// However we’ve decided to keep this as-is until we figure a case
// where some other approach seems to be strictly better than other.
targets: Vec<BasicBlock>,
},

Expand Down
7 changes: 5 additions & 2 deletions src/librustc/mir/tcx.rs
Expand Up @@ -171,10 +171,13 @@ impl<'tcx> Rvalue<'tcx> {
Some(operand.ty(mir, tcx))
}
Rvalue::Discriminant(ref lval) => {
if let ty::TyAdt(adt_def, _) = lval.ty(mir, tcx).to_ty(tcx).sty {
let ty = lval.ty(mir, tcx).to_ty(tcx);
if let ty::TyAdt(adt_def, _) = ty.sty {
Some(adt_def.discr_ty.to_ty(tcx))
} else {
None
// Undefined behaviour, bug for now; may want to return something for
// the `discriminant` intrinsic later.
bug!("Rvalue::Discriminant on Lvalue of type {:?}", ty);
}
}
Rvalue::Box(t) => {
Expand Down
24 changes: 18 additions & 6 deletions src/librustc_borrowck/borrowck/mir/elaborate_drops.rs
Expand Up @@ -626,7 +626,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
adt: &'tcx ty::AdtDef,
substs: &'tcx Substs<'tcx>,
variant_index: usize)
-> BasicBlock
-> (BasicBlock, bool)
{
let subpath = super::move_path_children_matching(
self.move_data(), c.path, |proj| match proj {
Expand All @@ -645,13 +645,13 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
variant_path,
&adt.variants[variant_index],
substs);
self.drop_ladder(c, fields)
(self.drop_ladder(c, fields), true)
} else {
// variant not found - drop the entire enum
if let None = *drop_block {
*drop_block = Some(self.complete_drop(c, true));
}
return drop_block.unwrap();
(drop_block.unwrap(), false)
}
}

Expand All @@ -674,13 +674,25 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
}
_ => {
let mut values = Vec::with_capacity(adt.variants.len());
let mut blocks = Vec::with_capacity(adt.variants.len() + 1);
let mut blocks = Vec::with_capacity(adt.variants.len());
let mut otherwise = None;
for (idx, variant) in adt.variants.iter().enumerate() {
let discr = ConstInt::new_inttype(variant.disr_val, adt.discr_ty,
self.tcx.sess.target.uint_type,
self.tcx.sess.target.int_type).unwrap();
values.push(discr);
blocks.push(self.open_drop_for_variant(c, &mut drop_block, adt, substs, idx));
let (blk, is_ladder) = self.open_drop_for_variant(c, &mut drop_block, adt,
substs, idx);
if is_ladder {
values.push(discr);
blocks.push(blk);
} else {
otherwise = Some(blk)
}
}
if let Some(block) = otherwise {
blocks.push(block);
} else {
values.pop();
}
// If there are multiple variants, then if something
// is present within the enum the discriminant, tracked
Expand Down

0 comments on commit 4be1848

Please sign in to comment.