Skip to content

also check let arms and nested patterns for mutable borrows #51274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 34 additions & 5 deletions src/librustc_passes/rvalue_promotion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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(_) => {}
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down
63 changes: 63 additions & 0 deletions src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.nll.stderr
Original file line number Diff line number Diff line change
@@ -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`.
41 changes: 41 additions & 0 deletions src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.rs
Original file line number Diff line number Diff line change
@@ -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() {
}
57 changes: 57 additions & 0 deletions src/test/ui/borrowck/promote-ref-mut-in-let-issue-46557.stderr
Original file line number Diff line number Diff line change
@@ -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`.