improve borrowck error messages to explain regions better

Author: nikomatsakis

src/libsyntax/parse/parser.rs

@@ -2917,7 +2917,7 @@ class parser {
                             body: d_body},
                      span: d_s}
                 };
-                
+
                 kind = struct_variant_kind(@{
                     traits: ~[],
                     members: ms,

src/rustc/middle/borrowck.rs

@@ -220,7 +220,7 @@ import syntax::visit;
 import syntax::ast_util;
 import syntax::ast_map;
 import syntax::codemap::span;
-import util::ppaux::{ty_to_str, region_to_str};
+import util::ppaux::{ty_to_str, region_to_str, explain_region};
 import std::map::{int_hash, hashmap, set};
 import std::list;
 import std::list::{list, cons, nil};
@@ -626,16 +626,16 @@ impl to_str_methods for borrowck_ctxt {
             ~"rooting is not permitted"
           }
           err_out_of_root_scope(super_scope, sub_scope) => {
-            fmt!{"managed value would have to be rooted for lifetime %s, \
-                  but can only be rooted for lifetime %s",
-                 self.region_to_str(sub_scope),
-                 self.region_to_str(super_scope)}
+            fmt!{"managed value would have to be rooted for %s, \
+                  but can only be rooted for %s",
+                  explain_region(self.tcx, sub_scope),
+                  explain_region(self.tcx, super_scope)}
           }
           err_out_of_scope(super_scope, sub_scope) => {
-            fmt!{"borrowed pointer has lifetime %s, \
-                  but the borrowed value only has lifetime %s",
-                 self.region_to_str(sub_scope),
-                 self.region_to_str(super_scope)}
+            fmt!{"borrowed pointer must be valid for %s, \
+                  but the borrowed value is only valid for %s",
+                  explain_region(self.tcx, sub_scope),
+                  explain_region(self.tcx, super_scope)}
           }
         }
     }

src/rustc/util/ppaux.rs

@@ -27,7 +27,7 @@ import driver::session::session;
 fn explain_region(cx: ctxt, region: ty::region) -> ~str {
     return match region {
       re_scope(node_id) => {
-        let scope_str = match cx.items.find(node_id) {
+        match cx.items.find(node_id) {
           some(ast_map::node_block(blk)) => {
             explain_span(cx, ~"block", blk.span)
           }
@@ -42,36 +42,36 @@ fn explain_region(cx: ctxt, region: ty::region) -> ~str {
             // this really should not happen
             fmt!{"unknown scope: %d.  Please report a bug.", node_id}
           }
-        };
-        fmt!{"reference valid for the %s", scope_str}
+        }
       }
 
       re_free(id, br) => {
         match cx.items.find(id) {
           some(ast_map::node_block(blk)) => {
-            fmt!{"reference with lifetime %s as defined on %s",
+            fmt!{"the lifetime %s as defined on %s",
                  bound_region_to_str(cx, br),
                  explain_span(cx, ~"the block", blk.span)}
           }
           some(_) | none => {
             // this really should not happen
-            fmt!{"reference with lifetime %s as defined on node %d",
+            fmt!{"the lifetime %s as defined on node %d",
                  bound_region_to_str(cx, br), id}
           }
         }
       }
 
-      re_static => { ~"reference to static data" }
+      re_static => { ~"the static lifetime" }
 
-      // I believe these cases should not occur.
+      // I believe these cases should not occur (except when debugging,
+      // perhaps)
       re_var(_) | re_bound(_) => {
-        fmt!{"reference with lifetime %?", region}
+        fmt!{"lifetime %?", region}
       }
     };
 
     fn explain_span(cx: ctxt, heading: ~str, span: span) -> ~str {
         let lo = codemap::lookup_char_pos_adj(cx.sess.codemap, span.lo);
-        fmt!{"%s at %u:%u", heading, lo.line, lo.col}
+        fmt!{"the %s at %u:%u", heading, lo.line, lo.col}
     }
 }
 

src/test/compile-fail/borrowck-confuse-region.rs

@@ -0,0 +1,14 @@
+// Here we are checking that a reasonable error msg is provided.
+//
+// The current message is not ideal, but we used to say "borrowed
+// pointer has lifetime &, but the borrowed value only has lifetime &"
+// which is definitely no good.
+
+
+fn get() -> &int {
+    let x = 3;
+    return &x;
+    //~^ ERROR illegal borrow: borrowed pointer must be valid for the lifetime & as defined on the the block at 8:17, but the borrowed value is only valid for the block at 8:17
+}
+
+fn main() {}

src/test/compile-fail/regions-infer-borrow-scope-within-loop.rs

@@ -7,7 +7,7 @@ fn foo(cond: fn() -> bool, box: fn() -> @int) {
 
 	// Here we complain because the resulting region
 	// of this borrow is the fn body as a whole.
-        y = borrow(x); //~ ERROR managed value would have to be rooted for lifetime 
+        y = borrow(x); //~ ERROR illegal borrow: managed value would have to be rooted
 
         assert *x == *y;
         if cond() { break; }