Improve error messages when illegal lifetimes are used

8 files changed, 109 insertions, 56 deletions

diff --git a/src/librustc/middle/typeck/astconv.rs b/src/librustc/middle/typeck/astconv.rs
index e018cf6f940..606ba59fbf6 100644
--- a/src/librustc/middle/typeck/astconv.rs
+++ b/src/librustc/middle/typeck/astconv.rs
@@ -58,14 +58,14 @@ use middle::ty::{arg, field, substs};
 use middle::ty::{ty_param_substs_and_ty};
 use middle::ty;
 use middle::typeck::rscope::{in_binding_rscope};
-use middle::typeck::rscope::{region_scope, type_rscope};
+use middle::typeck::rscope::{region_scope, type_rscope, RegionError};
 use middle::typeck::{CrateCtxt, write_substs_to_tcx, write_ty_to_tcx};
 
 use core::result;
 use core::vec;
 use syntax::ast;
 use syntax::codemap::span;
-use syntax::print::pprust::path_to_str;
+use syntax::print::pprust::{region_to_str, path_to_str};
 use util::common::indenter;
 
 pub trait AstConv {
@@ -76,17 +76,31 @@ pub trait AstConv {
     fn ty_infer(&self, span: span) -> ty::t;
 }
 
-pub fn get_region_reporting_err(tcx: ty::ctxt,
-                                span: span,
-                                res: Result<ty::Region, ~str>)
-                             -> ty::Region {
-
+pub fn get_region_reporting_err(
+    tcx: ty::ctxt,
+    span: span,
+    a_r: Option<@ast::region>,
+    res: Result<ty::Region, RegionError>) -> ty::Region
+{
     match res {
-      result::Ok(r) => r,
-      result::Err(ref e) => {
-        tcx.sess.span_err(span, (/*bad*/copy *e));
-        ty::re_static
-      }
+        result::Ok(r) => r,
+        result::Err(ref e) => {
+            let descr = match a_r {
+                None => ~"anonymous lifetime",
+                Some(a) if a.node == ast::re_anon => {
+                    ~"anonymous lifetime"
+                }
+                Some(a) => {
+                    fmt!("lifetime %s",
+                         region_to_str(a, tcx.sess.intr()))
+                }
+            };
+            tcx.sess.span_err(
+                span,
+                fmt!("Illegal %s: %s",
+                     descr, e.msg));
+            e.replacement
+        }
     }
 }
 
@@ -103,7 +117,7 @@ pub fn ast_region_to_region<AC:AstConv,RS:region_scope + Copy + Durable>(
         ast::re_named(id) => rscope.named_region(span, id)
     };
 
-    get_region_reporting_err(self.tcx(), span, res)
+    get_region_reporting_err(self.tcx(), span, Some(a_r), res)
 }
 
 pub fn ast_path_to_substs_and_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
@@ -139,7 +153,7 @@ pub fn ast_path_to_substs_and_ty<AC:AstConv,RS:region_scope + Copy + Durable>(
       }
       (Some(_), None) => {
         let res = rscope.anon_region(path.span);
-        let r = get_region_reporting_err(self.tcx(), path.span, res);
+        let r = get_region_reporting_err(self.tcx(), path.span, None, res);
         Some(r)
       }
       (Some(_), Some(r)) => {
@@ -521,7 +535,7 @@ pub fn ty_of_closure<AC:AstConv,RS:region_scope + Copy + Durable>(
                 ast::BorrowedSigil => {
                     // &fn() defaults to an anonymous region:
                     let r_result = rscope.anon_region(span);
-                    get_region_reporting_err(self.tcx(), span, r_result)
+                    get_region_reporting_err(self.tcx(), span, None, r_result)
                 }
             }
         }
diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs
index 2cb06e783dd..66c2a28da3d 100644
--- a/src/librustc/middle/typeck/check/mod.rs
+++ b/src/librustc/middle/typeck/check/mod.rs
@@ -1,4 +1,4 @@
-n// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
 //
@@ -96,6 +96,7 @@ use middle::typeck::CrateCtxt;
 use middle::typeck::infer::{resolve_type, force_tvar};
 use middle::typeck::infer;
 use middle::typeck::rscope::{binding_rscope, bound_self_region};
+use middle::typeck::rscope::{RegionError};
 use middle::typeck::rscope::{in_binding_rscope, region_scope, type_rscope};
 use middle::typeck::rscope;
 use middle::typeck::{isr_alist, lookup_def_ccx, method_map_entry};
@@ -651,7 +652,8 @@ pub impl FnCtxt {
     fn infcx(&self) -> @mut infer::InferCtxt { self.inh.infcx }
     fn search_in_scope_regions(
         &self,
-        br: ty::bound_region) -> Result<ty::Region, ~str>
+        span: span,
+        br: ty::bound_region) -> Result<ty::Region, RegionError>
     {
         let in_scope_regions = self.in_scope_regions;
         match in_scope_regions.find(br) {
@@ -661,8 +663,11 @@ pub impl FnCtxt {
                 if br == blk_br {
                     result::Ok(self.block_region())
                 } else {
-                    result::Err(fmt!("named region `%s` not in scope here",
-                                     bound_region_to_str(self.tcx(), br)))
+                    result::Err(RegionError {
+                        msg: fmt!("named region `%s` not in scope here",
+                                  bound_region_to_str(self.tcx(), br)),
+                        replacement: self.infcx().next_region_var_nb(span)
+                    })
                 }
             }
         }
@@ -670,16 +675,16 @@ pub impl FnCtxt {
 }
 
 impl region_scope for FnCtxt {
-    fn anon_region(&self, span: span) -> Result<ty::Region, ~str> {
+    fn anon_region(&self, span: span) -> Result<ty::Region, RegionError> {
         result::Ok(self.infcx().next_region_var_nb(span))
     }
-    fn self_region(&self, _span: span) -> Result<ty::Region, ~str> {
-        self.search_in_scope_regions(ty::br_self)
+    fn self_region(&self, span: span) -> Result<ty::Region, RegionError> {
+        self.search_in_scope_regions(span, ty::br_self)
     }
     fn named_region(&self,
-                    _span: span,
-                    id: ast::ident) -> Result<ty::Region, ~str> {
-        self.search_in_scope_regions(ty::br_named(id))
+                    span: span,
+                    id: ast::ident) -> Result<ty::Region, RegionError> {
+        self.search_in_scope_regions(span, ty::br_named(id))
     }
 }
 
diff --git a/src/librustc/middle/typeck/rscope.rs b/src/librustc/middle/typeck/rscope.rs
index 77da34a31e9..3c342e9986b 100644
--- a/src/librustc/middle/typeck/rscope.rs
+++ b/src/librustc/middle/typeck/rscope.rs
@@ -17,24 +17,33 @@ use core::result;
 use syntax::ast;
 use syntax::codemap::span;
 
+pub struct RegionError {
+    msg: ~str,
+    replacement: ty::Region
+}
+
 pub trait region_scope {
-    fn anon_region(&self, span: span) -> Result<ty::Region, ~str>;
-    fn self_region(&self, span: span) -> Result<ty::Region, ~str>;
+    fn anon_region(&self, span: span) -> Result<ty::Region, RegionError>;
+    fn self_region(&self, span: span) -> Result<ty::Region, RegionError>;
     fn named_region(&self, span: span, id: ast::ident)
-                      -> Result<ty::Region, ~str>;
+                      -> Result<ty::Region, RegionError>;
 }
 
 pub enum empty_rscope { empty_rscope }
 impl region_scope for empty_rscope {
-    fn anon_region(&self, _span: span) -> Result<ty::Region, ~str> {
-        result::Err(~"only the static region is allowed here")
+    fn anon_region(&self, _span: span) -> Result<ty::Region, RegionError> {
+        result::Err(RegionError {
+            msg: ~"only 'static is allowed here",
+            replacement: ty::re_static
+        })
     }
-    fn self_region(&self, _span: span) -> Result<ty::Region, ~str> {
-        result::Err(~"only the static region is allowed here")
+    fn self_region(&self, _span: span) -> Result<ty::Region, RegionError> {
+        self.anon_region(_span)
     }
     fn named_region(&self, _span: span, _id: ast::ident)
-        -> Result<ty::Region, ~str> {
-        result::Err(~"only the static region is allowed here")
+        -> Result<ty::Region, RegionError>
+    {
+        self.anon_region(_span)
     }
 }
 
@@ -43,38 +52,59 @@ pub struct MethodRscope {
     region_parameterization: Option<ty::region_variance>
 }
 impl region_scope for MethodRscope {
-    fn anon_region(&self, _span: span) -> Result<ty::Region, ~str> {
-        result::Err(~"anonymous region types are not permitted here")
+    fn anon_region(&self, _span: span) -> Result<ty::Region, RegionError> {
+        result::Err(RegionError {
+            msg: ~"anonymous lifetimes are not permitted here",
+            replacement: ty::re_bound(ty::br_self)
+        })
     }
-    fn self_region(&self, _span: span) -> Result<ty::Region, ~str> {
+    fn self_region(&self, _span: span) -> Result<ty::Region, RegionError> {
         assert self.region_parameterization.is_some() ||
             self.self_ty.is_borrowed();
         result::Ok(ty::re_bound(ty::br_self))
     }
     fn named_region(&self, span: span, id: ast::ident)
-                      -> Result<ty::Region, ~str> {
+                      -> Result<ty::Region, RegionError> {
         do empty_rscope.named_region(span, id).chain_err |_e| {
-            result::Err(~"region is not in scope here")
+            result::Err(RegionError {
+                msg: ~"lifetime is not in scope",
+                replacement: ty::re_bound(ty::br_self)
+            })
         }
     }
 }
 
 pub enum type_rscope = Option<ty::region_variance>;
+impl type_rscope {
+    priv fn replacement(&self) -> ty::Region {
+        if self.is_some() {
+            ty::re_bound(ty::br_self)
+        } else {
+            ty::re_static
+        }
+    }
+}
 impl region_scope for type_rscope {
-    fn anon_region(&self, _span: span) -> Result<ty::Region, ~str> {
-        result::Err(~"anonymous region types are not permitted here")
+    fn anon_region(&self, _span: span) -> Result<ty::Region, RegionError> {
+        result::Err(RegionError {
+            msg: ~"anonymous lifetimes are not permitted here",
+            replacement: self.replacement()
+        })
     }
-    fn self_region(&self, _span: span) -> Result<ty::Region, ~str> {
+    fn self_region(&self, _span: span) -> Result<ty::Region, RegionError> {
         // if the self region is used, region parameterization should
         // have inferred that this type is RP
         assert self.is_some();
         result::Ok(ty::re_bound(ty::br_self))
     }
     fn named_region(&self, span: span, id: ast::ident)
-                      -> Result<ty::Region, ~str> {
+                      -> Result<ty::Region, RegionError> {
         do empty_rscope.named_region(span, id).chain_err |_e| {
-            result::Err(~"named regions other than `self` are not \
-                          allowed as part of a type declaration")
+            result::Err(RegionError {
+                msg: ~"only 'self is allowed allowed as \
+                       part of a type declaration",
+                replacement: self.replacement()
+            })
         }
     }
 }
@@ -98,17 +128,17 @@ pub fn in_binding_rscope<RS:region_scope + Copy + Durable>(self: &RS)
     binding_rscope { base: base, anon_bindings: @mut 0 }
 }
 impl region_scope for binding_rscope {
-    fn anon_region(&self, _span: span) -> Result<ty::Region, ~str> {
+    fn anon_region(&self, _span: span) -> Result<ty::Region, RegionError> {
         let idx = *self.anon_bindings;
         *self.anon_bindings += 1;
         result::Ok(ty::re_bound(ty::br_anon(idx)))
     }
-    fn self_region(&self, span: span) -> Result<ty::Region, ~str> {
+    fn self_region(&self, span: span) -> Result<ty::Region, RegionError> {
         self.base.self_region(span)
     }
     fn named_region(&self,
                     span: span,
-                    id: ast::ident) -> Result<ty::Region, ~str>
+                    id: ast::ident) -> Result<ty::Region, RegionError>
     {
         do self.base.named_region(span, id).chain_err |_e| {
             result::Ok(ty::re_bound(ty::br_named(id)))
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index 3b4b198e595..048f4f098ba 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -147,6 +147,10 @@ pub fn expr_to_str(e: @ast::expr, intr: @ident_interner) -> ~str {
     to_str(e, print_expr, intr)
 }
 
+pub fn region_to_str(e: @ast::region, intr: @ident_interner) -> ~str {
+    to_str(e, |s, e| print_region(s, ~"&", e, ~""), intr)
+}
+
 pub fn tt_to_str(tt: ast::token_tree, intr: @ident_interner) -> ~str {
     to_str(tt, print_tt, intr)
 }
diff --git a/src/test/compile-fail/regions-in-consts.rs b/src/test/compile-fail/regions-in-consts.rs
index d3e5bfd3e85..19dea9a57ee 100644
--- a/src/test/compile-fail/regions-in-consts.rs
+++ b/src/test/compile-fail/regions-in-consts.rs
@@ -8,8 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-const c_x: &'blk int = &22; //~ ERROR only the static region is allowed here
-const c_y: &int = &22; //~ ERROR only the static region is allowed here
+const c_x: &'blk int = &22; //~ ERROR Illegal lifetime &blk: only 'static is allowed here
+const c_y: &int = &22; //~ ERROR Illegal anonymous lifetime: only 'static is allowed here
 const c_z: &'static int = &22;
 
 fn main() {
diff --git a/src/test/compile-fail/regions-in-enums.rs b/src/test/compile-fail/regions-in-enums.rs
index b399ef8a747..5c2269977d1 100644
--- a/src/test/compile-fail/regions-in-enums.rs
+++ b/src/test/compile-fail/regions-in-enums.rs
@@ -10,7 +10,7 @@
 
 enum yes0<'lt> {
     // This will eventually be legal (and in fact the only way):
-    X3(&'lt uint) //~ ERROR named regions other than `self` are not allowed as part of a type declaration
+    X3(&'lt uint) //~ ERROR Illegal lifetime &lt: only 'self is allowed allowed as part of a type declaration
 }
 
 enum yes1 {
@@ -18,7 +18,7 @@ enum yes1 {
 }
 
 enum yes2 {
-    X5(&'foo uint) //~ ERROR named regions other than `self` are not allowed as part of a type declaration
+    X5(&'foo uint) //~ ERROR Illegal lifetime &foo: only 'self is allowed allowed as part of a type declaration
 }
 
 fn main() {}
diff --git a/src/test/compile-fail/regions-in-structs.rs b/src/test/compile-fail/regions-in-structs.rs
index a7656a53e39..10d7a921ed0 100644
--- a/src/test/compile-fail/regions-in-structs.rs
+++ b/src/test/compile-fail/regions-in-structs.rs
@@ -9,7 +9,7 @@
 // except according to those terms.
 
 struct yes0<'self> {
-  x: &uint, //~ ERROR anonymous region types are not permitted here
+  x: &uint, //~ ERROR Illegal anonymous lifetime: anonymous lifetimes are not permitted here
 }
 
 struct yes1<'self> {
@@ -17,7 +17,7 @@ struct yes1<'self> {
 }
 
 struct yes2<'self> {
-  x: &'foo uint, //~ ERROR named regions other than `self` are not allowed as part of a type declaration
+  x: &'foo uint, //~ ERROR Illegal lifetime &foo: only 'self is allowed allowed as part of a type declaration
 }
 
 fn main() {}
diff --git a/src/test/compile-fail/regions-in-type-items.rs b/src/test/compile-fail/regions-in-type-items.rs
index 5519a99d4b8..2397c8f2311 100644
--- a/src/test/compile-fail/regions-in-type-items.rs
+++ b/src/test/compile-fail/regions-in-type-items.rs
@@ -9,7 +9,7 @@
 // except according to those terms.
 
 type item_ty_yes0 = {
-    x: &uint //~ ERROR anonymous region types are not permitted here
+    x: &uint //~ ERROR Illegal anonymous lifetime: anonymous lifetimes are not permitted here
 };
 
 type item_ty_yes1 = {
@@ -17,7 +17,7 @@ type item_ty_yes1 = {
 };
 
 type item_ty_yes2 = {
-    x: &'foo uint //~ ERROR named regions other than `self` are not allowed as part of a type declaration
+    x: &'foo uint //~ ERROR Illegal lifetime &foo: only 'self is allowed allowed as part of a type declaration
 };
 
 fn main() {}