auto merge of #11718 : ktt3ja/rust/borrowck-error-msg, r=brson

A mutable and immutable borrow place some restrictions on what you can
with the variable until the borrow ends. This commit attempts to convey
to the user what those restrictions are. Also, if the original borrow is
a mutable borrow, the error message has been changed (more specifically,
i. "cannot borrow `x` as immutable because it is also borrowed as
mutable" and ii. "cannot borrow `x` as mutable more than once" have
been changed to "cannot borrow `x` because it is already borrowed as
mutable").

In addition, this adds a (custom) span note to communicate where the
original borrow ends.

```rust
fn main() {
    match true {
        true => {
            let mut x = 1;
            let y = &x;
            let z = &mut x;
        }
        false => ()
    }
}

test.rs:6:21: 6:27 error: cannot borrow `x` as mutable because it is already borrowed as immutable
test.rs:6             let z = &mut x;
                              ^~~~~~
test.rs:5:21: 5:23 note: previous borrow of `x` occurs here; the immutable borrow prevents subsequent moves or mutable borrows of `x` until the borrow ends
test.rs:5             let y = &x;
                              ^~
test.rs:7:10: 7:10 note: previous borrow ends here
test.rs:3         true => {
test.rs:4             let mut x = 1;
test.rs:5             let y = &x;
test.rs:6             let z = &mut x;
test.rs:7         }
                  ^
```

```rust
fn foo3(t0: &mut &mut int) {
    let t1 = &mut *t0;
    let p: &int = &**t0;
}

fn main() {}

test.rs:3:19: 3:24 error: cannot borrow `**t0` because it is already borrowed as mutable
test.rs:3     let p: &int = &**t0;
                            ^~~~~
test.rs:2:14: 2:22 note: previous borrow of `**t0` as mutable occurs here; the mutable borrow prevents subsequent moves, borrows, or modification of `**t0` until the borrow ends
test.rs:2     let t1 = &mut *t0;
                       ^~~~~~~~
test.rs:4:2: 4:2 note: previous borrow ends here
test.rs:1 fn foo3(t0: &mut &mut int) {
test.rs:2     let t1 = &mut *t0;
test.rs:3     let p: &int = &**t0;
test.rs:4 }
          ^
```

For the "previous borrow ends here" note, if the span is too long (has too many lines), then only the first and last lines are printed, and the middle is replaced with dot dot dot:
```rust
fn foo3(t0: &mut &mut int) {
    let t1 = &mut *t0;
    let p: &int = &**t0;



}

fn main() {}

test.rs:3:19: 3:24 error: cannot borrow `**t0` because it is already borrowed as mutable
test.rs:3     let p: &int = &**t0;
                            ^~~~~
test.rs:2:14: 2:22 note: previous borrow of `**t0` as mutable occurs here; the mutable borrow prevents subsequent moves, borrows, or modification of `**t0` until the borrow ends
test.rs:2     let t1 = &mut *t0;
                       ^~~~~~~~
test.rs:7:2: 7:2 note: previous borrow ends here
test.rs:1 fn foo3(t0: &mut &mut int) {
...
test.rs:7 }
          ^
```

(Sidenote: the `span_end_note` currently also has issue #11715)

9 files changed, 144 insertions, 33 deletions

diff --git a/src/librustc/driver/session.rs b/src/librustc/driver/session.rs
index 75094bc8084..42b716e8cdf 100644
--- a/src/librustc/driver/session.rs
+++ b/src/librustc/driver/session.rs
@@ -250,6 +250,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)
     }
diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs
index fe9371af1fe..0fd92079489 100644
--- a/src/librustc/middle/borrowck/check_loans.rs
+++ b/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
diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs
index 74845878907..f1cccab1239 100644
--- a/src/librustc/middle/borrowck/mod.rs
+++ b/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) => {
diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index ebf02f7691e..d81999c1e5f 100644
--- a/src/libsyntax/codemap.rs
+++ b/src/libsyntax/codemap.rs
@@ -308,10 +308,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();
diff --git a/src/libsyntax/diagnostic.rs b/src/libsyntax/diagnostic.rs
index 0eca56e2691..a9d3f6fea24 100644
--- a/src/libsyntax/diagnostic.rs
+++ b/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));
diff --git a/src/test/compile-fail/borrowck-borrow-mut-object-twice.rs b/src/test/compile-fail/borrowck-borrow-mut-object-twice.rs
index 502d7e017b5..c338aac2ddf 100644
--- a/src/test/compile-fail/borrowck-borrow-mut-object-twice.rs
+++ b/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() {}
diff --git a/src/test/compile-fail/borrowck-report-with-custom-diagnostic.rs b/src/test/compile-fail/borrowck-report-with-custom-diagnostic.rs
new file mode 100644
index 00000000000..3c01045369f
--- /dev/null
+++ b/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
+}
diff --git a/src/test/compile-fail/mut-cant-alias.rs b/src/test/compile-fail/mut-cant-alias.rs
index f031467328f..5b8079b832e 100644
--- a/src/test/compile-fail/mut-cant-alias.rs
+++ b/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
 }
diff --git a/src/test/compile-fail/vec-mut-iter-borrow.rs b/src/test/compile-fail/vec-mut-iter-borrow.rs
index 19c786b553f..21ffc1ae7f9 100644
--- a/src/test/compile-fail/vec-mut-iter-borrow.rs
+++ b/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
     }
 }