Auto merge of rust-lang#51274 - nikomatsakis:issue-46557-promote-ref-mut, r=eddyb

bors · bors · commit d830f46b77bb · 2018-06-02T13:22:38.000Z
also check `let` arms and nested patterns for mutable borrows Fixes rust-lang#46557 r? @eddyb
diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs
@@ -936,16 +936,32 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
                            span: Span,
                            expr_ty: Ty<'tcx>)
                            -> cmt_<'tcx> {
+        debug!(
+            "cat_rvalue_node(id={:?}, span={:?}, expr_ty={:?})",
+            id,
+            span,
+            expr_ty,
+        );
         let hir_id = self.tcx.hir.node_to_hir_id(id);
         let promotable = self.rvalue_promotable_map.as_ref().map(|m| m.contains(&hir_id.local_id))
                                                             .unwrap_or(false);
 
+        debug!(
+            "cat_rvalue_node: promotable = {:?}",
+            promotable,
+        );
+
         // Always promote `[T; 0]` (even when e.g. borrowed mutably).
         let promotable = match expr_ty.sty {
             ty::TyArray(_, len) if len.assert_usize(self.tcx) == Some(0) => true,
             _ => promotable,
         };
 
+        debug!(
+            "cat_rvalue_node: promotable = {:?} (2)",
+            promotable,
+        );
+
         // Compute maximum lifetime of this rvalue. This is 'static if
         // we can promote to a constant, otherwise equal to enclosing temp
         // lifetime.
diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs
@@ -150,6 +150,23 @@ impl<'a, 'gcx> CheckCrateVisitor<'a, 'gcx> {
                 span.allows_unstable();
         }
     }
+
+    /// While the `ExprUseVisitor` walks, we will identify which
+    /// expressions are borrowed, and insert their ids into this
+    /// table. Actually, we insert the "borrow-id", which is normally
+    /// the id of the expession being borrowed: but in the case of
+    /// `ref mut` borrows, the `id` of the pattern is
+    /// inserted. Therefore later we remove that entry from the table
+    /// and transfer it over to the value being matched. This will
+    /// then prevent said value from being promoted.
+    fn remove_mut_rvalue_borrow(&mut self, pat: &hir::Pat) -> bool {
+        let mut any_removed = false;
+        pat.walk(|p| {
+            any_removed |= self.mut_rvalue_borrows.remove(&p.id);
+            true
+        });
+        any_removed
+    }
 }
 
 impl<'a, 'tcx> Visitor<'tcx> for CheckCrateVisitor<'a, 'tcx> {
@@ -200,9 +217,15 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckCrateVisitor<'a, 'tcx> {
     fn visit_stmt(&mut self, stmt: &'tcx hir::Stmt) {
         match stmt.node {
             hir::StmtDecl(ref decl, _) => {
-                match decl.node {
-                    hir::DeclLocal(_) => {
+                match &decl.node {
+                    hir::DeclLocal(local) => {
                         self.promotable = false;
+
+                        if self.remove_mut_rvalue_borrow(&local.pat) {
+                            if let Some(init) = &local.init {
+                                self.mut_rvalue_borrows.insert(init.id);
+                            }
+                        }
                     }
                     // Item statements are allowed
                     hir::DeclItem(_) => {}
@@ -229,9 +252,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckCrateVisitor<'a, 'tcx> {
             // patterns and set that on the discriminator.
             let mut mut_borrow = false;
             for pat in arms.iter().flat_map(|arm| &arm.pats) {
-                if self.mut_rvalue_borrows.remove(&pat.id) {
-                    mut_borrow = true;
-                }
+                mut_borrow = self.remove_mut_rvalue_borrow(pat);
             }
             if mut_borrow {
                 self.mut_rvalue_borrows.insert(discr.id);
@@ -498,6 +519,14 @@ impl<'a, 'gcx, 'tcx> euv::Delegate<'tcx> for CheckCrateVisitor<'a, 'gcx> {
               _loan_region: ty::Region<'tcx>,
               bk: ty::BorrowKind,
               loan_cause: euv::LoanCause) {
+        debug!(
+            "borrow(borrow_id={:?}, cmt={:?}, bk={:?}, loan_cause={:?})",
+            borrow_id,
+            cmt,
+            bk,
+            loan_cause,
+        );
+
         // Kind of hacky, but we allow Unsafe coercions in constants.
         // These occur when we convert a &T or *T to a *U, as well as
         // when making a thin pointer (e.g., `*T`) into a fat pointer
diff --git a/src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.nll.stderr b/src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.nll.stderr
@@ -0,0 +1,63 @@
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:15:21
+   |
+LL |   fn gimme_static_mut_let() -> &'static mut u32 {
+   |  _______________________________________________-
+LL | |     let ref mut x = 1234543; //~ ERROR
+   | |                     ^^^^^^^ temporary value does not live long enough
+LL | |     x
+LL | | }
+   | | -
+   | | |
+   | |_temporary value only lives until here
+   |   borrow later used here
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:20:25
+   |
+LL |   fn gimme_static_mut_let_nested() -> &'static mut u32 {
+   |  ______________________________________________________-
+LL | |     let (ref mut x, ) = (1234543, ); //~ ERROR
+   | |                         ^^^^^^^^^^^ temporary value does not live long enough
+LL | |     x
+LL | | }
+   | | -
+   | | |
+   | |_temporary value only lives until here
+   |   borrow later used here
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:25:11
+   |
+LL |     match 1234543 {
+   |           ^^^^^^^ temporary value does not live long enough
+...
+LL | }
+   | - temporary value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:31:11
+   |
+LL |     match (123443,) {
+   |           ^^^^^^^^^ temporary value does not live long enough
+...
+LL | }
+   | - temporary value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:37:10
+   |
+LL |     &mut 1234543 //~ ERROR
+   |          ^^^^^^^ temporary value does not live long enough
+LL | }
+   | - temporary value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error: aborting due to 5 previous errors
+
+For more information about this error, try `rustc --explain E0597`.
diff --git a/src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.rs b/src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.rs
@@ -0,0 +1,41 @@
+// Copyright 2014 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.
+
+// Test that we fail to promote the constant here which has a `ref
+// mut` borrow.
+
+fn gimme_static_mut_let() -> &'static mut u32 {
+    let ref mut x = 1234543; //~ ERROR
+    x
+}
+
+fn gimme_static_mut_let_nested() -> &'static mut u32 {
+    let (ref mut x, ) = (1234543, ); //~ ERROR
+    x
+}
+
+fn gimme_static_mut_match() -> &'static mut u32 {
+    match 1234543 {
+        ref mut x => x //~ ERROR
+    }
+}
+
+fn gimme_static_mut_match_nested() -> &'static mut u32 {
+    match (123443,) {
+        (ref mut x,) => x, //~ ERROR
+    }
+}
+
+fn gimme_static_mut_ampersand() -> &'static mut u32 {
+    &mut 1234543 //~ ERROR
+}
+
+fn main() {
+}
diff --git a/src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.stderr b/src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.stderr
@@ -0,0 +1,57 @@
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:15:9
+   |
+LL |     let ref mut x = 1234543; //~ ERROR
+   |         ^^^^^^^^^ temporary value does not live long enough
+LL |     x
+LL | }
+   | - temporary value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:20:10
+   |
+LL |     let (ref mut x, ) = (1234543, ); //~ ERROR
+   |          ^^^^^^^^^ borrowed value does not live long enough
+LL |     x
+LL | }
+   | - borrowed value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:26:9
+   |
+LL |         ref mut x => x //~ ERROR
+   |         ^^^^^^^^^ temporary value does not live long enough
+LL |     }
+LL | }
+   | - temporary value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:32:10
+   |
+LL |         (ref mut x,) => x, //~ ERROR
+   |          ^^^^^^^^^ borrowed value does not live long enough
+LL |     }
+LL | }
+   | - borrowed value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error[E0597]: borrowed value does not live long enough
+  --> $DIR/promote-ref-mut-in-let-issue-46557.rs:37:10
+   |
+LL |     &mut 1234543 //~ ERROR
+   |          ^^^^^^^ temporary value does not live long enough
+LL | }
+   | - temporary value only lives until here
+   |
+   = note: borrowed value must be valid for the static lifetime...
+
+error: aborting due to 5 previous errors
+
+For more information about this error, try `rustc --explain E0597`.
