Skip to content

Commit

Permalink
auto merge of #17434 : P1start/rust/borrowck-messages, r=nikomatsakis
Browse files Browse the repository at this point in the history 
This was originally part of #17215.

Closes #15506.
Closes #15630.
Closes #17263.

This also partially implements #15838.
  • Loading branch information
@bors
bors committed Oct 2, 2014
2 parents dd7f00d + 02c6ebd commit 84a4a07
Show file tree
Hide file tree
Showing 12 changed files with 299 additions and 72 deletions.
84 changes: 57 additions & 27 deletions src/librustc/middle/borrowck/check_loans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,50 +400,82 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
for restr_path in loan1.restricted_paths.iter() {
if *restr_path != loan2_base_path { continue; }

let old_pronoun = if new_loan.loan_path == old_loan.loan_path {
// If new_loan is something like `x.a`, and old_loan is something like `x.b`, we would
// normally generate a rather confusing message (in this case, for multiple mutable
// borrows):
//
// error: cannot borrow `x.b` as mutable more than once at a time
// note: previous borrow of `x.a` occurs here; the mutable borrow prevents
// subsequent moves, borrows, or modification of `x.a` until the borrow ends
//
// What we want to do instead is get the 'common ancestor' of the two borrow paths and
// use that for most of the message instead, giving is something like this:
//
// error: cannot borrow `x` as mutable more than once at a time
// note: previous borrow of `x` occurs here (through borrowing `x.a`); the mutable
// borrow prevents subsequent moves, borrows, or modification of `x` until the
// borrow ends

let common = new_loan.loan_path.common(&*old_loan.loan_path);
let (nl, ol, new_loan_msg, old_loan_msg) =
if new_loan.loan_path.has_fork(&*old_loan.loan_path) && common.is_some() {
let nl = self.bccx.loan_path_to_string(&common.unwrap());
let ol = nl.clone();
let new_loan_msg = format!(" (here through borrowing `{}`)",
self.bccx.loan_path_to_string(
&*new_loan.loan_path));
let old_loan_msg = format!(" (through borrowing `{}`)",
self.bccx.loan_path_to_string(
&*old_loan.loan_path));
(nl, ol, new_loan_msg, old_loan_msg)
} else {
(self.bccx.loan_path_to_string(&*new_loan.loan_path),
self.bccx.loan_path_to_string(&*old_loan.loan_path),
String::new(), String::new())
};

let ol_pronoun = if new_loan.loan_path == old_loan.loan_path {
"it".to_string()
} else {
format!("`{}`",
self.bccx.loan_path_to_string(&*old_loan.loan_path))
format!("`{}`", ol)
};

match (new_loan.kind, old_loan.kind) {
(ty::MutBorrow, ty::MutBorrow) => {
self.bccx.span_err(
new_loan.span,
format!("cannot borrow `{}` as mutable \
format!("cannot borrow `{}`{} as mutable \
more than once at a time",
self.bccx.loan_path_to_string(
&*new_loan.loan_path)).as_slice());
nl, new_loan_msg).as_slice())
}

(ty::UniqueImmBorrow, _) => {
self.bccx.span_err(
new_loan.span,
format!("closure requires unique access to `{}` \
but {} is already borrowed",
self.bccx.loan_path_to_string(&*new_loan.loan_path),
old_pronoun).as_slice());
but {} is already borrowed{}",
nl, ol_pronoun, old_loan_msg).as_slice());
}

(_, ty::UniqueImmBorrow) => {
self.bccx.span_err(
new_loan.span,
format!("cannot borrow `{}` as {} because \
format!("cannot borrow `{}`{} as {} because \
previous closure requires unique access",
self.bccx.loan_path_to_string(&*new_loan.loan_path),
new_loan.kind.to_user_str()).as_slice());
nl, new_loan_msg, new_loan.kind.to_user_str()).as_slice());
}

(_, _) => {
self.bccx.span_err(
new_loan.span,
format!("cannot borrow `{}` as {} because \
{} is also borrowed as {}",
self.bccx.loan_path_to_string(&*new_loan.loan_path),
format!("cannot borrow `{}`{} as {} because \
{} is also borrowed as {}{}",
nl,
new_loan_msg,
new_loan.kind.to_user_str(),
old_pronoun,
old_loan.kind.to_user_str()).as_slice());
ol_pronoun,
old_loan.kind.to_user_str(),
old_loan_msg).as_slice());
}
}

Expand All @@ -452,8 +484,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
self.bccx.span_note(
span,
format!("borrow occurs due to use of `{}` in closure",
self.bccx.loan_path_to_string(
&*new_loan.loan_path)).as_slice());
nl).as_slice());
}
_ => { }
}
Expand All @@ -463,30 +494,29 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
format!("the mutable borrow prevents subsequent \
moves, borrows, or modification of `{0}` \
until the borrow ends",
self.bccx.loan_path_to_string(
&*old_loan.loan_path))
ol)
}

ty::ImmBorrow => {
format!("the immutable borrow prevents subsequent \
moves or mutable borrows of `{0}` \
until the borrow ends",
self.bccx.loan_path_to_string(&*old_loan.loan_path))
ol)
}

ty::UniqueImmBorrow => {
format!("the unique capture prevents subsequent \
moves or borrows of `{0}` \
until the borrow ends",
self.bccx.loan_path_to_string(&*old_loan.loan_path))
ol)
}
};

let borrow_summary = match old_loan.cause {
euv::ClosureCapture(_) => {
format!("previous borrow of `{}` occurs here due to \
format!("previous borrow of `{}` occurs here{} due to \
use in closure",
self.bccx.loan_path_to_string(&*old_loan.loan_path))
ol, old_loan_msg)
}

euv::OverloadedOperator(..) |
Expand All @@ -496,8 +526,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
euv::ForLoop(..) |
euv::RefBinding(..) |
euv::MatchDiscriminant(..) => {
format!("previous borrow of `{}` occurs here",
self.bccx.loan_path_to_string(&*old_loan.loan_path))
format!("previous borrow of `{}` occurs here{}",
ol, old_loan_msg)
}
};

Expand Down
114 changes: 102 additions & 12 deletions src/librustc/middle/borrowck/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,51 @@ impl LoanPath {
LpExtend(ref base, _, _) => base.kill_scope(tcx),
}
}

fn has_fork(&self, other: &LoanPath) -> bool {
match (self, other) {
(&LpExtend(ref base, _, LpInterior(id)), &LpExtend(ref base2, _, LpInterior(id2))) =>
if id == id2 {
base.has_fork(&**base2)
} else {
true
},
(&LpExtend(ref base, _, LpDeref(_)), _) => base.has_fork(other),
(_, &LpExtend(ref base, _, LpDeref(_))) => self.has_fork(&**base),
_ => false,
}
}

fn depth(&self) -> uint {
match *self {
LpExtend(ref base, _, LpDeref(_)) => base.depth(),
LpExtend(ref base, _, LpInterior(_)) => base.depth() + 1,
_ => 0,
}
}

fn common(&self, other: &LoanPath) -> Option<LoanPath> {
match (self, other) {
(&LpExtend(ref base, a, LpInterior(id)), &LpExtend(ref base2, _, LpInterior(id2))) =>
if id == id2 {
base.common(&**base2).map(|x| {
let xd = x.depth();
if base.depth() == xd && base2.depth() == xd {
LpExtend(Rc::new(x), a, LpInterior(id))
} else {
x
}
})
} else {
base.common(&**base2)
},
(&LpExtend(ref base, _, LpDeref(_)), _) => base.common(other),
(_, &LpExtend(ref other, _, LpDeref(_))) => self.common(&**other),
(&LpVar(id), &LpVar(id2)) => if id == id2 { Some(LpVar(id)) } else { None },
(&LpUpvar(id), &LpUpvar(id2)) => if id == id2 { Some(LpUpvar(id)) } else { None },
_ => None,
}
}
}

pub fn opt_loan_path(cmt: &mc::cmt) -> Option<Rc<LoanPath>> {
Expand Down Expand Up @@ -416,24 +461,58 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
MovedInCapture => "capture",
};

match the_move.kind {
let (ol, moved_lp_msg) = match the_move.kind {
move_data::Declared => {
self.tcx.sess.span_err(
use_span,
format!("{} of possibly uninitialized variable: `{}`",
verb,
self.loan_path_to_string(lp)).as_slice());
(self.loan_path_to_string(moved_lp),
String::new())
}
_ => {
let partially = if lp == moved_lp {""} else {"partially "};
// If moved_lp is something like `x.a`, and lp is something like `x.b`, we would
// normally generate a rather confusing message:
//
// error: use of moved value: `x.b`
// note: `x.a` moved here...
//
// What we want to do instead is get the 'common ancestor' of the two moves and
// use that for most of the message instead, giving is something like this:
//
// error: use of moved value: `x`
// note: `x` moved here (through moving `x.a`)...

let common = moved_lp.common(lp);
let has_common = common.is_some();
let has_fork = moved_lp.has_fork(lp);
let (nl, ol, moved_lp_msg) =
if has_fork && has_common {
let nl = self.loan_path_to_string(&common.unwrap());
let ol = nl.clone();
let moved_lp_msg = format!(" (through moving `{}`)",
self.loan_path_to_string(moved_lp));
(nl, ol, moved_lp_msg)
} else {
(self.loan_path_to_string(lp),
self.loan_path_to_string(moved_lp),
String::new())
};

let partial = moved_lp.depth() > lp.depth();
let msg = if !has_fork && partial { "partially " }
else if has_fork && !has_common { "collaterally "}
else { "" };
self.tcx.sess.span_err(
use_span,
format!("{} of {}moved value: `{}`",
verb,
partially,
self.loan_path_to_string(lp)).as_slice());
msg,
nl).as_slice());
(ol, moved_lp_msg)
}
}
};

match the_move.kind {
move_data::Declared => {}
Expand All @@ -456,19 +535,21 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
"moved by default (use `copy` to override)");
self.tcx.sess.span_note(
expr_span,
format!("`{}` moved here because it has type `{}`, which is {}",
self.loan_path_to_string(moved_lp),
format!("`{}` moved here{} because it has type `{}`, which is {}",
ol,
moved_lp_msg,
expr_ty.user_string(self.tcx),
suggestion).as_slice());
}

move_data::MovePat => {
let pat_ty = ty::node_id_to_type(self.tcx, the_move.id);
self.tcx.sess.span_note(self.tcx.map.span(the_move.id),
format!("`{}` moved here because it has type `{}`, \
format!("`{}` moved here{} because it has type `{}`, \
which is moved by default (use `ref` to \
override)",
self.loan_path_to_string(moved_lp),
ol,
moved_lp_msg,
pat_ty.user_string(self.tcx)).as_slice());
}

Expand All @@ -491,9 +572,10 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
capture that instead to override)");
self.tcx.sess.span_note(
expr_span,
format!("`{}` moved into closure environment here because it \
format!("`{}` moved into closure environment here{} because it \
has type `{}`, which is {}",
self.loan_path_to_string(moved_lp),
ol,
moved_lp_msg,
expr_ty.user_string(self.tcx),
suggestion).as_slice());
}
Expand Down Expand Up @@ -602,6 +684,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
span: Span,
kind: AliasableViolationKind,
cause: mc::AliasableReason) {
let mut is_closure = false;
let prefix = match kind {
MutabilityViolation => {
"cannot assign to data"
Expand All @@ -625,6 +708,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}

BorrowViolation(euv::ClosureInvocation) => {
is_closure = true;
"closure invocation"
}

Expand All @@ -649,14 +733,20 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
mc::AliasableManaged => {
self.tcx.sess.span_err(
span,
format!("{} in a `@` pointer", prefix).as_slice());
format!("{} in a `Gc` pointer", prefix).as_slice());
}
mc::AliasableBorrowed => {
self.tcx.sess.span_err(
span,
format!("{} in a `&` reference", prefix).as_slice());
}
}

if is_closure {
self.tcx.sess.span_note(
span,
"closures behind references must be called via `&mut`");
}
}

pub fn note_and_explain_bckerr(&self, err: BckError) {
Expand Down
Loading

0 comments on commit 84a4a07

Please sign in to comment.