Skip to content

Commit ff167a6

Browse files
committed
Fix SwitchInt building in ElaborateDrops pass
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.
1 parent cd41479 commit ff167a6

File tree

3 files changed

+34
-12
lines changed

3 files changed

+34
-12
lines changed

src/librustc/mir/mod.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -469,10 +469,17 @@ pub enum TerminatorKind<'tcx> {
469469
/// are found in the corresponding indices from the `targets` vector.
470470
values: Cow<'tcx, [ConstInt]>,
471471

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

src/librustc/mir/tcx.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,13 @@ impl<'tcx> Rvalue<'tcx> {
171171
Some(operand.ty(mir, tcx))
172172
}
173173
Rvalue::Discriminant(ref lval) => {
174-
if let ty::TyAdt(adt_def, _) = lval.ty(mir, tcx).to_ty(tcx).sty {
174+
let ty = lval.ty(mir, tcx).to_ty(tcx);
175+
if let ty::TyAdt(adt_def, _) = ty.sty {
175176
Some(adt_def.discr_ty.to_ty(tcx))
176177
} else {
177-
None
178+
// Undefined behaviour, bug for now; may want to return something for
179+
// the `discriminant` intrinsic later.
180+
bug!("Rvalue::Discriminant on Lvalue of type {:?}", ty);
178181
}
179182
}
180183
Rvalue::Box(t) => {

src/librustc_borrowck/borrowck/mir/elaborate_drops.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
626626
adt: &'tcx ty::AdtDef,
627627
substs: &'tcx Substs<'tcx>,
628628
variant_index: usize)
629-
-> BasicBlock
629+
-> (BasicBlock, bool)
630630
{
631631
let subpath = super::move_path_children_matching(
632632
self.move_data(), c.path, |proj| match proj {
@@ -645,13 +645,13 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
645645
variant_path,
646646
&adt.variants[variant_index],
647647
substs);
648-
self.drop_ladder(c, fields)
648+
(self.drop_ladder(c, fields), true)
649649
} else {
650650
// variant not found - drop the entire enum
651651
if let None = *drop_block {
652652
*drop_block = Some(self.complete_drop(c, true));
653653
}
654-
return drop_block.unwrap();
654+
(drop_block.unwrap(), false)
655655
}
656656
}
657657

@@ -674,13 +674,25 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
674674
}
675675
_ => {
676676
let mut values = Vec::with_capacity(adt.variants.len());
677-
let mut blocks = Vec::with_capacity(adt.variants.len() + 1);
677+
let mut blocks = Vec::with_capacity(adt.variants.len());
678+
let mut otherwise = None;
678679
for (idx, variant) in adt.variants.iter().enumerate() {
679680
let discr = ConstInt::new_inttype(variant.disr_val, adt.discr_ty,
680681
self.tcx.sess.target.uint_type,
681682
self.tcx.sess.target.int_type).unwrap();
682-
values.push(discr);
683-
blocks.push(self.open_drop_for_variant(c, &mut drop_block, adt, substs, idx));
683+
let (blk, is_ladder) = self.open_drop_for_variant(c, &mut drop_block, adt,
684+
substs, idx);
685+
if is_ladder {
686+
values.push(discr);
687+
blocks.push(blk);
688+
} else {
689+
otherwise = Some(blk)
690+
}
691+
}
692+
if let Some(block) = otherwise {
693+
blocks.push(block);
694+
} else {
695+
values.pop();
684696
}
685697
// If there are multiple variants, then if something
686698
// is present within the enum the discriminant, tracked

0 commit comments

Comments
 (0)