OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #51964 - matthewjasper:unused-mut-mir-generation, r=nikomatsakis

Browse Source 
[NLL] Fix various unused mut errors

Closes #51801
Closes #50897
Closes #51830
Closes #51904
cc #51918 - keeping this one open in case there are any more issues

This PR contains multiple changes. List of changes with examples of what they fix:

* Change mir generation so that the parameter variable doesn't get a name when a `ref` pattern is used as an argument
```rust
fn f(ref y: i32) {} // doesn't trigger lint
```
* Change mir generation so that by-move closure captures don't get first moved into a temporary.
```rust
let mut x = 0; // doesn't trigger lint
move || {
    x = 1;
};
```
* Treat generator upvars the same as closure upvars
```rust
let mut x = 0; // This mut is now necessary and is not linted against.
move || {
    x = 1;
    yield;
};
```

r? @nikomatsakis
This commit is contained in:
bors 2018-07-05 00:22:14 +00:00
commit b51ca20ce5
10 changed files with 212 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
src/librustc_mir/borrow_check/mod.rs
View File

@ -1182,26 +1182,37 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// We need to report back the list of mutable upvars that were
// moved into the closure and subsequently used by the closure,
// in order to populate our used_mut set.
if let AggregateKind::Closure(def_id, _) = &**aggregate_kind {
let BorrowCheckResult {
used_mut_upvars, ..
} = self.tcx.mir_borrowck(*def_id);
debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars);
for field in used_mut_upvars {
match operands[field.index()] {
Operand::Move(Place::Local(local)) => {
self.used_mut.insert(local);
}
Operand::Move(ref place @ Place::Projection(_)) => {
if let Some(field) = self.is_upvar_field_projection(place) {
self.used_mut_upvars.push(field);
match **aggregate_kind {
AggregateKind::Closure(def_id, _)
| AggregateKind::Generator(def_id, _, _) => {
let BorrowCheckResult {
used_mut_upvars, ..
} = self.tcx.mir_borrowck(def_id);
debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars);
for field in used_mut_upvars {
// This relies on the current way that by-value
// captures of a closure are copied/moved directly
// when generating MIR.
match operands[field.index()] {
Operand::Move(Place::Local(local))
| Operand::Copy(Place::Local(local)) => {
self.used_mut.insert(local);
}
Operand::Move(ref place @ Place::Projection(_))
| Operand::Copy(ref place @ Place::Projection(_)) => {
if let Some(field) = self.is_upvar_field_projection(place) {
self.used_mut_upvars.push(field);
}
}
Operand::Move(Place::Static(..))
| Operand::Copy(Place::Static(..))
| Operand::Constant(..) => {}
}
Operand::Move(Place::Static(..))
| Operand::Copy(..)
| Operand::Constant(..) => {}
}
}
AggregateKind::Adt(..)
| AggregateKind::Array(..)
| AggregateKind::Tuple { .. } => (),
}
for operand in operands {
@ -1940,6 +1951,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}
}
}
RootPlace {
place: _,
is_local_mutation_allowed: LocalMutationIsAllowed::Yes,
} => {}
RootPlace {
place: place @ Place::Projection(_),
is_local_mutation_allowed: _,
@ -2115,13 +2130,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
match *place {
Place::Projection(ref proj) => match proj.elem {
ProjectionElem::Field(field, _ty) => {
let is_projection_from_ty_closure = proj
.base
.ty(self.mir, self.tcx)
.to_ty(self.tcx)
.is_closure();
let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
if is_projection_from_ty_closure {
if base_ty.is_closure() || base_ty.is_generator() {
Some(field)
} else {
None

27
src/librustc_mir/build/expr/as_rvalue.rs
View File

@ -186,10 +186,29 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}
ExprKind::Closure { closure_id, substs, upvars, movability } => {
// see (*) above
let mut operands: Vec<_> =
upvars.into_iter()
.map(|upvar| unpack!(block = this.as_operand(block, scope, upvar)))
.collect();
let mut operands: Vec<_> = upvars
.into_iter()
.map(|upvar| {
let upvar = this.hir.mirror(upvar);
match Category::of(&upvar.kind) {
// Use as_place to avoid creating a temporary when
// moving a variable into a closure, so that
// borrowck knows which variables to mark as being
// used as mut. This is OK here because the upvar
// expressions have no side effects and act on
// disjoint places.
// This occurs when capturing by copy/move, while
// by reference captures use as_operand
Some(Category::Place) => {
let place = unpack!(block = this.as_place(block, upvar));
this.consume_by_copy_or_move(place)
}
_ => {
unpack!(block = this.as_operand(block, scope, upvar))
}
}
})
.collect();
let result = match substs {
UpvarSubsts::Generator(substs) => {
let movability = movability.unwrap();

13
src/librustc_mir/build/mod.rs
View File

@ -672,11 +672,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
{
// Allocate locals for the function arguments
for &ArgInfo(ty, _, pattern, _) in arguments.iter() {
// If this is a simple binding pattern, give the local a nice name for debuginfo.
// If this is a simple binding pattern, give the local a name for
// debuginfo and so that error reporting knows that this is a user
// variable. For any other pattern the pattern introduces new
// variables which will be named instead.
let mut name = None;
if let Some(pat) = pattern {
if let hir::PatKind::Binding(_, _, ident, _) = pat.node {
name = Some(ident.name);
match pat.node {
hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, ident, _)
| hir::PatKind::Binding(hir::BindingAnnotation::Mutable, _, ident, _) => {
name = Some(ident.name);
}
_ => (),
}
}

21
src/test/mir-opt/end_region_7.rs
View File

@ -34,38 +34,31 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
// ...
// let mut _2: ();
// let mut _3: [closure@NodeId(22) d:D];
// let mut _4: D;
// bb0: {
// StorageLive(_1);
// _1 = D::{{constructor}}(const 0i32,);
// StorageLive(_3);
// StorageLive(_4);
// _4 = move _1;
// _3 = [closure@NodeId(22)] { d: move _4 };
// drop(_4) -> [return: bb4, unwind: bb3];
// _3 = [closure@NodeId(22)] { d: move _1 };
// _2 = const foo(move _3) -> [return: bb2, unwind: bb4];
// }
// bb1: {
// resume;
// }
// bb2: {
// drop(_1) -> bb1;
// drop(_3) -> [return: bb5, unwind: bb3];
// }
// bb3: {
// drop(_3) -> bb2;
// drop(_1) -> bb1;
// }
// bb4: {
// StorageDead(_4);
// _2 = const foo(move _3) -> [return: bb5, unwind: bb3];
// drop(_3) -> bb3;
// }
// bb5: {
// drop(_3) -> [return: bb6, unwind: bb2];
// }
// bb6: {
// StorageDead(_3);
// _0 = ();
// drop(_1) -> [return: bb7, unwind: bb1];
// drop(_1) -> [return: bb6, unwind: bb1];
// }
// bb7: {
// bb6: {
// StorageDead(_1);
// return;
// }

6
src/test/mir-opt/end_region_8.rs
View File

@ -37,17 +37,13 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
// ...
// let mut _3: ();
// let mut _4: [closure@NodeId(22) r:&'19s D];
// let mut _5: &'21_1rs D;
// bb0: {
// StorageLive(_1);
// _1 = D::{{constructor}}(const 0i32,);
// StorageLive(_2);
// _2 = &'21_1rs _1;
// StorageLive(_4);
// StorageLive(_5);
// _5 = _2;
// _4 = [closure@NodeId(22)] { r: move _5 };
// StorageDead(_5);
// _4 = [closure@NodeId(22)] { r: _2 };
// _3 = const foo(move _4) -> [return: bb2, unwind: bb3];
// }
// bb1: {

25
src/test/ui/nll/capture-mut-ref.rs Normal file
View File

@ -0,0 +1,25 @@
// Copyright 2018 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 capturing a mutable reference by move and assigning to its
// referent doesn't make the unused mut lint think that it is mutable.
#![feature(nll)]
#![deny(unused_mut)]
fn mutable_upvar() {
let mut x = &mut 0;
//~^ ERROR
move || {
*x = 1;
};
}
fn main() {}

16
src/test/ui/nll/capture-mut-ref.stderr Normal file
View File

@ -0,0 +1,16 @@
error: variable does not need to be mutable
--> $DIR/capture-mut-ref.rs:18:9
|
LL | let mut x = &mut 0;
| ----^
| |
| help: remove this `mut`
|
note: lint level defined here
--> $DIR/capture-mut-ref.rs:15:9
|
LL | #![deny(unused_mut)]
| ^^^^^^^^^^
error: aborting due to previous error

64
src/test/ui/nll/extra-unused-mut.rs Normal file
View File

@ -0,0 +1,64 @@
// Copyright 2018 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.
// extra unused mut lint tests for #51918
// run-pass
#![feature(generators, nll)]
#![deny(unused_mut)]
fn ref_argument(ref _y: i32) {}
// #51801
fn mutable_upvar() {
let mut x = 0;
move || {
x = 1;
};
}
// #50897
fn generator_mutable_upvar() {
let mut x = 0;
move || {
x = 1;
yield;
};
}
// #51830
fn ref_closure_argument() {
let _ = Some(0).as_ref().map(|ref _a| true);
}
struct Expr {
attrs: Vec<u32>,
}
// #51904
fn parse_dot_or_call_expr_with(mut attrs: Vec<u32>) {
let x = Expr { attrs: vec![] };
Some(Some(x)).map(|expr|
expr.map(|mut expr| {
attrs.push(666);
expr.attrs = attrs;
expr
})
);
}
fn main() {
ref_argument(0);
mutable_upvar();
generator_mutable_upvar();
ref_closure_argument();
parse_dot_or_call_expr_with(Vec::new());
}

24
src/test/ui/nll/generator-upvar-mutability.rs Normal file
View File

@ -0,0 +1,24 @@
// Copyright 2018 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 generators respect the muatability of their upvars.
#![feature(generators, nll)]
fn mutate_upvar() {
let x = 0;
move || {
x = 1;
//~^ ERROR
yield;
};
}
fn main() {}

9
src/test/ui/nll/generator-upvar-mutability.stderr Normal file
View File

@ -0,0 +1,9 @@
error[E0594]: cannot assign to immutable item `x`
--> $DIR/generator-upvar-mutability.rs:18:9
|
LL | x = 1;
| ^^^^^ cannot assign
error: aborting due to previous error
For more information about this error, try `rustc --explain E0594`.