From 52c517383ef57f96ce1a97babc627d03329ac5e6 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 7 Aug 2012 19:48:24 -0700 Subject: [PATCH] improve borrowck error messages to explain regions better --- src/libsyntax/parse/parser.rs | 2 +- src/rustc/middle/borrowck.rs | 18 +++++++++--------- src/rustc/util/ppaux.rs | 18 +++++++++--------- .../compile-fail/borrowck-confuse-region.rs | 14 ++++++++++++++ .../regions-infer-borrow-scope-within-loop.rs | 2 +- 5 files changed, 34 insertions(+), 20 deletions(-) create mode 100644 src/test/compile-fail/borrowck-confuse-region.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 6f19f272331..6f8128b9b76 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2917,7 +2917,7 @@ class parser { body: d_body}, span: d_s} }; - + kind = struct_variant_kind(@{ traits: ~[], members: ms, diff --git a/src/rustc/middle/borrowck.rs b/src/rustc/middle/borrowck.rs index 50a903551cd..2fbfb4fe9f6 100644 --- a/src/rustc/middle/borrowck.rs +++ b/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)} } } } diff --git a/src/rustc/util/ppaux.rs b/src/rustc/util/ppaux.rs index b4f04ffab2b..b64c7232137 100644 --- a/src/rustc/util/ppaux.rs +++ b/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} } } diff --git a/src/test/compile-fail/borrowck-confuse-region.rs b/src/test/compile-fail/borrowck-confuse-region.rs new file mode 100644 index 00000000000..cb78ca00fd2 --- /dev/null +++ b/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() {} \ No newline at end of file diff --git a/src/test/compile-fail/regions-infer-borrow-scope-within-loop.rs b/src/test/compile-fail/regions-infer-borrow-scope-within-loop.rs index 0c62532f86a..4342fa6e961 100644 --- a/src/test/compile-fail/regions-infer-borrow-scope-within-loop.rs +++ b/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; }