From 2f45294a506904f2768a8f991b0cf33b7cb0bcd2 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 17 Jun 2015 21:54:22 +0300 Subject: [PATCH 1/2] Clean-up assignment checking in borrowck --- src/librustc_borrowck/borrowck/check_loans.rs | 217 +++--------------- .../borrowck/gather_loans/lifetime.rs | 2 +- .../borrowck/gather_loans/mod.rs | 155 ++++++++----- .../borrowck/gather_loans/restrictions.rs | 2 +- src/librustc_borrowck/borrowck/mod.rs | 35 +-- 5 files changed, 142 insertions(+), 269 deletions(-) diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index 9d4fb4c994d..f06dc105d9c 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -565,13 +565,6 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { true } - fn is_local_variable_or_arg(&self, cmt: mc::cmt<'tcx>) -> bool { - match cmt.cat { - mc::cat_local(_) => true, - _ => false - } - } - fn consume_common(&self, id: ast::NodeId, span: Span, @@ -793,198 +786,40 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { mode: euv::MutateMode) { debug!("check_assignment(assignee_cmt={:?})", assignee_cmt); - // Mutable values can be assigned, as long as they obey loans - // and aliasing restrictions: - if assignee_cmt.mutbl.is_mutable() { - if check_for_aliasable_mutable_writes(self, assignment_span, assignee_cmt.clone()) { - if mode != euv::Init { - check_for_assignment_to_borrowed_path( - self, assignment_id, assignment_span, assignee_cmt.clone()); - mark_variable_as_used_mut(self, assignee_cmt); - } - } - - return; - } - - // Initializations are OK if and only if they aren't partial - // reinitialization of a partially-uninitialized structure. + // Initializations never cause borrow errors as they only + // affect a fresh local. if mode == euv::Init { return } - // For immutable local variables, assignments are legal - // if they cannot already have been assigned - if self.is_local_variable_or_arg(assignee_cmt.clone()) { - assert!(assignee_cmt.mutbl.is_immutable()); // no "const" locals + // Check that we don't invalidate any outstanding loans + if let Some(loan_path) = opt_loan_path(&assignee_cmt) { + let scope = region::CodeExtent::from_node_id(assignment_id); + self.each_in_scope_loan_affecting_path(scope, &*loan_path, |loan| { + self.report_illegal_mutation(assignment_span, &*loan_path, loan); + false + }); + } + + // Local variables can always be assigned to, expect for reassignments + // of immutable variables (or assignments that invalidate loans, + // of course). + if let mc::cat_local(local_id) = assignee_cmt.cat { + if assignee_cmt.mutbl.is_mutable() { + self.tcx().used_mut_nodes.borrow_mut().insert(local_id); + } + let lp = opt_loan_path(&assignee_cmt).unwrap(); self.move_data.each_assignment_of(assignment_id, &lp, |assign| { - self.bccx.report_reassigned_immutable_variable( - assignment_span, - &*lp, - assign); - false - }); - return; - } - - // Otherwise, just a plain error. - match assignee_cmt.note { - mc::NoteClosureEnv(upvar_id) => { - // If this is an `Fn` closure, it simply can't mutate upvars. - // If it's an `FnMut` closure, the original variable was declared immutable. - // We need to determine which is the case here. - let kind = match assignee_cmt.upvar().unwrap().cat { - mc::cat_upvar(mc::Upvar { kind, .. }) => kind, - _ => unreachable!() - }; - if kind == ty::FnClosureKind { - self.bccx.span_err( - assignment_span, - &format!("cannot assign to {}", - self.bccx.cmt_to_string(&*assignee_cmt))); - self.bccx.span_help( - self.tcx().map.span(upvar_id.closure_expr_id), - "consider changing this closure to take self by mutable reference"); - } else { - self.bccx.span_err( - assignment_span, - &format!("cannot assign to {} {}", - assignee_cmt.mutbl.to_user_str(), - self.bccx.cmt_to_string(&*assignee_cmt))); - } - } - _ => match opt_loan_path(&assignee_cmt) { - Some(lp) => { - self.bccx.span_err( - assignment_span, - &format!("cannot assign to {} {} `{}`", - assignee_cmt.mutbl.to_user_str(), - self.bccx.cmt_to_string(&*assignee_cmt), - self.bccx.loan_path_to_string(&*lp))); - } - None => { - self.bccx.span_err( - assignment_span, - &format!("cannot assign to {} {}", - assignee_cmt.mutbl.to_user_str(), - self.bccx.cmt_to_string(&*assignee_cmt))); - } - } - } - return; - - fn mark_variable_as_used_mut<'a, 'tcx>(this: &CheckLoanCtxt<'a, 'tcx>, - mut cmt: mc::cmt<'tcx>) { - //! 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. - - loop { - debug!("mark_variable_as_used_mut(cmt={:?})", cmt); - match cmt.cat.clone() { - mc::cat_upvar(mc::Upvar { id: ty::UpvarId { var_id: id, .. }, .. }) | - mc::cat_local(id) => { - this.tcx().used_mut_nodes.borrow_mut().insert(id); - return; - } - - mc::cat_rvalue(..) | - mc::cat_static_item | - mc::cat_deref(_, _, mc::UnsafePtr(..)) | - mc::cat_deref(_, _, mc::Implicit(..)) => { - assert_eq!(cmt.mutbl, mc::McDeclared); - return; - } - - mc::cat_deref(_, _, mc::BorrowedPtr(..)) => { - assert_eq!(cmt.mutbl, mc::McDeclared); - // We need to drill down to upvar if applicable - match cmt.upvar() { - Some(b) => cmt = b, - None => return - } - } - - mc::cat_deref(b, _, mc::Unique) => { - assert_eq!(cmt.mutbl, mc::McInherited); - cmt = b; - } - - mc::cat_downcast(b, _) | - mc::cat_interior(b, _) => { - assert_eq!(cmt.mutbl, mc::McInherited); - cmt = b; - } - } - } - } - - fn check_for_aliasable_mutable_writes<'a, 'tcx>(this: &CheckLoanCtxt<'a, 'tcx>, - span: Span, - cmt: mc::cmt<'tcx>) -> bool { - //! Safety checks related to writes to aliasable, mutable locations - - let guarantor = cmt.guarantor(); - debug!("check_for_aliasable_mutable_writes(cmt={:?}, guarantor={:?})", - cmt, guarantor); - if let mc::cat_deref(ref b, _, mc::BorrowedPtr(ty::MutBorrow, _)) = guarantor.cat { - // Statically prohibit writes to `&mut` when aliasable - check_for_aliasability_violation(this, span, b.clone()); - } - - return true; // no errors reported - } - - fn check_for_aliasability_violation<'a, 'tcx>(this: &CheckLoanCtxt<'a, 'tcx>, - span: Span, - cmt: mc::cmt<'tcx>) - -> bool { - match cmt.freely_aliasable(this.tcx()) { - mc::Aliasability::NonAliasable => { - return true; - } - mc::Aliasability::FreelyAliasable(mc::AliasableStaticMut(..)) => { - return true; - } - mc::Aliasability::ImmutableUnique(_) => { - this.bccx.report_aliasability_violation( - span, - MutabilityViolation, - mc::AliasableReason::UnaliasableImmutable); - return false; - } - mc::Aliasability::FreelyAliasable(cause) => { - this.bccx.report_aliasability_violation( - span, - MutabilityViolation, - cause); - return false; - } - } - } - - fn check_for_assignment_to_borrowed_path<'a, 'tcx>( - this: &CheckLoanCtxt<'a, 'tcx>, - assignment_id: ast::NodeId, - assignment_span: Span, - assignee_cmt: mc::cmt<'tcx>) - { - //! Check for assignments that violate the terms of an - //! outstanding loan. - - let loan_path = match opt_loan_path(&assignee_cmt) { - Some(lp) => lp, - None => { return; /* no loan path, can't be any loans */ } - }; - - let scope = region::CodeExtent::from_node_id(assignment_id); - this.each_in_scope_loan_affecting_path(scope, &*loan_path, |loan| { - this.report_illegal_mutation(assignment_span, &*loan_path, loan); + if !assignee_cmt.mutbl.is_mutable() { + self.bccx.report_reassigned_immutable_variable( + assignment_span, + &*lp, + assign); + } false }); + return } } diff --git a/src/librustc_borrowck/borrowck/gather_loans/lifetime.rs b/src/librustc_borrowck/borrowck/gather_loans/lifetime.rs index 427d78e89b3..919bc45f00d 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/lifetime.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/lifetime.rs @@ -137,7 +137,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> { fn report_error(&self, code: bckerr_code) { self.bccx.report(BckError { cmt: self.cmt_original.clone(), span: self.span, - cause: self.cause, + cause: BorrowViolation(self.cause), code: code }); } } diff --git a/src/librustc_borrowck/borrowck/gather_loans/mod.rs b/src/librustc_borrowck/borrowck/gather_loans/mod.rs index 44a4a0d2504..39b9f076043 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/mod.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/mod.rs @@ -151,22 +151,10 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> { assignee_cmt: mc::cmt<'tcx>, mode: euv::MutateMode) { - let opt_lp = opt_loan_path(&assignee_cmt); - debug!("mutate(assignment_id={}, assignee_cmt={:?}) opt_lp={:?}", - assignment_id, assignee_cmt, opt_lp); - - match opt_lp { - Some(lp) => { - gather_moves::gather_assignment(self.bccx, &self.move_data, - assignment_id, assignment_span, - lp, assignee_cmt.id, mode); - } - None => { - // This can occur with e.g. `*foo() = 5`. In such - // cases, there is no need to check for conflicts - // with moves etc, just ignore. - } - } + self.guarantee_assignment_valid(assignment_id, + assignment_span, + assignee_cmt, + mode); } fn decl_without_init(&mut self, id: ast::NodeId, span: Span) { @@ -177,7 +165,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> { /// Implements the A-* rules in README.md. fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, borrow_span: Span, - loan_cause: euv::LoanCause, + loan_cause: AliasableViolationKind, cmt: mc::cmt<'tcx>, req_kind: ty::BorrowKind) -> Result<(),()> { @@ -203,7 +191,7 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, (mc::Aliasability::ImmutableUnique(_), ty::MutBorrow) => { bccx.report_aliasability_violation( borrow_span, - BorrowViolation(loan_cause), + loan_cause, mc::AliasableReason::UnaliasableImmutable); Err(()) } @@ -211,7 +199,7 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, (mc::Aliasability::FreelyAliasable(alias_cause), ty::MutBorrow) => { bccx.report_aliasability_violation( borrow_span, - BorrowViolation(loan_cause), + loan_cause, alias_cause); Err(()) } @@ -221,9 +209,94 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, } } +/// Implements the M-* rules in README.md. +fn check_mutability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, + borrow_span: Span, + cause: AliasableViolationKind, + cmt: mc::cmt<'tcx>, + req_kind: ty::BorrowKind) + -> Result<(),()> { + debug!("check_mutability(cause={:?} cmt={:?} req_kind={:?}", + cause, cmt, req_kind); + match req_kind { + ty::UniqueImmBorrow | ty::ImmBorrow => { + match cmt.mutbl { + // I am intentionally leaving this here to help + // refactoring if, in the future, we should add new + // kinds of mutability. + mc::McImmutable | mc::McDeclared | mc::McInherited => { + // both imm and mut data can be lent as imm; + // for mutable data, this is a freeze + Ok(()) + } + } + } + + ty::MutBorrow => { + // Only mutable data can be lent as mutable. + if !cmt.mutbl.is_mutable() { + Err(bccx.report(BckError { span: borrow_span, + cause: cause, + cmt: cmt, + code: err_mutbl })) + } else { + Ok(()) + } + } + } +} + impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> { pub fn tcx(&self) -> &'a ty::ctxt<'tcx> { self.bccx.tcx } + /// Guarantees that `cmt` is assignable, or reports an error. + fn guarantee_assignment_valid(&mut self, + assignment_id: ast::NodeId, + assignment_span: Span, + cmt: mc::cmt<'tcx>, + mode: euv::MutateMode) { + + let opt_lp = opt_loan_path(&cmt); + debug!("guarantee_assignment_valid(assignment_id={}, cmt={:?}) opt_lp={:?}", + assignment_id, cmt, opt_lp); + + if let mc::cat_local(..) = cmt.cat { + // Only re-assignments to locals require it to be + // mutable - this is checked in check_loans. + } else { + // Check that we don't allow assignments to non-mutable data. + if check_mutability(self.bccx, assignment_span, MutabilityViolation, + cmt.clone(), ty::MutBorrow).is_err() { + return; // reported an error, no sense in reporting more. + } + } + + // Check that we don't allow assignments to aliasable data + if check_aliasability(self.bccx, assignment_span, MutabilityViolation, + cmt.clone(), ty::MutBorrow).is_err() { + return; // reported an error, no sense in reporting more. + } + + match opt_lp { + Some(lp) => { + if let mc::cat_local(..) = cmt.cat { + // Only re-assignments to locals require it to be + // mutable - this is checked in check_loans. + } else { + self.mark_loan_path_as_mutated(&lp); + } + gather_moves::gather_assignment(self.bccx, &self.move_data, + assignment_id, assignment_span, + lp, cmt.id, mode); + } + None => { + // This can occur with e.g. `*foo() = 5`. In such + // cases, there is no need to check for conflicts + // with moves etc, just ignore. + } + } + } + /// Guarantees that `addr_of(cmt)` will be valid for the duration of `static_scope_r`, or /// reports an error. This may entail taking out loans, which will be added to the /// `req_loan_map`. @@ -256,13 +329,13 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> { } // Check that we don't allow mutable borrows of non-mutable data. - if check_mutability(self.bccx, borrow_span, cause, + if check_mutability(self.bccx, borrow_span, BorrowViolation(cause), cmt.clone(), req_kind).is_err() { return; // reported an error, no sense in reporting more. } // Check that we don't allow mutable borrows of aliasable data. - if check_aliasability(self.bccx, borrow_span, cause, + if check_aliasability(self.bccx, borrow_span, BorrowViolation(cause), cmt.clone(), req_kind).is_err() { return; // reported an error, no sense in reporting more. } @@ -368,43 +441,6 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> { // restrictions: restrictions // } // } - - fn check_mutability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, - borrow_span: Span, - cause: euv::LoanCause, - cmt: mc::cmt<'tcx>, - req_kind: ty::BorrowKind) - -> Result<(),()> { - //! Implements the M-* rules in README.md. - debug!("check_mutability(cause={:?} cmt={:?} req_kind={:?}", - cause, cmt, req_kind); - match req_kind { - ty::UniqueImmBorrow | ty::ImmBorrow => { - match cmt.mutbl { - // I am intentionally leaving this here to help - // refactoring if, in the future, we should add new - // kinds of mutability. - mc::McImmutable | mc::McDeclared | mc::McInherited => { - // both imm and mut data can be lent as imm; - // for mutable data, this is a freeze - Ok(()) - } - } - } - - ty::MutBorrow => { - // Only mutable data can be lent as mutable. - if !cmt.mutbl.is_mutable() { - Err(bccx.report(BckError { span: borrow_span, - cause: cause, - cmt: cmt, - code: err_mutbl })) - } else { - Ok(()) - } - } - } - } } pub fn mark_loan_path_as_mutated(&self, loan_path: &LoanPath) { @@ -495,7 +531,8 @@ impl<'a, 'tcx, 'v> Visitor<'v> for StaticInitializerCtxt<'a, 'tcx> { let base_cmt = mc.cat_expr(&**base).unwrap(); let borrow_kind = ty::BorrowKind::from_mutbl(mutbl); // Check that we don't allow borrows of unsafe static items. - if check_aliasability(self.bccx, ex.span, euv::AddrOf, + if check_aliasability(self.bccx, ex.span, + BorrowViolation(euv::AddrOf), base_cmt, borrow_kind).is_err() { return; // reported an error, no sense in reporting more. } diff --git a/src/librustc_borrowck/borrowck/gather_loans/restrictions.rs b/src/librustc_borrowck/borrowck/gather_loans/restrictions.rs index 345f5378f69..4c186dd8406 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/restrictions.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/restrictions.rs @@ -124,7 +124,7 @@ impl<'a, 'tcx> RestrictionsContext<'a, 'tcx> { self.bccx.report( BckError { span: self.span, - cause: self.cause, + cause: BorrowViolation(self.cause), cmt: cmt_base, code: err_borrowed_pointer_too_short( self.loan_region, lt)}); diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 4f726044a1b..65e3e443e7d 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -546,12 +546,12 @@ pub enum bckerr_code { #[derive(PartialEq)] pub struct BckError<'tcx> { span: Span, - cause: euv::LoanCause, + cause: AliasableViolationKind, cmt: mc::cmt<'tcx>, code: bckerr_code } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug, PartialEq)] pub enum AliasableViolationKind { MutabilityViolation, BorrowViolation(euv::LoanCause) @@ -576,8 +576,10 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { pub fn report(&self, err: BckError<'tcx>) { // Catch and handle some particular cases. match (&err.code, &err.cause) { - (&err_out_of_scope(ty::ReScope(_), ty::ReStatic), &euv::ClosureCapture(span)) | - (&err_out_of_scope(ty::ReScope(_), ty::ReFree(..)), &euv::ClosureCapture(span)) => { + (&err_out_of_scope(ty::ReScope(_), ty::ReStatic), + &BorrowViolation(euv::ClosureCapture(span))) | + (&err_out_of_scope(ty::ReScope(_), ty::ReFree(..)), + &BorrowViolation(euv::ClosureCapture(span))) => { return self.report_out_of_scope_escaping_closure_capture(&err, span); } _ => { } @@ -796,10 +798,6 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { self.tcx.sess.span_end_note(s, m); } - pub fn span_help(&self, s: Span, m: &str) { - self.tcx.sess.span_help(s, m); - } - pub fn fileline_help(&self, s: Span, m: &str) { self.tcx.sess.fileline_help(s, m); } @@ -827,19 +825,22 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { }; match err.cause { - euv::ClosureCapture(_) => { + MutabilityViolation => { + format!("cannot assign to {}", descr) + } + BorrowViolation(euv::ClosureCapture(_)) => { format!("closure cannot assign to {}", descr) } - euv::OverloadedOperator | - euv::AddrOf | - euv::RefBinding | - euv::AutoRef | - euv::AutoUnsafe | - euv::ForLoop | - euv::MatchDiscriminant => { + BorrowViolation(euv::OverloadedOperator) | + BorrowViolation(euv::AddrOf) | + BorrowViolation(euv::RefBinding) | + BorrowViolation(euv::AutoRef) | + BorrowViolation(euv::AutoUnsafe) | + BorrowViolation(euv::ForLoop) | + BorrowViolation(euv::MatchDiscriminant) => { format!("cannot borrow {} as mutable", descr) } - euv::ClosureInvocation => { + BorrowViolation(euv::ClosureInvocation) => { self.tcx.sess.span_bug(err.span, "err_mutbl with a closure invocation"); } From a18d9842ed94ecca3e7161945bb2f749d98d18ee Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 18 Jun 2015 01:02:58 +0300 Subject: [PATCH 2/2] Make the unused_mut lint smarter with respect to locals. Fixes #26332 --- src/libcore/iter.rs | 4 +-- src/libfmt_macros/lib.rs | 2 +- src/librustc/middle/infer/error_reporting.rs | 2 +- src/librustc_borrowck/borrowck/check_loans.rs | 25 ++++++------------- src/librustc_trans/save/dump_csv.rs | 2 +- src/libsyntax/parse/lexer/mod.rs | 2 +- .../compile-fail/lint-unused-mut-variables.rs | 9 +++++++ 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libcore/iter.rs b/src/libcore/iter.rs index 3026f91e853..4c8511eb190 100644 --- a/src/libcore/iter.rs +++ b/src/libcore/iter.rs @@ -2655,8 +2655,8 @@ macro_rules! step_impl_signed { #[allow(trivial_numeric_casts)] fn steps_between(start: &$t, end: &$t, by: &$t) -> Option { if *by == 0 { return None; } - let mut diff: usize; - let mut by_u: usize; + let diff: usize; + let by_u: usize; if *by > 0 { if *start >= *end { return Some(0); diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index c2b28bd134d..7ca89cfd0c9 100644 --- a/src/libfmt_macros/lib.rs +++ b/src/libfmt_macros/lib.rs @@ -399,7 +399,7 @@ impl<'a> Parser<'a> { } Some(..) | None => { return &self.input[..0]; } }; - let mut end; + let end; loop { match self.cur.clone().next() { Some((_, c)) if c.is_xid_continue() => { diff --git a/src/librustc/middle/infer/error_reporting.rs b/src/librustc/middle/infer/error_reporting.rs index 17075c0cba6..6d5b47d8ed9 100644 --- a/src/librustc/middle/infer/error_reporting.rs +++ b/src/librustc/middle/infer/error_reporting.rs @@ -1854,7 +1854,7 @@ impl LifeGiver { } fn give_lifetime(&self) -> ast::Lifetime { - let mut lifetime; + let lifetime; loop { let mut s = String::from("'"); s.push_str(&num_to_string(self.counter.get())); diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index f06dc105d9c..237add6ff86 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -182,7 +182,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for CheckLoanCtxt<'a, 'tcx> { None => { } } - self.check_assignment(assignment_id, assignment_span, assignee_cmt, mode); + self.check_assignment(assignment_id, assignment_span, assignee_cmt); } fn decl_without_init(&mut self, _id: ast::NodeId, _span: Span) { } @@ -782,16 +782,9 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { fn check_assignment(&self, assignment_id: ast::NodeId, assignment_span: Span, - assignee_cmt: mc::cmt<'tcx>, - mode: euv::MutateMode) { + assignee_cmt: mc::cmt<'tcx>) { debug!("check_assignment(assignee_cmt={:?})", assignee_cmt); - // Initializations never cause borrow errors as they only - // affect a fresh local. - if mode == euv::Init { - return - } - // Check that we don't invalidate any outstanding loans if let Some(loan_path) = opt_loan_path(&assignee_cmt) { let scope = region::CodeExtent::from_node_id(assignment_id); @@ -801,17 +794,15 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { }); } - // Local variables can always be assigned to, expect for reassignments - // of immutable variables (or assignments that invalidate loans, - // of course). + // Check for reassignments to (immutable) local variables. This + // needs to be done here instead of in check_loans because we + // depend on move data. if let mc::cat_local(local_id) = assignee_cmt.cat { - if assignee_cmt.mutbl.is_mutable() { - self.tcx().used_mut_nodes.borrow_mut().insert(local_id); - } - let lp = opt_loan_path(&assignee_cmt).unwrap(); self.move_data.each_assignment_of(assignment_id, &lp, |assign| { - if !assignee_cmt.mutbl.is_mutable() { + if assignee_cmt.mutbl.is_mutable() { + self.tcx().used_mut_nodes.borrow_mut().insert(local_id); + } else { self.bccx.report_reassigned_immutable_variable( assignment_span, &*lp, diff --git a/src/librustc_trans/save/dump_csv.rs b/src/librustc_trans/save/dump_csv.rs index d86242f39ce..f747f2deec4 100644 --- a/src/librustc_trans/save/dump_csv.rs +++ b/src/librustc_trans/save/dump_csv.rs @@ -308,7 +308,7 @@ impl <'l, 'tcx> DumpCsvVisitor<'l, 'tcx> { debug!("process_method: {}:{}", id, token::get_name(name)); - let mut scope_id; + let scope_id; // The qualname for a method is the trait name or name of the struct in an impl in // which the method is declared in, followed by the method's name. let qualname = match self.tcx.impl_of_method(ast_util::local_def(id)) { diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index b6b5ac5c01e..507bd9de2a1 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -598,7 +598,7 @@ impl<'a> StringReader<'a> { /// Lex a LIT_INTEGER or a LIT_FLOAT fn scan_number(&mut self, c: char) -> token::Lit { - let mut num_digits; + let num_digits; let mut base = 10; let start_bpos = self.last_pos; diff --git a/src/test/compile-fail/lint-unused-mut-variables.rs b/src/test/compile-fail/lint-unused-mut-variables.rs index dcc82b8920f..8165dd0fa29 100644 --- a/src/test/compile-fail/lint-unused-mut-variables.rs +++ b/src/test/compile-fail/lint-unused-mut-variables.rs @@ -23,6 +23,15 @@ fn main() { let mut b = 3; //~ ERROR: variable does not need to be mutable let mut a = vec!(3); //~ ERROR: variable does not need to be mutable let (mut a, b) = (1, 2); //~ ERROR: variable does not need to be mutable + let mut a; //~ ERROR: variable does not need to be mutable + a = 3; + + let mut b; //~ ERROR: variable does not need to be mutable + if true { + b = 3; + } else { + b = 4; + } match 30 { mut x => {} //~ ERROR: variable does not need to be mutable