user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Clean-up assignment checking in borrowck

Browse source
This commit is contained in:
Ariel Ben-Yehuda 2015-06-17 21:54:22 +03:00 • committed by Ariel Ben-Yehuda
parent 40db46c6ba
commit 2f45294a50
5 changed files with 142 additions and 269 deletions
Download patch file Download diff file Expand all files Collapse all files

217
src/librustc_borrowck/borrowck/check_loans.rs
View file

@ -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
}
}

2
src/librustc_borrowck/borrowck/gather_loans/lifetime.rs
View file

@ -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 });
}
}

155
src/librustc_borrowck/borrowck/gather_loans/mod.rs
View file

@ -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.
}

2
src/librustc_borrowck/borrowck/gather_loans/restrictions.rs
View file

@ -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)});

35
src/librustc_borrowck/borrowck/mod.rs
View file

@ -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");
}