diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index ea303cd4e6c..932c201d32b 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1181,26 +1181,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 { @@ -1939,6 +1950,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: _, @@ -2114,13 +2129,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 diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index d660b40e9cb..9a756cdfb41 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -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(); diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 4db5c8e9278..c0821cdd3ba 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -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); + } + _ => (), } } diff --git a/src/test/mir-opt/end_region_7.rs b/src/test/mir-opt/end_region_7.rs index e44b41993aa..59261ec9687 100644 --- a/src/test/mir-opt/end_region_7.rs +++ b/src/test/mir-opt/end_region_7.rs @@ -34,38 +34,31 @@ fn foo(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; // } diff --git a/src/test/mir-opt/end_region_8.rs b/src/test/mir-opt/end_region_8.rs index 7fdf971b3b9..a49913a62d9 100644 --- a/src/test/mir-opt/end_region_8.rs +++ b/src/test/mir-opt/end_region_8.rs @@ -37,17 +37,13 @@ fn foo(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: { diff --git a/src/test/ui/nll/capture-mut-ref.rs b/src/test/ui/nll/capture-mut-ref.rs new file mode 100644 index 00000000000..c3c5053af0c --- /dev/null +++ b/src/test/ui/nll/capture-mut-ref.rs @@ -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 or the MIT license +// , 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() {} diff --git a/src/test/ui/nll/capture-mut-ref.stderr b/src/test/ui/nll/capture-mut-ref.stderr new file mode 100644 index 00000000000..50a77ee10e5 --- /dev/null +++ b/src/test/ui/nll/capture-mut-ref.stderr @@ -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 + diff --git a/src/test/ui/nll/extra-unused-mut.rs b/src/test/ui/nll/extra-unused-mut.rs new file mode 100644 index 00000000000..ce07e2b0e21 --- /dev/null +++ b/src/test/ui/nll/extra-unused-mut.rs @@ -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 or the MIT license +// , 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, +} + +// #51904 +fn parse_dot_or_call_expr_with(mut attrs: Vec) { + 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()); +} diff --git a/src/test/ui/nll/generator-upvar-mutability.rs b/src/test/ui/nll/generator-upvar-mutability.rs new file mode 100644 index 00000000000..a32e076cb93 --- /dev/null +++ b/src/test/ui/nll/generator-upvar-mutability.rs @@ -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 or the MIT license +// , 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() {} diff --git a/src/test/ui/nll/generator-upvar-mutability.stderr b/src/test/ui/nll/generator-upvar-mutability.stderr new file mode 100644 index 00000000000..9c5c57687a2 --- /dev/null +++ b/src/test/ui/nll/generator-upvar-mutability.stderr @@ -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`.