Make some borrow checker errors more user friendly

src/librustc/driver/session.rs

@@ -248,6 +248,9 @@ impl Session_ {
     pub fn span_note(&self, sp: Span, msg: &str) {
         self.span_diagnostic.span_note(sp, msg)
     }
+    pub fn span_end_note(&self, sp: Span, msg: &str) {
+        self.span_diagnostic.span_end_note(sp, msg)
+    }
     pub fn note(&self, msg: &str) {
         self.span_diagnostic.handler().note(msg)
     }

src/librustc/middle/borrowck/check_loans.rs

@@ -231,34 +231,47 @@ impl<'a> CheckLoanCtxt<'a> {
             if restr.loan_path != loan2.loan_path { continue; }
 
             match (new_loan.mutbl, old_loan.mutbl) {
-                (MutableMutability, MutableMutability) => {
+                (_, MutableMutability) => {
+                    let var = self.bccx.loan_path_to_str(new_loan.loan_path);
                     self.bccx.span_err(
                         new_loan.span,
-                        format!("cannot borrow `{}` as mutable \
-                              more than once at a time",
-                             self.bccx.loan_path_to_str(new_loan.loan_path)));
+                        format!("cannot borrow `{}` because it is already \
+                                 borrowed as mutable", var));
                     self.bccx.span_note(
                         old_loan.span,
-                        format!("previous borrow of `{}` as mutable occurs here",
-                             self.bccx.loan_path_to_str(new_loan.loan_path)));
-                    return false;
+                        format!("previous borrow of `{0}` as mutable occurs \
+                                 here; the mutable borrow prevents subsequent \
+                                 moves, borrows, or modification of `{0}` \
+                                 until the borrow ends", var));
                 }
 
-                _ => {
+                (_, mutability) => {
                     self.bccx.span_err(
                         new_loan.span,
                         format!("cannot borrow `{}` as {} because \
-                              it is also borrowed as {}",
+                              it is already borrowed as {}",
                              self.bccx.loan_path_to_str(new_loan.loan_path),
                              self.bccx.mut_to_str(new_loan.mutbl),
                              self.bccx.mut_to_str(old_loan.mutbl)));
-                    self.bccx.span_note(
-                        old_loan.span,
-                        format!("previous borrow of `{}` occurs here",
-                             self.bccx.loan_path_to_str(new_loan.loan_path)));
-                    return false;
+
+                    let var = self.bccx.loan_path_to_str(new_loan.loan_path);
+                    let mut note = format!("previous borrow of `{}` occurs \
+                                            here", var);
+                    if mutability == ImmutableMutability {
+                        note.push_str(format!("; the immutable borrow prevents \
+                                               subsequent moves or mutable
+                                               borrows of `{}` until the
+                                               borrow ends", var));
+                    }
+                    self.bccx.span_note(old_loan.span, note);
                 }
             }
+
+            let old_loan_span = ast_map::node_span(self.tcx().items,
+                                                   old_loan.kill_scope);
+            self.bccx.span_end_note(old_loan_span,
+                                    "previous borrow ends here");
+            return false;
         }
 
         true

src/librustc/middle/borrowck/mod.rs

@@ -631,6 +631,10 @@ impl BorrowckCtxt {
         self.tcx.sess.span_note(s, m);
     }
 
+    pub fn span_end_note(&self, s: Span, m: &str) {
+        self.tcx.sess.span_end_note(s, m);
+    }
+
     pub fn bckerr_to_str(&self, err: BckError) -> ~str {
         match err.code {
             err_mutbl(lk) => {

src/libsyntax/codemap.rs

@@ -311,10 +311,6 @@ impl CodeMap {
         }
     }
 
-    pub fn adjust_span(&self, sp: Span) -> Span {
-        sp
-    }
-
     pub fn span_to_str(&self, sp: Span) -> ~str {
         {
             let files = self.files.borrow();

src/libsyntax/diagnostic.rs

@@ -19,12 +19,16 @@ use extra::term;
 
 static BUG_REPORT_URL: &'static str =
     "http://static.rust-lang.org/doc/master/complement-bugreport.html";
+// maximum number of lines we will print for each error; arbitrary.
+static MAX_LINES: uint = 6u;
 
 pub trait Emitter {
     fn emit(&self,
             cmsp: Option<(&codemap::CodeMap, Span)>,
             msg: &str,
             lvl: Level);
+    fn custom_emit(&self, cm: &codemap::CodeMap,
+                   sp: Span, msg: &str, lvl: Level);
 }
 
 /// This structure is used to signify that a task has failed with a fatal error
@@ -55,6 +59,9 @@ impl SpanHandler {
     pub fn span_note(@self, sp: Span, msg: &str) {
         self.handler.emit(Some((&*self.cm, sp)), msg, Note);
     }
+    pub fn span_end_note(@self, sp: Span, msg: &str) {
+        self.handler.custom_emit(&*self.cm, sp, msg, Note);
+    }
     pub fn span_bug(@self, sp: Span, msg: &str) -> ! {
         self.span_fatal(sp, ice_msg(msg));
     }
@@ -122,6 +129,10 @@ impl Handler {
             lvl: Level) {
         self.emit.emit(cmsp, msg, lvl);
     }
+    pub fn custom_emit(@self, cm: &codemap::CodeMap,
+                       sp: Span, msg: &str, lvl: Level) {
+        self.emit.custom_emit(cm, sp, msg, lvl);
+    }
 }
 
 pub fn ice_msg(msg: &str) -> ~str {
@@ -239,17 +250,34 @@ impl Emitter for DefaultEmitter {
             msg: &str,
             lvl: Level) {
         match cmsp {
-            Some((cm, sp)) => {
-                let sp = cm.adjust_span(sp);
-                let ss = cm.span_to_str(sp);
-                let lines = cm.span_to_lines(sp);
-                print_diagnostic(ss, lvl, msg);
-                highlight_lines(cm, sp, lvl, lines);
-                print_macro_backtrace(cm, sp);
-            }
+            Some((cm, sp)) => emit(cm, sp, msg, lvl, false),
             None => print_diagnostic("", lvl, msg),
         }
     }
+
+    fn custom_emit(&self, cm: &codemap::CodeMap,
+                   sp: Span, msg: &str, lvl: Level) {
+        emit(cm, sp, msg, lvl, true);
+    }
+}
+
+fn emit(cm: &codemap::CodeMap, sp: Span,
+        msg: &str, lvl: Level, custom: bool) {
+    let ss = cm.span_to_str(sp);
+    let lines = cm.span_to_lines(sp);
+    if custom {
+        // we want to tell compiletest/runtest to look at the last line of the
+        // span (since `custom_highlight_lines` displays an arrow to the end of
+        // the span)
+        let span_end = Span { lo: sp.hi, hi: sp.hi, expn_info: sp.expn_info};
+        let ses = cm.span_to_str(span_end);
+        print_diagnostic(ses, lvl, msg);
+        custom_highlight_lines(cm, sp, lvl, lines);
+    } else {
+        print_diagnostic(ss, lvl, msg);
+        highlight_lines(cm, sp, lvl, lines);
+    }
+    print_macro_backtrace(cm, sp);
 }
 
 fn highlight_lines(cm: &codemap::CodeMap,
@@ -260,12 +288,10 @@ fn highlight_lines(cm: &codemap::CodeMap,
     let mut err = io::stderr();
     let err = &mut err as &mut io::Writer;
 
-    // arbitrarily only print up to six lines of the error
-    let max_lines = 6u;
     let mut elided = false;
     let mut display_lines = lines.lines.as_slice();
-    if display_lines.len() > max_lines {
-        display_lines = display_lines.slice(0u, max_lines);
+    if display_lines.len() > MAX_LINES {
+        display_lines = display_lines.slice(0u, MAX_LINES);
         elided = true;
     }
     // Print the offending lines
@@ -319,6 +345,44 @@ fn highlight_lines(cm: &codemap::CodeMap,
     }
 }
 
+// Here are the differences between this and the normal `highlight_lines`:
+// `custom_highlight_lines` will always put arrow on the last byte of the
+// span (instead of the first byte). Also, when the span is too long (more
+// than 6 lines), `custom_highlight_lines` will print the first line, then
+// dot dot dot, then last line, whereas `highlight_lines` prints the first
+// six lines.
+fn custom_highlight_lines(cm: &codemap::CodeMap,
+                          sp: Span,
+                          lvl: Level,
+                          lines: &codemap::FileLines) {
+    let fm = lines.file;
+    let mut err = io::stderr();
+    let err = &mut err as &mut io::Writer;
+
+    let lines = lines.lines.as_slice();
+    if lines.len() > MAX_LINES {
+        write!(err, "{}:{} {}\n", fm.name,
+               lines[0] + 1, fm.get_line(lines[0] as int));
+        write!(err, "...\n");
+        let last_line = lines[lines.len()-1];
+        write!(err, "{}:{} {}\n", fm.name,
+               last_line + 1, fm.get_line(last_line as int));
+    } else {
+        for line in lines.iter() {
+            write!(err, "{}:{} {}\n", fm.name,
+                   *line + 1, fm.get_line(*line as int));
+        }
+    }
+    let last_line_start = format!("{}:{} ", fm.name, lines[lines.len()-1]+1);
+    let hi = cm.lookup_char_pos(sp.hi);
+    // Span seems to use half-opened interval, so subtract 1
+    let skip = last_line_start.len() + hi.col.to_uint() - 1;
+    let mut s = ~"";
+    skip.times(|| s.push_char(' '));
+    s.push_char('^');
+    print_maybe_styled(s + "\n", term::attr::ForegroundColor(lvl.color()));
+}
+
 fn print_macro_backtrace(cm: &codemap::CodeMap, sp: Span) {
     for ei in sp.expn_info.iter() {
         let ss = ei.callee.span.as_ref().map_or(~"", |span| cm.span_to_str(*span));

src/test/compile-fail/borrowck-borrow-mut-object-twice.rs

@@ -18,7 +18,7 @@ trait Foo {
 
 fn test(x: &mut Foo) {
     let _y = x.f1();
-    x.f2(); //~ ERROR cannot borrow `*x` as mutable more than once at a time
+    x.f2(); //~ ERROR cannot borrow `*x` because it is already borrowed as mutable
 }
 
 fn main() {}

src/test/compile-fail/borrowck-report-with-custom-diagnostic.rs

@@ -0,0 +1,31 @@
+#[allow(dead_code)];
+fn main() {
+    // Original borrow ends at end of function
+    let mut x = 1u;
+    let y = &mut x;
+    let z = &x; //~ ERROR cannot borrow
+}
+//~^ NOTE previous borrow ends here
+
+fn foo() {
+    match true {
+        true => {
+            // Original borrow ends at end of match arm
+            let mut x = 1u;
+            let y = &x;
+            let z = &mut x; //~ ERROR cannot borrow
+        }
+     //~^ NOTE previous borrow ends here
+        false => ()
+    }
+}
+
+fn bar() {
+    // Original borrow ends at end of closure
+    || {
+        let mut x = 1u;
+        let y = &mut x;
+        let z = &mut x; //~ ERROR cannot borrow
+    };
+ //~^ NOTE previous borrow ends here
+}

src/test/compile-fail/mut-cant-alias.rs

@@ -14,5 +14,5 @@ fn main() {
     let m = RefCell::new(0);
     let mut b = m.borrow_mut();
     let b1 = b.get();
-    let b2 = b.get(); //~ ERROR cannot borrow `b` as mutable more than once at a time
+    let b2 = b.get(); //~ ERROR cannot borrow `b` because it is already borrowed as mutable
 }

src/test/compile-fail/vec-mut-iter-borrow.rs

@@ -12,6 +12,6 @@ fn main() {
     let mut xs = ~[1, 2, 3, 4];
 
     for x in xs.mut_iter() {
-        xs.push(1) //~ ERROR cannot borrow `xs` as mutable
+        xs.push(1) //~ ERROR cannot borrow `xs` because it is already borrowed as mutable
     }
 }