Skip to content

Commit 35ff2de

Browse files
ftxqxdftxqxd
committed
Clarify some borrowck errors
Closes #17263.
1 parent 36b8502 commit 35ff2de

File tree

3 files changed

+125
-27
lines changed

3 files changed

+125
-27
lines changed

‎src/librustc/middle/borrowck/check_loans.rs

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -400,50 +400,82 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
400400
for restr_path in loan1.restricted_paths.iter() {
401401
if *restr_path != loan2_base_path { continue; }
402402

403-
let old_pronoun = if new_loan.loan_path == old_loan.loan_path {
403+
// If new_loan is something like `x.a`, and old_loan is something like `x.b`, we would
404+
// normally generate a rather confusing message (in this case, for multiple mutable
405+
// borrows):
406+
//
407+
// error: cannot borrow `x.b` as mutable more than once at a time
408+
// note: previous borrow of `x.a` occurs here; the mutable borrow prevents
409+
// subsequent moves, borrows, or modification of `x.a` until the borrow ends
410+
//
411+
// What we want to do instead is get the 'common ancestor' of the two borrow paths and
412+
// use that for most of the message instead, giving is something like this:
413+
//
414+
// error: cannot borrow `x` as mutable more than once at a time
415+
// note: previous borrow of `x` occurs here (through borrowing `x.a`); the mutable
416+
// borrow prevents subsequent moves, borrows, or modification of `x` until the
417+
// borrow ends
418+
419+
let common = new_loan.loan_path.common(&*old_loan.loan_path);
420+
let (nl, ol, new_loan_msg, old_loan_msg) =
421+
if new_loan.loan_path.has_fork(&*old_loan.loan_path) && common.is_some() {
422+
let nl = self.bccx.loan_path_to_string(&common.unwrap());
423+
let ol = nl.clone();
424+
let new_loan_msg = format!(" (here through borrowing `{}`)",
425+
self.bccx.loan_path_to_string(
426+
&*new_loan.loan_path));
427+
let old_loan_msg = format!(" (through borrowing `{}`)",
428+
self.bccx.loan_path_to_string(
429+
&*old_loan.loan_path));
430+
(nl, ol, new_loan_msg, old_loan_msg)
431+
} else {
432+
(self.bccx.loan_path_to_string(&*new_loan.loan_path),
433+
self.bccx.loan_path_to_string(&*old_loan.loan_path),
434+
String::new(), String::new())
435+
};
436+
437+
let ol_pronoun = if new_loan.loan_path == old_loan.loan_path {
404438
"it".to_string()
405439
} else {
406-
format!("`{}`",
407-
self.bccx.loan_path_to_string(&*old_loan.loan_path))
440+
format!("`{}`", ol)
408441
};
409442

410443
match (new_loan.kind, old_loan.kind) {
411444
(ty::MutBorrow, ty::MutBorrow) => {
412445
self.bccx.span_err(
413446
new_loan.span,
414-
format!("cannot borrow `{}` as mutable \
447+
format!("cannot borrow `{}`{} as mutable \
415448
more than once at a time",
416-
self.bccx.loan_path_to_string(
417-
&*new_loan.loan_path)).as_slice());
449+
nl, new_loan_msg).as_slice())
418450
}
419451

420452
(ty::UniqueImmBorrow, _) => {
421453
self.bccx.span_err(
422454
new_loan.span,
423455
format!("closure requires unique access to `{}` \
424-
but {} is already borrowed",
425-
self.bccx.loan_path_to_string(&*new_loan.loan_path),
426-
old_pronoun).as_slice());
456+
but {} is already borrowed{}",
457+
nl, ol_pronoun, old_loan_msg).as_slice());
427458
}
428459

429460
(_, ty::UniqueImmBorrow) => {
430461
self.bccx.span_err(
431462
new_loan.span,
432-
format!("cannot borrow `{}` as {} because \
463+
format!("cannot borrow `{}`{} as {} because \
433464
previous closure requires unique access",
434-
self.bccx.loan_path_to_string(&*new_loan.loan_path),
435-
new_loan.kind.to_user_str()).as_slice());
465+
nl, new_loan_msg, new_loan.kind.to_user_str()).as_slice());
436466
}
437467

438468
(_, _) => {
439469
self.bccx.span_err(
440470
new_loan.span,
441-
format!("cannot borrow `{}` as {} because \
442-
{} is also borrowed as {}",
443-
self.bccx.loan_path_to_string(&*new_loan.loan_path),
471+
format!("cannot borrow `{}`{} as {} because \
472+
{} is also borrowed as {}{}",
473+
nl,
474+
new_loan_msg,
444475
new_loan.kind.to_user_str(),
445-
old_pronoun,
446-
old_loan.kind.to_user_str()).as_slice());
476+
ol_pronoun,
477+
old_loan.kind.to_user_str(),
478+
old_loan_msg).as_slice());
447479
}
448480
}
449481

@@ -452,8 +484,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
452484
self.bccx.span_note(
453485
span,
454486
format!("borrow occurs due to use of `{}` in closure",
455-
self.bccx.loan_path_to_string(
456-
&*new_loan.loan_path)).as_slice());
487+
nl).as_slice());
457488
}
458489
_ => { }
459490
}
@@ -463,30 +494,29 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
463494
format!("the mutable borrow prevents subsequent \
464495
moves, borrows, or modification of `{0}` \
465496
until the borrow ends",
466-
self.bccx.loan_path_to_string(
467-
&*old_loan.loan_path))
497+
ol)
468498
}
469499

470500
ty::ImmBorrow => {
471501
format!("the immutable borrow prevents subsequent \
472502
moves or mutable borrows of `{0}` \
473503
until the borrow ends",
474-
self.bccx.loan_path_to_string(&*old_loan.loan_path))
504+
ol)
475505
}
476506

477507
ty::UniqueImmBorrow => {
478508
format!("the unique capture prevents subsequent \
479509
moves or borrows of `{0}` \
480510
until the borrow ends",
481-
self.bccx.loan_path_to_string(&*old_loan.loan_path))
511+
ol)
482512
}
483513
};
484514

485515
let borrow_summary = match old_loan.cause {
486516
euv::ClosureCapture(_) => {
487-
format!("previous borrow of `{}` occurs here due to \
517+
format!("previous borrow of `{}` occurs here{} due to \
488518
use in closure",
489-
self.bccx.loan_path_to_string(&*old_loan.loan_path))
519+
ol, old_loan_msg)
490520
}
491521

492522
euv::OverloadedOperator(..) |
@@ -496,8 +526,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
496526
euv::ForLoop(..) |
497527
euv::RefBinding(..) |
498528
euv::MatchDiscriminant(..) => {
499-
format!("previous borrow of `{}` occurs here",
500-
self.bccx.loan_path_to_string(&*old_loan.loan_path))
529+
format!("previous borrow of `{}` occurs here{}",
530+
ol, old_loan_msg)
501531
}
502532
};
503533

‎src/librustc/middle/borrowck/mod.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,51 @@ impl LoanPath {
298298
LpExtend(ref base, _, _) => base.kill_scope(tcx),
299299
}
300300
}
301+
302+
fn has_fork(&self, other: &LoanPath) -> bool {
303+
match (self, other) {
304+
(&LpExtend(ref base, _, LpInterior(id)), &LpExtend(ref base2, _, LpInterior(id2))) =>
305+
if id == id2 {
306+
base.has_fork(&**base2)
307+
} else {
308+
true
309+
},
310+
(&LpExtend(ref base, _, LpDeref(_)), _) => base.has_fork(other),
311+
(_, &LpExtend(ref base, _, LpDeref(_))) => self.has_fork(&**base),
312+
_ => false,
313+
}
314+
}
315+
316+
fn depth(&self) -> uint {
317+
match *self {
318+
LpExtend(ref base, _, LpDeref(_)) => base.depth(),
319+
LpExtend(ref base, _, LpInterior(_)) => base.depth() + 1,
320+
_ => 0,
321+
}
322+
}
323+
324+
fn common(&self, other: &LoanPath) -> Option<LoanPath> {
325+
match (self, other) {
326+
(&LpExtend(ref base, a, LpInterior(id)), &LpExtend(ref base2, _, LpInterior(id2))) =>
327+
if id == id2 {
328+
base.common(&**base2).map(|x| {
329+
let xd = x.depth();
330+
if base.depth() == xd && base2.depth() == xd {
331+
LpExtend(Rc::new(x), a, LpInterior(id))
332+
} else {
333+
x
334+
}
335+
})
336+
} else {
337+
base.common(&**base2)
338+
},
339+
(&LpExtend(ref base, _, LpDeref(_)), _) => base.common(other),
340+
(_, &LpExtend(ref other, _, LpDeref(_))) => self.common(&**other),
341+
(&LpVar(id), &LpVar(id2)) => if id == id2 { Some(LpVar(id)) } else { None },
342+
(&LpUpvar(id), &LpUpvar(id2)) => if id == id2 { Some(LpUpvar(id)) } else { None },
343+
_ => None,
344+
}
345+
}
301346
}
302347

303348
pub fn opt_loan_path(cmt: &mc::cmt) -> Option<Rc<LoanPath>> {

‎src/test/compile-fail/issue-17263.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
struct Foo { a: int, b: int }
12+
13+
fn main() {
14+
let mut x = box Foo { a: 1, b: 2 };
15+
let (a, b) = (&mut x.a, &mut x.b);
16+
//~^ ERROR cannot borrow `x` (here through borrowing `x.b`) as mutable more than once at a time
17+
//~^^ NOTE previous borrow of `x` occurs here (through borrowing `x.a`)
18+
19+
let mut foo = box Foo { a: 1, b: 2 };
20+
let (c, d) = (&mut foo.a, &foo.b);
21+
//~^ ERROR cannot borrow `foo` (here through borrowing `foo.b`) as immutable
22+
//~^^ NOTE previous borrow of `foo` occurs here (through borrowing `foo.a`)
23+
}

0 commit comments

Comments
 (0)