Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make the unused_mut lint smarter #26378

Merged
merged 2 commits into from
Jul 3, 2015
Merged

Make the unused_mut lint smarter #26378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f45294
Clean-up assignment checking in borrowck
arielb1 Jun 17, 2015
a18d984
Make the unused_mut lint smarter with respect to locals.
arielb1 Jun 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/libcore/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2655,8 +2655,8 @@ macro_rules! step_impl_signed {
#[allow(trivial_numeric_casts)]
fn steps_between(start: &$t, end: &$t, by: &$t) -> Option<usize> {
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);
Expand Down
2 changes: 1 addition & 1 deletion src/libfmt_macros/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/infer/error_reporting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
212 changes: 19 additions & 193 deletions src/librustc_borrowck/borrowck/check_loans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -789,202 +782,35 @@ 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);

// 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.
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
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);
// 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
});
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");
// 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 {
let lp = opt_loan_path(&assignee_cmt).unwrap();
self.move_data.each_assignment_of(assignment_id, &lp, |assign| {
if assignee_cmt.mutbl.is_mutable() {
self.tcx().used_mut_nodes.borrow_mut().insert(local_id);
} else {
self.bccx.span_err(
self.bccx.report_reassigned_immutable_variable(
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;
&*lp,
assign);
}
}
}

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);
false
});
return
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_borrowck/borrowck/gather_loans/lifetime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
}
Loading