move "ADT master drop flag" logic to `open_drop_for_adt_contents`

Fixes #41888.

3 files changed, 351 insertions, 133 deletions

diff --git a/src/librustc_mir/util/elaborate_drops.rs b/src/librustc_mir/util/elaborate_drops.rs
index afec04dafe3..3ec27db60c2 100644
--- a/src/librustc_mir/util/elaborate_drops.rs
+++ b/src/librustc_mir/util/elaborate_drops.rs
@@ -243,30 +243,37 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
     }
 
     /// Create one-half of the drop ladder for a list of fields, and return
-    /// the list of steps in it in reverse order.
+    /// the list of steps in it in reverse order, with the first step
+    /// dropping 0 fields and so on.
     ///
     /// `unwind_ladder` is such a list of steps in reverse order,
     /// which is called if the matching step of the drop glue panics.
     fn drop_halfladder(&mut self,
                        unwind_ladder: &[Unwind],
-                       succ: BasicBlock,
+                       mut succ: BasicBlock,
                        fields: &[(Lvalue<'tcx>, Option<D::Path>)])
                        -> Vec<BasicBlock>
     {
-        let goto = TerminatorKind::Goto { target: succ };
-        let mut succ = self.new_block(unwind_ladder[0], goto);
-
-        // Always clear the "master" drop flag at the bottom of the
-        // ladder. This is needed because the "master" drop flag
-        // protects the ADT's discriminant, which is invalidated
-        // after the ADT is dropped.
-        let succ_loc = Location { block: succ, statement_index: 0 };
-        self.elaborator.clear_drop_flag(succ_loc, self.path, DropFlagMode::Shallow);
-
-        fields.iter().rev().zip(unwind_ladder).map(|(&(ref lv, path), &unwind_succ)| {
-            succ = self.drop_subpath(lv, path, succ, unwind_succ);
-            succ
-        }).collect()
+        Some(succ).into_iter().chain(
+            fields.iter().rev().zip(unwind_ladder)
+                .map(|(&(ref lv, path), &unwind_succ)| {
+                    succ = self.drop_subpath(lv, path, succ, unwind_succ);
+                    succ
+                })
+        ).collect()
+    }
+
+    fn drop_ladder_bottom(&mut self) -> (BasicBlock, Unwind) {
+        // Clear the "master" drop flag at the end. This is needed
+        // because the "master" drop protects the ADT's discriminant,
+        // which is invalidated after the ADT is dropped.
+        let (succ, unwind) = (self.succ, self.unwind); // FIXME(#6393)
+        (
+            self.drop_flag_reset_block(DropFlagMode::Shallow, succ, unwind),
+            unwind.map(|unwind| {
+                self.drop_flag_reset_block(DropFlagMode::Shallow, unwind, Unwind::InCleanup)
+            })
+        )
     }
 
     /// Create a full drop ladder, consisting of 2 connected half-drop-ladders
@@ -283,8 +290,13 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
     ///     ELAB(drop location.1 [target=.c2])
     /// .c2:
     ///     ELAB(drop location.2 [target=`self.unwind`])
+    ///
+    /// NOTE: this does not clear the master drop flag, so you need
+    /// to point succ/unwind on a `drop_ladder_bottom`.
     fn drop_ladder<'a>(&mut self,
-                       fields: Vec<(Lvalue<'tcx>, Option<D::Path>)>)
+                       fields: Vec<(Lvalue<'tcx>, Option<D::Path>)>,
+                       succ: BasicBlock,
+                       unwind: Unwind)
                        -> (BasicBlock, Unwind)
     {
         debug!("drop_ladder({:?}, {:?})", self, fields);
@@ -297,20 +309,17 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
         debug!("drop_ladder - fields needing drop: {:?}", fields);
 
         let unwind_ladder = vec![Unwind::InCleanup; fields.len() + 1];
-        let unwind_ladder: Vec<_> = if let Unwind::To(target) = self.unwind {
+        let unwind_ladder: Vec<_> = if let Unwind::To(target) = unwind {
             let halfladder = self.drop_halfladder(&unwind_ladder, target, &fields);
-            Some(self.unwind).into_iter().chain(halfladder.into_iter().map(Unwind::To))
-                .collect()
+            halfladder.into_iter().map(Unwind::To).collect()
         } else {
             unwind_ladder
         };
 
-        let succ = self.succ; // FIXME(#6393)
         let normal_ladder =
             self.drop_halfladder(&unwind_ladder, succ, &fields);
 
-        (normal_ladder.last().cloned().unwrap_or(succ),
-         unwind_ladder.last().cloned().unwrap_or(self.unwind))
+        (*normal_ladder.last().unwrap(), *unwind_ladder.last().unwrap())
     }
 
     fn open_drop_for_tuple<'a>(&mut self, tys: &[Ty<'tcx>])
@@ -323,7 +332,8 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
              self.elaborator.field_subpath(self.path, Field::new(i)))
         }).collect();
 
-        self.drop_ladder(fields).0
+        let (succ, unwind) = self.drop_ladder_bottom();
+        self.drop_ladder(fields, succ, unwind).0
     }
 
     fn open_drop_for_box<'a>(&mut self, ty: Ty<'tcx>) -> BasicBlock
@@ -370,106 +380,100 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
         }
     }
 
-    fn open_drop_for_adt_contents<'a>(&mut self, adt: &'tcx ty::AdtDef,
-                                      substs: &'tcx Substs<'tcx>)
-                                      -> (BasicBlock, Unwind) {
-        match adt.variants.len() {
-            1 => {
-                let fields = self.move_paths_for_fields(
-                    self.lvalue,
-                    self.path,
-                    &adt.variants[0],
-                    substs
-                );
-                self.drop_ladder(fields)
-            }
-            _ => {
-                let succ = self.succ;
-                let unwind = self.unwind; // FIXME(#6393)
+    fn open_drop_for_adt_contents(&mut self, adt: &'tcx ty::AdtDef,
+                                  substs: &'tcx Substs<'tcx>)
+                                  -> (BasicBlock, Unwind) {
+        let (succ, unwind) = self.drop_ladder_bottom();
+        if adt.variants.len() == 1 {
+            let fields = self.move_paths_for_fields(
+                self.lvalue,
+                self.path,
+                &adt.variants[0],
+                substs
+            );
+            self.drop_ladder(fields, succ, unwind)
+        } else {
+            self.open_drop_for_multivariant(adt, substs, succ, unwind)
+        }
+    }
+
+    fn open_drop_for_multivariant(&mut self, adt: &'tcx ty::AdtDef,
+                                  substs: &'tcx Substs<'tcx>,
+                                  succ: BasicBlock,
+                                  unwind: Unwind)
+                                  -> (BasicBlock, Unwind) {
+        let mut values = Vec::with_capacity(adt.variants.len());
+        let mut normal_blocks = Vec::with_capacity(adt.variants.len());
+        let mut unwind_blocks = if unwind.is_cleanup() {
+            None
+        } else {
+            Some(Vec::with_capacity(adt.variants.len()))
+        };
+
+        let mut have_otherwise = false;
 
-                let mut values = Vec::with_capacity(adt.variants.len());
-                let mut normal_blocks = Vec::with_capacity(adt.variants.len());
-                let mut unwind_blocks = if unwind.is_cleanup() {
-                    None
-                } else {
-                    Some(Vec::with_capacity(adt.variants.len()))
-                };
-                let mut otherwise = None;
-                let mut unwind_otherwise = None;
-                for (variant_index, discr) in adt.discriminants(self.tcx()).enumerate() {
-                    let subpath = self.elaborator.downcast_subpath(
-                        self.path, variant_index);
-                    if let Some(variant_path) = subpath {
-                        let base_lv = self.lvalue.clone().elem(
-                            ProjectionElem::Downcast(adt, variant_index)
+        for (variant_index, discr) in adt.discriminants(self.tcx()).enumerate() {
+            let subpath = self.elaborator.downcast_subpath(
+                self.path, variant_index);
+            if let Some(variant_path) = subpath {
+                let base_lv = self.lvalue.clone().elem(
+                    ProjectionElem::Downcast(adt, variant_index)
                         );
-                        let fields = self.move_paths_for_fields(
-                            &base_lv,
-                            variant_path,
-                            &adt.variants[variant_index],
-                            substs);
-                        values.push(discr);
-                        if let Unwind::To(unwind) = unwind {
-                            // We can't use the half-ladder from the original
-                            // drop ladder, because this breaks the
-                            // "funclet can't have 2 successor funclets"
-                            // requirement from MSVC:
-                            //
-                            //           switch       unwind-switch
-                            //          /      \         /        \
-                            //         v1.0    v2.0  v2.0-unwind  v1.0-unwind
-                            //         |        |      /             |
-                            //    v1.1-unwind  v2.1-unwind           |
-                            //      ^                                |
-                            //       \-------------------------------/
-                            //
-                            // Create a duplicate half-ladder to avoid that. We
-                            // could technically only do this on MSVC, but I
-                            // I want to minimize the divergence between MSVC
-                            // and non-MSVC.
-
-                            let unwind_blocks = unwind_blocks.as_mut().unwrap();
-                            let unwind_ladder = vec![Unwind::InCleanup; fields.len() + 1];
-                            let halfladder =
-                                self.drop_halfladder(&unwind_ladder, unwind, &fields);
-                            unwind_blocks.push(halfladder.last().cloned().unwrap_or(unwind));
-                        }
-                        let (normal, _) = self.drop_ladder(fields);
-                        normal_blocks.push(normal);
-                    } else {
-                        // variant not found - drop the entire enum
-                        if let None = otherwise {
-                            otherwise = Some(self.complete_drop(
-                                Some(DropFlagMode::Shallow),
-                                succ,
-                                unwind));
-                            if let Unwind::To(unwind) = unwind {
-                                unwind_otherwise = Some(self.complete_drop(
-                                    Some(DropFlagMode::Shallow),
-                                    unwind,
-                                    Unwind::InCleanup
-                                ));
-                            }
-                        }
-                    }
-                }
-                if let Some(block) = otherwise {
-                    normal_blocks.push(block);
-                    if let Some(ref mut unwind_blocks) = unwind_blocks {
-                        unwind_blocks.push(unwind_otherwise.unwrap());
-                    }
-                } else {
-                    values.pop();
+                let fields = self.move_paths_for_fields(
+                    &base_lv,
+                    variant_path,
+                    &adt.variants[variant_index],
+                    substs);
+                values.push(discr);
+                if let Unwind::To(unwind) = unwind {
+                    // We can't use the half-ladder from the original
+                    // drop ladder, because this breaks the
+                    // "funclet can't have 2 successor funclets"
+                    // requirement from MSVC:
+                    //
+                    //           switch       unwind-switch
+                    //          /      \         /        \
+                    //         v1.0    v2.0  v2.0-unwind  v1.0-unwind
+                    //         |        |      /             |
+                    //    v1.1-unwind  v2.1-unwind           |
+                    //      ^                                |
+                    //       \-------------------------------/
+                    //
+                    // Create a duplicate half-ladder to avoid that. We
+                    // could technically only do this on MSVC, but I
+                    // I want to minimize the divergence between MSVC
+                    // and non-MSVC.
+
+                    let unwind_blocks = unwind_blocks.as_mut().unwrap();
+                    let unwind_ladder = vec![Unwind::InCleanup; fields.len() + 1];
+                    let halfladder =
+                        self.drop_halfladder(&unwind_ladder, unwind, &fields);
+                    unwind_blocks.push(halfladder.last().cloned().unwrap());
                 }
+                let (normal, _) = self.drop_ladder(fields, succ, unwind);
+                normal_blocks.push(normal);
+            } else {
+                have_otherwise = true;
+            }
+        }
 
-                (self.adt_switch_block(adt, normal_blocks, &values, succ, unwind),
-                 unwind.map(|unwind| {
-                     self.adt_switch_block(
-                         adt, unwind_blocks.unwrap(), &values, unwind, Unwind::InCleanup
-                     )
-                 }))
+        if have_otherwise {
+            normal_blocks.push(self.drop_block(succ, unwind));
+            if let Unwind::To(unwind) = unwind {
+                unwind_blocks.as_mut().unwrap().push(
+                    self.drop_block(unwind, Unwind::InCleanup)
+                        );
             }
+        } else {
+            values.pop();
         }
+
+        (self.adt_switch_block(adt, normal_blocks, &values, succ, unwind),
+         unwind.map(|unwind| {
+             self.adt_switch_block(
+                 adt, unwind_blocks.unwrap(), &values, unwind, Unwind::InCleanup
+             )
+         }))
     }
 
     fn adt_switch_block(&mut self,
@@ -652,8 +656,8 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
         });
 
         // FIXME(#34708): handle partially-dropped array/slice elements.
-        self.drop_flag_test_and_reset_block(
-            Some(DropFlagMode::Deep), drop_block, succ, unwind)
+        let reset_block = self.drop_flag_reset_block(DropFlagMode::Deep, drop_block, unwind);
+        self.drop_flag_test_block(reset_block, succ, unwind)
     }
 
     /// The slow-path - create an "open", elaborated drop for a type
@@ -707,23 +711,26 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D>
         debug!("complete_drop({:?},{:?})", self, drop_mode);
 
         let drop_block = self.drop_block(succ, unwind);
-        self.drop_flag_test_and_reset_block(drop_mode, drop_block, succ, unwind)
+        let drop_block = if let Some(mode) = drop_mode {
+            self.drop_flag_reset_block(mode, drop_block, unwind)
+        } else {
+            drop_block
+        };
+
+        self.drop_flag_test_block(drop_block, succ, unwind)
     }
 
-    fn drop_flag_test_and_reset_block(&mut self,
-                                      drop_mode: Option<DropFlagMode>,
-                                      drop_block: BasicBlock,
-                                      succ: BasicBlock,
-                                      unwind: Unwind) -> BasicBlock
+    fn drop_flag_reset_block(&mut self,
+                             mode: DropFlagMode,
+                             succ: BasicBlock,
+                             unwind: Unwind) -> BasicBlock
     {
-        debug!("drop_flag_test_and_reset_block({:?},{:?})", self, drop_mode);
-
-        if let Some(mode) = drop_mode {
-            let block_start = Location { block: drop_block, statement_index: 0 };
-            self.elaborator.clear_drop_flag(block_start, self.path, mode);
-        }
+        debug!("drop_flag_reset_block({:?},{:?})", self, mode);
 
-        self.drop_flag_test_block(drop_block, succ, unwind)
+        let block = self.new_block(unwind, TerminatorKind::Goto { target: succ });
+        let block_start = Location { block: block, statement_index: 0 };
+        self.elaborator.clear_drop_flag(block_start, self.path, mode);
+        block
     }
 
     fn elaborated_drop_block<'a>(&mut self) -> BasicBlock {
diff --git a/src/test/mir-opt/issue-41888.rs b/src/test/mir-opt/issue-41888.rs
new file mode 100644
index 00000000000..ea4d7d3165d
--- /dev/null
+++ b/src/test/mir-opt/issue-41888.rs
@@ -0,0 +1,186 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// check that we clear the "ADT master drop flag" even when there are
+// no fields to be dropped.
+
+fn main() {
+    let e;
+    if cond() {
+        e = E::F(K);
+        if let E::F(_k) = e {
+            // older versions of rustc used to not clear the
+            // drop flag for `e` in this path.
+        }
+    }
+}
+
+fn cond() -> bool { false }
+
+struct K;
+
+enum E {
+    F(K),
+    G(Box<E>)
+}
+
+// END RUST SOURCE
+// fn main() -> () {
+//     let mut _0: ();
+//     scope 1 {
+//         let _1: E; // `e`
+//         scope 2 {
+//             let _6: K;
+//         }
+//     }
+//     let mut _2: bool;
+//     let mut _3: ();
+//     let mut _4: E;
+//     let mut _5: K;
+//     let mut _7: isize;
+//     let mut _8: bool; // drop flag for `e`
+//     let mut _9: bool;
+//     let mut _10: bool;
+//     let mut _11: isize;
+//     let mut _12: isize;
+//
+//     bb0: {
+//         _8 = const false;
+//         _10 = const false;
+//         _9 = const false;
+//         StorageLive(_1);
+//         StorageLive(_2);
+//         _2 = const cond() -> [return: bb3, unwind: bb2];
+//     }
+//
+//     bb1: {
+//         resume;
+//     }
+//
+//     bb2: {
+//         goto -> bb1;
+//     }
+//
+//     bb3: {
+//         switchInt(_2) -> [0u8: bb5, otherwise: bb4];
+//     }
+//
+//     bb4: {
+//         StorageLive(_4);
+//         StorageLive(_5);
+//         _5 = K::{{constructor}};
+//         _4 = E::F(_5,);
+//         StorageDead(_5);
+//         goto -> bb15;
+//     }
+//
+//     bb5: {
+//         _0 = ();
+//         goto -> bb12;
+//     }
+//
+//     bb6: {
+//         goto -> bb2;
+//     }
+//
+//     bb7: {
+//         goto -> bb8;
+//     }
+//
+//     bb8: {
+//         StorageDead(_4);
+//         _7 = discriminant(_1);
+//         switchInt(_7) -> [0isize: bb10, otherwise: bb9];
+//     }
+//
+//     bb9: {
+//         _0 = ();
+//         goto -> bb11;
+//     }
+//
+//     bb10: {
+//         StorageLive(_6);
+//         _10 = const false;
+//         _6 = ((_1 as F).0: K);
+//         _0 = ();
+//         goto -> bb11;
+//     }
+//
+//     bb11: {
+//         StorageDead(_6);
+//         goto -> bb12;
+//     }
+//
+//     bb12: {
+//         StorageDead(_2);
+//         goto -> bb22;
+//     }
+//
+//     bb13: {
+//         StorageDead(_1);
+//         return;
+//     }
+//
+//     bb14: {
+//         _8 = const true;
+//         _9 = const true;
+//         _10 = const true;
+//         _1 = _4;
+//         goto -> bb6;
+//     }
+//
+//     bb15: {
+//         _8 = const true;
+//         _9 = const true;
+//         _10 = const true;
+//         _1 = _4;
+//         goto -> bb7;
+//     }
+//
+//     bb16: {
+//         _8 = const false; // clear the drop flag - must always be reached
+//         goto -> bb13;
+//     }
+//
+//     bb17: {
+//         _8 = const false;
+//         goto -> bb1;
+//     }
+//
+//     bb18: {
+//         goto -> bb17;
+//     }
+//
+//     bb19: {
+//         drop(_1) -> [return: bb16, unwind: bb17];
+//     }
+//
+//     bb20: {
+//         drop(_1) -> bb17;
+//     }
+//
+//     bb21: {
+//         _11 = discriminant(_1);
+//         switchInt(_11) -> [0isize: bb16, otherwise: bb19];
+//     }
+//
+//     bb22: {
+//         switchInt(_8) -> [0u8: bb16, otherwise: bb21];
+//     }
+//
+//     bb23: {
+//         _12 = discriminant(_1);
+//         switchInt(_12) -> [0isize: bb18, otherwise: bb20];
+//     }
+//
+//     bb24: {
+//         switchInt(_8) -> [0u8: bb17, otherwise: bb23];
+//     }
+// }
diff --git a/src/test/run-pass/dynamic-drop.rs b/src/test/run-pass/dynamic-drop.rs
index 26e5fe987ee..6725a0c547f 100644
--- a/src/test/run-pass/dynamic-drop.rs
+++ b/src/test/run-pass/dynamic-drop.rs
@@ -90,6 +90,22 @@ fn dynamic_drop(a: &Allocator, c: bool) {
     };
 }
 
+struct TwoPtrs<'a>(Ptr<'a>, Ptr<'a>);
+fn struct_dynamic_drop(a: &Allocator, c0: bool, c1: bool, c: bool) {
+    for i in 0..2 {
+        let x;
+        let y;
+        if (c0 && i == 0) || (c1 && i == 1) {
+            x = (a.alloc(), a.alloc(), a.alloc());
+            y = TwoPtrs(a.alloc(), a.alloc());
+            if c {
+                drop(x.1);
+                drop(y.0);
+            }
+        }
+    }
+}
+
 fn assignment2(a: &Allocator, c0: bool, c1: bool) {
     let mut _v = a.alloc();
     let mut _w = a.alloc();
@@ -182,5 +198,14 @@ fn main() {
     run_test(|a| array_simple(a));
     run_test(|a| vec_simple(a));
 
+    run_test(|a| struct_dynamic_drop(a, false, false, false));
+    run_test(|a| struct_dynamic_drop(a, false, false, true));
+    run_test(|a| struct_dynamic_drop(a, false, true, false));
+    run_test(|a| struct_dynamic_drop(a, false, true, true));
+    run_test(|a| struct_dynamic_drop(a, true, false, false));
+    run_test(|a| struct_dynamic_drop(a, true, false, true));
+    run_test(|a| struct_dynamic_drop(a, true, true, false));
+    run_test(|a| struct_dynamic_drop(a, true, true, true));
+
     run_test_nopanic(|a| union1(a));
 }