From 4be18488a741b2bf9b6f32c0ae5b21f4c3f6c83e Mon Sep 17 00:00:00 2001
From: Simonas Kazlauskas <git@kazlauskas.me>
Date: Thu, 2 Feb 2017 20:35:54 +0200
Subject: [PATCH] Fix SwitchInt building in ElaborateDrops pass
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 src/librustc/mir/mod.rs                       | 15 ++++++++----
 src/librustc/mir/tcx.rs                       |  7 ++++--
 .../borrowck/mir/elaborate_drops.rs           | 24 ++++++++++++++-----
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index 28990fd323f..5657ec157e8 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -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>,
     },
 
diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs
index 6863468ec0d..7b0863b4c42 100644
--- a/src/librustc/mir/tcx.rs
+++ b/src/librustc/mir/tcx.rs
@@ -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) => {
diff --git a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs
index 7521b750d5a..144b0c2203a 100644
--- a/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs
+++ b/src/librustc_borrowck/borrowck/mir/elaborate_drops.rs
@@ -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 {
@@ -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)
         }
     }
 
@@ -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