From a667049c33753dec2ea236fcca3dbf3a7540a5df Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 1 Jun 2018 09:41:44 -0400 Subject: [PATCH] also check `let` arms and nested patterns for mutable borrows --- src/librustc/middle/mem_categorization.rs | 16 +++++ src/librustc_passes/rvalue_promotion.rs | 39 ++++++++++-- ...mote-ref-mut-in-let-issue-46557.nll.stderr | 63 +++++++++++++++++++ .../promote-ref-mut-in-let-issue-46557.rs | 41 ++++++++++++ .../promote-ref-mut-in-let-issue-46557.stderr | 57 +++++++++++++++++ 5 files changed, 211 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.nll.stderr create mode 100644 src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.rs create mode 100644 src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.stderr diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index f2120d9786827..959dda69e30ed 100644 --- 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 index 52ceb3ff5951d..74b9315f0c1ee 100644 --- 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 new file mode 100644 index 0000000000000..95acdab3e8001 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..4c5f458d6a35f --- /dev/null +++ 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 or the MIT license +// , 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 new file mode 100644 index 0000000000000..931eb7da744e9 --- /dev/null +++ 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`.