From 6559323a51ee4c3e148b09fa3aa7de27565ec160 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 4 Aug 2014 13:10:08 -0700 Subject: [PATCH 1/3] librustc: Allow mutation of moved upvars. --- src/librustc/middle/mem_categorization.rs | 57 ++++++++++++++--------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 52a32241faf7c..39a7b4aa3d68e 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -306,6 +306,29 @@ impl MutabilityCategory { } } + fn from_def(def: &def::Def) -> MutabilityCategory { + match *def { + def::DefFn(..) | def::DefStaticMethod(..) | def::DefSelfTy(..) | + def::DefMod(..) | def::DefForeignMod(..) | def::DefVariant(..) | + def::DefTy(..) | def::DefTrait(..) | def::DefPrimTy(..) | + def::DefTyParam(..) | def::DefUse(..) | def::DefStruct(..) | + def::DefTyParamBinder(..) | def::DefRegion(..) | def::DefLabel(..) | + def::DefMethod(..) => fail!("no MutabilityCategory for def: {}", *def), + + def::DefStatic(_, false) => McImmutable, + def::DefStatic(_, true) => McDeclared, + + def::DefArg(_, binding_mode) | + def::DefBinding(_, binding_mode) | + def::DefLocal(_, binding_mode) => match binding_mode { + ast::BindByValue(ast::MutMutable) => McDeclared, + _ => McImmutable + }, + + def::DefUpvar(_, def, _, _) => MutabilityCategory::from_def(&*def) + } + } + pub fn inherit(&self) -> MutabilityCategory { match *self { McImmutable => McImmutable, @@ -503,8 +526,8 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { def::DefStaticMethod(..) => { Ok(self.cat_rvalue_node(id, span, expr_ty)) } - def::DefMod(_) | def::DefForeignMod(_) | def::DefStatic(_, false) | - def::DefUse(_) | def::DefTrait(_) | def::DefTy(_) | def::DefPrimTy(_) | + def::DefMod(_) | def::DefForeignMod(_) | def::DefUse(_) | + def::DefTrait(_) | def::DefTy(_) | def::DefPrimTy(_) | def::DefTyParam(..) | def::DefTyParamBinder(..) | def::DefRegion(_) | def::DefLabel(_) | def::DefSelfTy(..) | def::DefMethod(..) => { Ok(Rc::new(cmt_ { @@ -516,30 +539,25 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { })) } - def::DefStatic(_, true) => { + def::DefStatic(_, _) => { Ok(Rc::new(cmt_ { id:id, span:span, cat:cat_static_item, - mutbl: McDeclared, + mutbl: MutabilityCategory::from_def(&def), ty:expr_ty })) } - def::DefArg(vid, binding_mode) => { + def::DefArg(vid, _) => { // Idea: make this could be rewritten to model by-ref // stuff as `&const` and `&mut`? - // m: mutability of the argument - let m = match binding_mode { - ast::BindByValue(ast::MutMutable) => McDeclared, - _ => McImmutable - }; Ok(Rc::new(cmt_ { id: id, span: span, cat: cat_arg(vid), - mutbl: m, + mutbl: MutabilityCategory::from_def(&def), ty:expr_ty })) } @@ -564,7 +582,6 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { if var_is_refd { self.cat_upvar(id, span, var_id, fn_node_id) } else { - // FIXME #2152 allow mutation of moved upvars Ok(Rc::new(cmt_ { id:id, span:span, @@ -573,13 +590,12 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { onceness: closure_ty.onceness, capturing_proc: fn_node_id, }), - mutbl:McImmutable, + mutbl: MutabilityCategory::from_def(&def), ty:expr_ty })) } } ty::ty_unboxed_closure(_) => { - // FIXME #2152 allow mutation of moved upvars Ok(Rc::new(cmt_ { id: id, span: span, @@ -588,7 +604,7 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { onceness: ast::Many, capturing_proc: fn_node_id, }), - mutbl: McImmutable, + mutbl: MutabilityCategory::from_def(&def), ty: expr_ty })) } @@ -602,19 +618,14 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { } } - def::DefLocal(vid, binding_mode) | - def::DefBinding(vid, binding_mode) => { + def::DefLocal(vid, _) | + def::DefBinding(vid, _) => { // by-value/by-ref bindings are local variables - let m = match binding_mode { - ast::BindByValue(ast::MutMutable) => McDeclared, - _ => McImmutable - }; - Ok(Rc::new(cmt_ { id: id, span: span, cat: cat_local(vid), - mutbl: m, + mutbl: MutabilityCategory::from_def(&def), ty: expr_ty })) } From ead3edb7b99216ed5e5b0c42b157cc7a6c0c1eec Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 4 Aug 2014 13:26:25 -0700 Subject: [PATCH 2/3] librustc: Update unused mut lint to properly track moved upvars. --- src/librustc/middle/borrowck/check_loans.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 7dec42538cffb..65c7e1a6031ee 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -764,18 +764,17 @@ impl<'a> CheckLoanCtxt<'a> { return; fn mark_variable_as_used_mut(this: &CheckLoanCtxt, - cmt: mc::cmt) { + mut cmt: mc::cmt) { //! If the mutability of the `cmt` being written is inherited //! from a local variable, liveness will //! not have been able to detect that this variable's mutability //! is important, so we must add the variable to the //! `used_mut_nodes` table here. - let mut cmt = cmt; loop { - debug!("mark_writes_through_upvars_as_used_mut(cmt={})", - cmt.repr(this.tcx())); + debug!("mark_variable_as_used_mut(cmt={})", cmt.repr(this.tcx())); match cmt.cat.clone() { + mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, .. }) | mc::cat_local(id) | mc::cat_arg(id) => { this.tcx().used_mut_nodes.borrow_mut().insert(id); return; @@ -792,7 +791,6 @@ impl<'a> CheckLoanCtxt<'a> { mc::cat_rvalue(..) | mc::cat_static_item | - mc::cat_copied_upvar(..) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_deref(_, _, mc::BorrowedPtr(..)) | mc::cat_deref(_, _, mc::Implicit(..)) => { From f765759af24ffa8ccc6f9b3913d2e135146e230e Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Mon, 4 Aug 2014 13:30:38 -0700 Subject: [PATCH 3/3] Add tests. --- .../cannot-mutate-captured-non-mut-var.rs | 15 +++++++++++++ .../unused-mut-warning-captured-var.rs | 17 ++++++++++++++ src/test/run-pass/issue-11958.rs | 22 +++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 src/test/compile-fail/cannot-mutate-captured-non-mut-var.rs create mode 100644 src/test/compile-fail/unused-mut-warning-captured-var.rs create mode 100644 src/test/run-pass/issue-11958.rs diff --git a/src/test/compile-fail/cannot-mutate-captured-non-mut-var.rs b/src/test/compile-fail/cannot-mutate-captured-non-mut-var.rs new file mode 100644 index 0000000000000..6bc436d3c18ce --- /dev/null +++ b/src/test/compile-fail/cannot-mutate-captured-non-mut-var.rs @@ -0,0 +1,15 @@ +// 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. + +fn main() { + let x = 1i; + proc() { x = 2; }; + //~^ ERROR: cannot assign to immutable captured outer variable in a proc `x` +} diff --git a/src/test/compile-fail/unused-mut-warning-captured-var.rs b/src/test/compile-fail/unused-mut-warning-captured-var.rs new file mode 100644 index 0000000000000..a3db84b0ac65f --- /dev/null +++ b/src/test/compile-fail/unused-mut-warning-captured-var.rs @@ -0,0 +1,17 @@ +// 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. + +#![forbid(unused_mut)] + +fn main() { + let mut x = 1i; + //~^ ERROR: variable does not need to be mutable + proc() { println!("{}", x); }; +} diff --git a/src/test/run-pass/issue-11958.rs b/src/test/run-pass/issue-11958.rs new file mode 100644 index 0000000000000..f4ed7c5d9c871 --- /dev/null +++ b/src/test/run-pass/issue-11958.rs @@ -0,0 +1,22 @@ +// 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. + +#![forbid(warnings)] + +// Pretty printing tests complain about `use std::predule::*` +#![allow(unused_imports)] + +// We shouldn't need to rebind a moved upvar as mut if it's already +// marked as mut + +pub fn main() { + let mut x = 1i; + proc() { x = 2; }; +}