Auto merge of #44129 - pnkfelix:fix-end-region-emission-order, r=nmatsakis

Fix end region emission order

Fix #43481

14 files changed, 319 insertions, 23 deletions

diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs
index c08620c1e41..bac884b4d01 100644
--- a/src/librustc_mir/build/scope.rs
+++ b/src/librustc_mir/build/scope.rs
@@ -352,6 +352,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
         }
         let scope = self.scopes.pop().unwrap();
         assert_eq!(scope.region_scope, region_scope.0);
+
+        self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope);
         unpack!(block = build_scope_drops(&mut self.cfg,
                                           &scope,
                                           &self.scopes,
@@ -359,7 +361,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                                           self.arg_count,
                                           false));
 
-        self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope);
         block.unit()
     }
 
@@ -406,15 +407,16 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 scope.cached_exits.insert((target, region_scope.0), b);
                 b
             };
+
+            // End all regions for scopes out of which we are breaking.
+            self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope);
+
             unpack!(block = build_scope_drops(&mut self.cfg,
                                               scope,
                                               rest,
                                               block,
                                               self.arg_count,
                                               false));
-
-            // End all regions for scopes out of which we are breaking.
-            self.cfg.push_end_region(self.hir.tcx(), block, region_scope.1, scope.region_scope);
         }
         }
         let scope = &self.scopes[len - scope_count];
diff --git a/src/test/mir-opt/end_region_1.rs b/src/test/mir-opt/end_region_1.rs
index 9c8cfc0ab20..1941d1bc7be 100644
--- a/src/test/mir-opt/end_region_1.rs
+++ b/src/test/mir-opt/end_region_1.rs
@@ -30,8 +30,8 @@ fn main() {
 //         StorageLive(_2);
 //         _2 = &'10_1rs _1;
 //         _0 = ();
-//         StorageDead(_2);
 //         EndRegion('10_1rs);
+//         StorageDead(_2);
 //         StorageDead(_1);
 //         return;
 //     }
diff --git a/src/test/mir-opt/end_region_2.rs b/src/test/mir-opt/end_region_2.rs
index 6a4e209a14b..d8dd4aeadf4 100644
--- a/src/test/mir-opt/end_region_2.rs
+++ b/src/test/mir-opt/end_region_2.rs
@@ -46,8 +46,8 @@ fn main() {
 //     bb2: {
 //         _0 = ();
 //         StorageDead(_5);
-//         StorageDead(_3);
 //         EndRegion('23_1rs);
+//         StorageDead(_3);
 //         StorageDead(_2);
 //         return;
 //     }
@@ -56,10 +56,10 @@ fn main() {
 //         StorageLive(_7);
 //         _7 = &'23_3rs _2;
 //         _1 = ();
-//         StorageDead(_7);
 //         EndRegion('23_3rs);
-//         StorageDead(_3);
+//         StorageDead(_7);
 //         EndRegion('23_1rs);
+//         StorageDead(_3);
 //         StorageDead(_2);
 //         goto -> bb1;
 //     }
diff --git a/src/test/mir-opt/end_region_3.rs b/src/test/mir-opt/end_region_3.rs
index cb1d45e37c8..e404af838ce 100644
--- a/src/test/mir-opt/end_region_3.rs
+++ b/src/test/mir-opt/end_region_3.rs
@@ -49,8 +49,8 @@ fn main() {
 //     bb2: {
 //         _0 = ();
 //         StorageDead(_5);
-//         StorageDead(_3);
 //         EndRegion('26_1rs);
+//         StorageDead(_3);
 //         StorageDead(_1);
 //         return;
 //     }
@@ -60,10 +60,10 @@ fn main() {
 //         StorageLive(_7);
 //         _7 = &'26_3rs _1;
 //         _2 = ();
-//         StorageDead(_7);
 //         EndRegion('26_3rs);
-//         StorageDead(_3);
+//         StorageDead(_7);
 //         EndRegion('26_1rs);
+//         StorageDead(_3);
 //         goto -> bb1;
 //     }
 // END rustc.node4.SimplifyCfg-qualify-consts.after.mir
diff --git a/src/test/mir-opt/end_region_4.rs b/src/test/mir-opt/end_region_4.rs
index b714b6bc55b..d51c627d14b 100644
--- a/src/test/mir-opt/end_region_4.rs
+++ b/src/test/mir-opt/end_region_4.rs
@@ -53,10 +53,10 @@ fn foo(i: i32) {
 //         StorageLive(_6);
 //         _6 = &'26_4rs _2;
 //         _0 = ();
-//         StorageDead(_6);
 //         EndRegion('26_4rs);
-//         StorageDead(_3);
+//         StorageDead(_6);
 //         EndRegion('26_2rs);
+//         StorageDead(_3);
 //         StorageDead(_2);
 //         drop(_1) -> bb4;
 //     }
diff --git a/src/test/mir-opt/end_region_5.rs b/src/test/mir-opt/end_region_5.rs
index a4c53675b9c..6299ec3815c 100644
--- a/src/test/mir-opt/end_region_5.rs
+++ b/src/test/mir-opt/end_region_5.rs
@@ -44,8 +44,8 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 //         _2 = const foo(_3) -> [return: bb1, unwind: bb3];
 //     }
 //     bb1: {
-//         StorageDead(_3);
 //         EndRegion('14s);
+//         StorageDead(_3);
 //         _0 = ();
 //         drop(_1) -> bb4;
 //     }
diff --git a/src/test/mir-opt/end_region_6.rs b/src/test/mir-opt/end_region_6.rs
index daa94769430..13ab3e4f2dd 100644
--- a/src/test/mir-opt/end_region_6.rs
+++ b/src/test/mir-opt/end_region_6.rs
@@ -44,8 +44,8 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 //         _2 = const foo(_3) -> [return: bb1, unwind: bb3];
 //     }
 //     bb1: {
-//         StorageDead(_3);
 //         EndRegion('19s);
+//         StorageDead(_3);
 //         _0 = ();
 //         drop(_1) -> bb4;
 //     }
@@ -75,8 +75,8 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 //         _3 = ((*_2).0: i32);
 //         _0 = _3;
 //         StorageDead(_3);
-//         StorageDead(_2);
 //         EndRegion('15_0rs);
+//         StorageDead(_2);
 //         return;
 //     }
 // END rustc.node22.SimplifyCfg-qualify-consts.after.mir
diff --git a/src/test/mir-opt/end_region_7.rs b/src/test/mir-opt/end_region_7.rs
index 9feacd04fe0..826d3749167 100644
--- a/src/test/mir-opt/end_region_7.rs
+++ b/src/test/mir-opt/end_region_7.rs
@@ -84,8 +84,8 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 //         _3 = ((*_2).0: i32);
 //         _0 = _3;
 //         StorageDead(_3);
-//         StorageDead(_2);
 //         EndRegion('15_0rs);
+//         StorageDead(_2);
 //         drop(_1) -> bb1;
 //     }
 //     bb1: {
diff --git a/src/test/mir-opt/end_region_8.rs b/src/test/mir-opt/end_region_8.rs
index 30dda7a4062..6438484fcfa 100644
--- a/src/test/mir-opt/end_region_8.rs
+++ b/src/test/mir-opt/end_region_8.rs
@@ -50,8 +50,8 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
 //    bb1: {
 //        StorageDead(_4);
 //        _0 = ();
-//        StorageDead(_2);
 //        EndRegion('21_1rs);
+//        StorageDead(_2);
 //        drop(_1) -> bb4;
 //    }
 //    bb2: {
diff --git a/src/test/mir-opt/end_region_9.rs b/src/test/mir-opt/end_region_9.rs
index bbfdc0e77a8..59d5d934391 100644
--- a/src/test/mir-opt/end_region_9.rs
+++ b/src/test/mir-opt/end_region_9.rs
@@ -42,7 +42,7 @@ fn main() {
 //     let mut _0: ();
 //     let mut _1: bool;
 //     let _2: i32;
-//     let mut _4: &'13_0rs i32;
+//     let mut _4: &'33_0rs i32;
 //     let mut _3: ();
 //     let mut _5: !;
 //     let mut _6: ();
@@ -67,15 +67,15 @@ fn main() {
 //    bb2: {
 //        _0 = ();
 //        StorageDead(_7);
+//        EndRegion('33_0rs);
 //        StorageDead(_4);
-//        EndRegion('13_0rs);
 //        StorageDead(_2);
 //        StorageDead(_1);
 //        return;
 //    }
 //
 //    bb3: {
-//        _4 = &'13_0rs _2;
+//        _4 = &'33_0rs _2;
 //        _6 = ();
 //        StorageDead(_7);
 //        _1 = const true;
@@ -83,3 +83,4 @@ fn main() {
 //        goto -> bb1;
 //    }
 // }
+// END rustc.node4.SimplifyCfg-qualify-consts.after.mir
diff --git a/src/test/mir-opt/end_region_cyclic.rs b/src/test/mir-opt/end_region_cyclic.rs
new file mode 100644
index 00000000000..8f9dd79cd75
--- /dev/null
+++ b/src/test/mir-opt/end_region_cyclic.rs
@@ -0,0 +1,132 @@
+// 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.
+
+// compile-flags: -Z identify_regions -Z span_free_formats -Z emit-end-regions
+// ignore-tidy-linelength
+
+// This test models a scenario with a cyclic reference. Rust obviously
+// needs to handle such cases.
+//
+// The interesting part about this test is that such case shows that
+// one cannot generally force all references to be dead before you hit
+// their EndRegion; at least, not without breaking the more important
+// property that all borrowed storage locations have their regions
+// ended strictly before their StorageDeads. (This test was inspired
+// by discussion on Issue #43481.)
+
+use std::cell::Cell;
+
+struct S<'a> {
+    r: Cell<Option<&'a S<'a>>>,
+}
+
+fn main() {
+    loop {
+        let x = S { r: Cell::new(None) };
+        x.r.set(Some(&x));
+        if query() { break; }
+        x.r.set(Some(&x));
+    }
+}
+
+fn query() -> bool { true }
+
+// END RUST SOURCE
+// START rustc.node16.SimplifyCfg-qualify-consts.after.mir
+// fn main() -> () {
+//     let mut _0: ();
+//     scope 1 {
+//         let _2: S<'35_0rs>;
+//     }
+//     let mut _1: ();
+//     let mut _3: std::cell::Cell<std::option::Option<&'35_0rs S<'35_0rs>>>;
+//     let mut _4: std::option::Option<&'35_0rs S<'35_0rs>>;
+//     let mut _5: ();
+//     let mut _6: &'16s std::cell::Cell<std::option::Option<&'35_0rs S<'35_0rs>>>;
+//     let mut _7: std::option::Option<&'35_0rs S<'35_0rs>>;
+//     let mut _8: &'35_0rs S<'35_0rs>;
+//     let mut _9: &'35_0rs S<'35_0rs>;
+//     let mut _10: ();
+//     let mut _11: bool;
+//     let mut _12: !;
+//     let mut _13: ();
+//     let mut _14: &'33s std::cell::Cell<std::option::Option<&'35_0rs S<'35_0rs>>>;
+//     let mut _15: std::option::Option<&'35_0rs S<'35_0rs>>;
+//     let mut _16: &'35_0rs S<'35_0rs>;
+//     let mut _17: &'35_0rs S<'35_0rs>;
+//     bb0: {
+//         goto -> bb1;
+//     }
+//     bb1: {
+//         StorageLive(_2);
+//         StorageLive(_3);
+//         StorageLive(_4);
+//         _4 = std::option::Option<&'35_0rs S<'35_0rs>>::None;
+//         _3 = const <std::cell::Cell<T>>::new(_4) -> bb2;
+//     }
+//     bb2: {
+//         StorageDead(_4);
+//         _2 = S<'35_0rs> { r: _3 };
+//         StorageDead(_3);
+//         StorageLive(_6);
+//         _6 = &'16s (_2.0: std::cell::Cell<std::option::Option<&'35_0rs S<'35_0rs>>>);
+//         StorageLive(_7);
+//         StorageLive(_8);
+//         StorageLive(_9);
+//         _9 = &'35_0rs _2;
+//         _8 = &'35_0rs (*_9);
+//         _7 = std::option::Option<&'35_0rs S<'35_0rs>>::Some(_8,);
+//         StorageDead(_8);
+//         _5 = const <std::cell::Cell<T>>::set(_6, _7) -> bb3;
+//     }
+//     bb3: {
+//         EndRegion('16s);
+//         StorageDead(_7);
+//         StorageDead(_6);
+//         StorageDead(_9);
+//         StorageLive(_11);
+//         _11 = const query() -> bb4;
+//     }
+//     bb4: {
+//         switchInt(_11) -> [0u8: bb6, otherwise: bb5];
+//     }
+//     bb5: {
+//         _0 = ();
+//         StorageDead(_11);
+//         EndRegion('35_0rs);
+//         StorageDead(_2);
+//         return;
+//     }
+//     bb6: {
+//         _10 = ();
+//         StorageDead(_11);
+//         StorageLive(_14);
+//         _14 = &'33s (_2.0: std::cell::Cell<std::option::Option<&'35_0rs S<'35_0rs>>>);
+//         StorageLive(_15);
+//         StorageLive(_16);
+//         StorageLive(_17);
+//         _17 = &'35_0rs _2;
+//         _16 = &'35_0rs (*_17);
+//         _15 = std::option::Option<&'35_0rs S<'35_0rs>>::Some(_16,);
+//         StorageDead(_16);
+//         _13 = const <std::cell::Cell<T>>::set(_14, _15) -> bb7;
+//     }
+//     bb7: {
+//         EndRegion('33s);
+//         StorageDead(_15);
+//         StorageDead(_14);
+//         StorageDead(_17);
+//         _1 = ();
+//         EndRegion('35_0rs);
+//         StorageDead(_2);
+//         goto -> bb1;
+//     }
+// }
+// END rustc.node16.SimplifyCfg-qualify-consts.after.mir
diff --git a/src/test/mir-opt/end_region_destruction_extents_1.rs b/src/test/mir-opt/end_region_destruction_extents_1.rs
new file mode 100644
index 00000000000..1f9ad988acc
--- /dev/null
+++ b/src/test/mir-opt/end_region_destruction_extents_1.rs
@@ -0,0 +1,161 @@
+// 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.
+
+// compile-flags: -Z identify_regions -Z span_free_formats -Z emit-end-regions
+// ignore-tidy-linelength
+
+// A scenario with significant destruction code extents (which have
+// suffix "dce" in current `-Z identify_regions` rendering).
+
+#![feature(generic_param_attrs)]
+#![feature(dropck_eyepatch)]
+
+fn main() {
+    // Since the second param to `D1` is may_dangle, it is legal for
+    // the region of that parameter to end before the drop code for D1
+    // is executed.
+    (D1(&S1("ex1"), &S1("dang1"))).0;
+}
+
+#[derive(Debug)]
+struct S1(&'static str);
+
+#[derive(Debug)]
+struct D1<'a, 'b>(&'a S1, &'b S1);
+
+// The `#[may_dangle]` means that references of type `&'b _` may be
+// invalid during the execution of this destructor; i.e. in this case
+// the destructor code is not allowed to read or write `*self.1`, while
+// it can read/write `*self.0`.
+unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> {
+    fn drop(&mut self) {
+        println!("D1({:?}, _)", self.0);
+    }
+}
+
+// Notes on the MIR output below:
+//
+// 1. The `EndRegion('10s)` is allowed to precede the `drop(_3)`
+//    solely because of the #[may_dangle] mentioned above.
+//
+// 2. Regarding the occurrence of `EndRegion('12ds)` *after* `StorageDead(_6)`
+//    (where we have borrows `&'12ds _6`): Eventually:
+//
+//    i. this code should be rejected (by mir-borrowck), or
+//
+//    ii. the MIR code generation should be changed so that the
+//        EndRegion('12ds)` precedes `StorageDead(_6)` in the
+//        control-flow.  (Note: arielb1 views drop+storagedead as one
+//        unit, and does not see this option as a useful avenue to
+//        explore.), or
+//
+//    iii. the presence of EndRegion should be made irrelevant by a
+//        transformation encoding the effects of rvalue-promotion.
+//        This may be the simplest and most-likely option; note in
+//        particular that `StorageDead(_6)` goes away below in
+//        rustc.node4.QualifyAndPromoteConstants.after.mir
+
+// END RUST SOURCE
+
+// START rustc.node4.QualifyAndPromoteConstants.before.mir
+// fn main() -> () {
+//     let mut _0: ();
+//     let mut _1: &'12ds S1;
+//     let mut _2: &'12ds S1;
+//     let mut _3: D1<'12ds, '10s>;
+//     let mut _4: &'12ds S1;
+//     let mut _5: &'12ds S1;
+//     let mut _6: S1;
+//     let mut _7: &'10s S1;
+//     let mut _8: &'10s S1;
+//     let mut _9: S1;
+//
+//     bb0: {
+//         StorageLive(_2);
+//         StorageLive(_3);
+//         StorageLive(_4);
+//         StorageLive(_5);
+//         StorageLive(_6);
+//         _6 = S1::{{constructor}}(const "ex1",);
+//         _5 = &'12ds _6;
+//         _4 = &'12ds (*_5);
+//         StorageLive(_7);
+//         StorageLive(_8);
+//         StorageLive(_9);
+//         _9 = S1::{{constructor}}(const "dang1",);
+//         _8 = &'10s _9;
+//         _7 = &'10s (*_8);
+//         _3 = D1<'12ds, '10s>::{{constructor}}(_4, _7);
+//         EndRegion('10s);
+//         StorageDead(_7);
+//         StorageDead(_4);
+//         _2 = (_3.0: &'12ds S1);
+//         _1 = _2;
+//         StorageDead(_2);
+//         drop(_3) -> bb1;
+//     }
+//
+//     bb1: {
+//         StorageDead(_3);
+//         StorageDead(_8);
+//         StorageDead(_9);
+//         StorageDead(_5);
+//         StorageDead(_6);
+//         EndRegion('12ds);
+//         _0 = ();
+//         return;
+//     }
+// }
+// END rustc.node4.QualifyAndPromoteConstants.before.mir
+
+// START rustc.node4.QualifyAndPromoteConstants.after.mir
+// fn main() -> () {
+//     let mut _0: ();
+//     let mut _1: &'12ds S1;
+//     let mut _2: &'12ds S1;
+//     let mut _3: D1<'12ds, '10s>;
+//     let mut _4: &'12ds S1;
+//     let mut _5: &'12ds S1;
+//     let mut _6: S1;
+//     let mut _7: &'10s S1;
+//     let mut _8: &'10s S1;
+//     let mut _9: S1;
+//
+//     bb0: {
+//         StorageLive(_2);
+//         StorageLive(_3);
+//         StorageLive(_4);
+//         StorageLive(_5);
+//         _5 = promoted1;
+//         _4 = &'12ds (*_5);
+//         StorageLive(_7);
+//         StorageLive(_8);
+//         _8 = promoted0;
+//         _7 = &'10s (*_8);
+//         _3 = D1<'12ds, '10s>::{{constructor}}(_4, _7);
+//         EndRegion('10s);
+//         StorageDead(_7);
+//         StorageDead(_4);
+//         _2 = (_3.0: &'12ds S1);
+//         _1 = _2;
+//         StorageDead(_2);
+//         drop(_3) -> bb1;
+//     }
+//
+//     bb1: {
+//         StorageDead(_3);
+//         StorageDead(_8);
+//         StorageDead(_5);
+//         EndRegion('12ds);
+//         _0 = ();
+//         return;
+//     }
+// }
+// END rustc.node4.QualifyAndPromoteConstants.after.mir
diff --git a/src/test/mir-opt/validate_1.rs b/src/test/mir-opt/validate_1.rs
index 842bc8374b6..ec044225b83 100644
--- a/src/test/mir-opt/validate_1.rs
+++ b/src/test/mir-opt/validate_1.rs
@@ -68,8 +68,8 @@ fn main() {
 //         _5 = (*_4);
 //         _0 = _5;
 //         StorageDead(_5);
-//         StorageDead(_4);
 //         EndRegion(ReScope(Remainder(BlockRemainder { block: ItemLocalId(22), first_statement_index: 0 })));
+//         StorageDead(_4);
 //         StorageDead(_3);
 //         return;
 //     }
diff --git a/src/test/mir-opt/validate_5.rs b/src/test/mir-opt/validate_5.rs
index 4bebc74ae46..ff0c781d1e3 100644
--- a/src/test/mir-opt/validate_5.rs
+++ b/src/test/mir-opt/validate_5.rs
@@ -54,8 +54,8 @@ fn main() {
 //         _5 = &ReErased mut (*_3);
 //         Validate(Acquire, [(*_5): i32/ReScope(Node(ItemLocalId(9)))]);
 //         _4 = _5 as *mut i32 (Misc);
-//         StorageDead(_5);
 //         EndRegion(ReScope(Node(ItemLocalId(9))));
+//         StorageDead(_5);
 //         Validate(Release, [_0: bool, _4: *mut i32]);
 //         _0 = const write_42(_4) -> bb1;
 //     }