Skip to content

Commit 125c9d9

Browse files
matthewjaspermatthewjasper
committed
Fix various nll unused mut errors
1 parent 1029775 commit 125c9d9

File tree

10 files changed

+212
-48
lines changed

10 files changed

+212
-48
lines changed

‎src/librustc_mir/borrow_check/mod.rs‎

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,26 +1181,37 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
11811181
// We need to report back the list of mutable upvars that were
11821182
// moved into the closure and subsequently used by the closure,
11831183
// in order to populate our used_mut set.
1184-
if let AggregateKind::Closure(def_id, _) = &**aggregate_kind {
1185-
let BorrowCheckResult {
1186-
used_mut_upvars, ..
1187-
} = self.tcx.mir_borrowck(*def_id);
1188-
debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars);
1189-
for field in used_mut_upvars {
1190-
match operands[field.index()] {
1191-
Operand::Move(Place::Local(local)) => {
1192-
self.used_mut.insert(local);
1193-
}
1194-
Operand::Move(ref place @ Place::Projection(_)) => {
1195-
if let Some(field) = self.is_upvar_field_projection(place) {
1196-
self.used_mut_upvars.push(field);
1184+
match **aggregate_kind {
1185+
AggregateKind::Closure(def_id, _)
1186+
| AggregateKind::Generator(def_id, _, _) => {
1187+
let BorrowCheckResult {
1188+
used_mut_upvars, ..
1189+
} = self.tcx.mir_borrowck(def_id);
1190+
debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars);
1191+
for field in used_mut_upvars {
1192+
// This relies on the current way that by-value
1193+
// captures of a closure are copied/moved directly
1194+
// when generating MIR.
1195+
match operands[field.index()] {
1196+
Operand::Move(Place::Local(local))
1197+
| Operand::Copy(Place::Local(local)) => {
1198+
self.used_mut.insert(local);
1199+
}
1200+
Operand::Move(ref place @ Place::Projection(_))
1201+
| Operand::Copy(ref place @ Place::Projection(_)) => {
1202+
if let Some(field) = self.is_upvar_field_projection(place) {
1203+
self.used_mut_upvars.push(field);
1204+
}
11971205
}
1206+
Operand::Move(Place::Static(..))
1207+
| Operand::Copy(Place::Static(..))
1208+
| Operand::Constant(..) => {}
11981209
}
1199-
Operand::Move(Place::Static(..))
1200-
| Operand::Copy(..)
1201-
| Operand::Constant(..) => {}
12021210
}
12031211
}
1212+
AggregateKind::Adt(..)
1213+
| AggregateKind::Array(..)
1214+
| AggregateKind::Tuple { .. } => (),
12041215
}
12051216

12061217
for operand in operands {
@@ -1939,6 +1950,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
19391950
}
19401951
}
19411952
}
1953+
RootPlace {
1954+
place: _,
1955+
is_local_mutation_allowed: LocalMutationIsAllowed::Yes,
1956+
} => {}
19421957
RootPlace {
19431958
place: place @ Place::Projection(_),
19441959
is_local_mutation_allowed: _,
@@ -2114,13 +2129,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
21142129
match *place {
21152130
Place::Projection(ref proj) => match proj.elem {
21162131
ProjectionElem::Field(field, _ty) => {
2117-
let is_projection_from_ty_closure = proj
2118-
.base
2119-
.ty(self.mir, self.tcx)
2120-
.to_ty(self.tcx)
2121-
.is_closure();
2132+
let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
21222133

2123-
if is_projection_from_ty_closure {
2134+
if base_ty.is_closure() || base_ty.is_generator() {
21242135
Some(field)
21252136
} else {
21262137
None

‎src/librustc_mir/build/expr/as_rvalue.rs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,29 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
186186
}
187187
ExprKind::Closure { closure_id, substs, upvars, movability } => {
188188
// see (*) above
189-
let mut operands: Vec<_> =
190-
upvars.into_iter()
191-
.map(|upvar| unpack!(block = this.as_operand(block, scope, upvar)))
192-
.collect();
189+
let mut operands: Vec<_> = upvars
190+
.into_iter()
191+
.map(|upvar| {
192+
let upvar = this.hir.mirror(upvar);
193+
match Category::of(&upvar.kind) {
194+
// Use as_place to avoid creating a temporary when
195+
// moving a variable into a closure, so that
196+
// borrowck knows which variables to mark as being
197+
// used as mut. This is OK here because the upvar
198+
// expressions have no side effects and act on
199+
// disjoint places.
200+
// This occurs when capturing by copy/move, while
201+
// by reference captures use as_operand
202+
Some(Category::Place) => {
203+
let place = unpack!(block = this.as_place(block, upvar));
204+
this.consume_by_copy_or_move(place)
205+
}
206+
_ => {
207+
unpack!(block = this.as_operand(block, scope, upvar))
208+
}
209+
}
210+
})
211+
.collect();
193212
let result = match substs {
194213
UpvarSubsts::Generator(substs) => {
195214
let movability = movability.unwrap();

‎src/librustc_mir/build/mod.rs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -672,11 +672,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
672672
{
673673
// Allocate locals for the function arguments
674674
for &ArgInfo(ty, _, pattern, _) in arguments.iter() {
675-
// If this is a simple binding pattern, give the local a nice name for debuginfo.
675+
// If this is a simple binding pattern, give the local a name for
676+
// debuginfo and so that error reporting knows that this is a user
677+
// variable. For any other pattern the pattern introduces new
678+
// variables which will be named instead.
676679
let mut name = None;
677680
if let Some(pat) = pattern {
678-
if let hir::PatKind::Binding(_, _, ident, _) = pat.node {
679-
name = Some(ident.name);
681+
match pat.node {
682+
hir::PatKind::Binding(hir::BindingAnnotation::Unannotated, _, ident, _)
683+
| hir::PatKind::Binding(hir::BindingAnnotation::Mutable, _, ident, _) => {
684+
name = Some(ident.name);
685+
}
686+
_ => (),
680687
}
681688
}
682689

‎src/test/mir-opt/end_region_7.rs‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,38 +34,31 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
3434
// ...
3535
// let mut _2: ();
3636
// let mut _3: [closure@NodeId(22) d:D];
37-
// let mut _4: D;
3837
// bb0: {
3938
// StorageLive(_1);
4039
// _1 = D::{{constructor}}(const 0i32,);
4140
// StorageLive(_3);
42-
// StorageLive(_4);
43-
// _4 = move _1;
44-
// _3 = [closure@NodeId(22)] { d: move _4 };
45-
// drop(_4) -> [return: bb4, unwind: bb3];
41+
// _3 = [closure@NodeId(22)] { d: move _1 };
42+
// _2 = const foo(move _3) -> [return: bb2, unwind: bb4];
4643
// }
4744
// bb1: {
4845
// resume;
4946
// }
5047
// bb2: {
51-
// drop(_1) -> bb1;
48+
// drop(_3) -> [return: bb5, unwind: bb3];
5249
// }
5350
// bb3: {
54-
// drop(_3) -> bb2;
51+
// drop(_1) -> bb1;
5552
// }
5653
// bb4: {
57-
// StorageDead(_4);
58-
// _2 = const foo(move _3) -> [return: bb5, unwind: bb3];
54+
// drop(_3) -> bb3;
5955
// }
6056
// bb5: {
61-
// drop(_3) -> [return: bb6, unwind: bb2];
62-
// }
63-
// bb6: {
6457
// StorageDead(_3);
6558
// _0 = ();
66-
// drop(_1) -> [return: bb7, unwind: bb1];
59+
// drop(_1) -> [return: bb6, unwind: bb1];
6760
// }
68-
// bb7: {
61+
// bb6: {
6962
// StorageDead(_1);
7063
// return;
7164
// }

‎src/test/mir-opt/end_region_8.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,13 @@ fn foo<F>(f: F) where F: FnOnce() -> i32 {
3737
// ...
3838
// let mut _3: ();
3939
// let mut _4: [closure@NodeId(22) r:&'19s D];
40-
// let mut _5: &'21_1rs D;
4140
// bb0: {
4241
// StorageLive(_1);
4342
// _1 = D::{{constructor}}(const 0i32,);
4443
// StorageLive(_2);
4544
// _2 = &'21_1rs _1;
4645
// StorageLive(_4);
47-
// StorageLive(_5);
48-
// _5 = _2;
49-
// _4 = [closure@NodeId(22)] { r: move _5 };
50-
// StorageDead(_5);
46+
// _4 = [closure@NodeId(22)] { r: _2 };
5147
// _3 = const foo(move _4) -> [return: bb2, unwind: bb3];
5248
// }
5349
// bb1: {

‎src/test/ui/nll/capture-mut-ref.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Check that capturing a mutable reference by move and assigning to its
12+
// referent doesn't make the unused mut lint think that it is mutable.
13+
14+
#![feature(nll)]
15+
#![deny(unused_mut)]
16+
17+
fn mutable_upvar() {
18+
let mut x = &mut 0;
19+
//~^ ERROR
20+
move || {
21+
*x = 1;
22+
};
23+
}
24+
25+
fn main() {}

‎src/test/ui/nll/capture-mut-ref.stderr‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: variable does not need to be mutable
2+
--> $DIR/capture-mut-ref.rs:18:9
3+
|
4+
LL | let mut x = &mut 0;
5+
| ----^
6+
| |
7+
| help: remove this `mut`
8+
|
9+
note: lint level defined here
10+
--> $DIR/capture-mut-ref.rs:15:9
11+
|
12+
LL | #![deny(unused_mut)]
13+
| ^^^^^^^^^^
14+
15+
error: aborting due to previous error
16+

‎src/test/ui/nll/extra-unused-mut.rs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// extra unused mut lint tests for #51918
12+
13+
// run-pass
14+
15+
#![feature(generators, nll)]
16+
#![deny(unused_mut)]
17+
18+
fn ref_argument(ref _y: i32) {}
19+
20+
// #51801
21+
fn mutable_upvar() {
22+
let mut x = 0;
23+
move || {
24+
x = 1;
25+
};
26+
}
27+
28+
// #50897
29+
fn generator_mutable_upvar() {
30+
let mut x = 0;
31+
move || {
32+
x = 1;
33+
yield;
34+
};
35+
}
36+
37+
// #51830
38+
fn ref_closure_argument() {
39+
let _ = Some(0).as_ref().map(|ref _a| true);
40+
}
41+
42+
struct Expr {
43+
attrs: Vec<u32>,
44+
}
45+
46+
// #51904
47+
fn parse_dot_or_call_expr_with(mut attrs: Vec<u32>) {
48+
let x = Expr { attrs: vec![] };
49+
Some(Some(x)).map(|expr|
50+
expr.map(|mut expr| {
51+
attrs.push(666);
52+
expr.attrs = attrs;
53+
expr
54+
})
55+
);
56+
}
57+
58+
fn main() {
59+
ref_argument(0);
60+
mutable_upvar();
61+
generator_mutable_upvar();
62+
ref_closure_argument();
63+
parse_dot_or_call_expr_with(Vec::new());
64+
}

‎src/test/ui/nll/generator-upvar-mutability.rs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Check that generators respect the muatability of their upvars.
12+
13+
#![feature(generators, nll)]
14+
15+
fn mutate_upvar() {
16+
let x = 0;
17+
move || {
18+
x = 1;
19+
//~^ ERROR
20+
yield;
21+
};
22+
}
23+
24+
fn main() {}

‎src/test/ui/nll/generator-upvar-mutability.stderr‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
error[E0594]: cannot assign to immutable item `x`
2+
--> $DIR/generator-upvar-mutability.rs:18:9
3+
|
4+
LL | x = 1;
5+
| ^^^^^ cannot assign
6+
7+
error: aborting due to previous error
8+
9+
For more information about this error, try `rustc --explain E0594`.

0 commit comments

Comments
 (0)