From 3280e5a33d839530bd77d27789fef4b9775ab37f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 27 Feb 2013 16:21:07 -0500 Subject: [PATCH] Improve error messages when illegal lifetimes are used --- src/librustc/middle/typeck/astconv.rs | 44 +++++++---- src/librustc/middle/typeck/check/mod.rs | 25 +++--- src/librustc/middle/typeck/rscope.rs | 78 +++++++++++++------ src/libsyntax/print/pprust.rs | 4 + src/test/compile-fail/regions-in-consts.rs | 4 +- src/test/compile-fail/regions-in-enums.rs | 4 +- src/test/compile-fail/regions-in-structs.rs | 4 +- .../compile-fail/regions-in-type-items.rs | 4 +- 8 files changed, 110 insertions(+), 57 deletions(-) diff --git a/src/librustc/middle/typeck/astconv.rs b/src/librustc/middle/typeck/astconv.rs index e018cf6f940e..606ba59fbf64 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 { - +pub fn get_region_reporting_err( + tcx: ty::ctxt, + span: span, + a_r: Option<@ast::region>, + res: Result) -> 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( 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( @@ -139,7 +153,7 @@ pub fn ast_path_to_substs_and_ty( } (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( 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 2cb06e783dda..66c2a28da3d6 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 + span: span, + br: ty::bound_region) -> Result { 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 { + fn anon_region(&self, span: span) -> Result { result::Ok(self.infcx().next_region_var_nb(span)) } - fn self_region(&self, _span: span) -> Result { - self.search_in_scope_regions(ty::br_self) + fn self_region(&self, span: span) -> Result { + self.search_in_scope_regions(span, ty::br_self) } fn named_region(&self, - _span: span, - id: ast::ident) -> Result { - self.search_in_scope_regions(ty::br_named(id)) + span: span, + id: ast::ident) -> Result { + 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 77da34a31e9a..3c342e9986bb 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; - fn self_region(&self, span: span) -> Result; + fn anon_region(&self, span: span) -> Result; + fn self_region(&self, span: span) -> Result; fn named_region(&self, span: span, id: ast::ident) - -> Result; + -> Result; } pub enum empty_rscope { empty_rscope } impl region_scope for empty_rscope { - fn anon_region(&self, _span: span) -> Result { - result::Err(~"only the static region is allowed here") + fn anon_region(&self, _span: span) -> Result { + result::Err(RegionError { + msg: ~"only 'static is allowed here", + replacement: ty::re_static + }) } - fn self_region(&self, _span: span) -> Result { - result::Err(~"only the static region is allowed here") + fn self_region(&self, _span: span) -> Result { + self.anon_region(_span) } fn named_region(&self, _span: span, _id: ast::ident) - -> Result { - result::Err(~"only the static region is allowed here") + -> Result + { + self.anon_region(_span) } } @@ -43,38 +52,59 @@ pub struct MethodRscope { region_parameterization: Option } impl region_scope for MethodRscope { - fn anon_region(&self, _span: span) -> Result { - result::Err(~"anonymous region types are not permitted here") + fn anon_region(&self, _span: span) -> Result { + result::Err(RegionError { + msg: ~"anonymous lifetimes are not permitted here", + replacement: ty::re_bound(ty::br_self) + }) } - fn self_region(&self, _span: span) -> Result { + fn self_region(&self, _span: span) -> Result { 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 { + -> Result { 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; -impl region_scope for type_rscope { - fn anon_region(&self, _span: span) -> Result { - result::Err(~"anonymous region types are not permitted here") +impl type_rscope { + priv fn replacement(&self) -> ty::Region { + if self.is_some() { + ty::re_bound(ty::br_self) + } else { + ty::re_static + } } - fn self_region(&self, _span: span) -> Result { +} +impl region_scope for type_rscope { + fn anon_region(&self, _span: span) -> Result { + result::Err(RegionError { + msg: ~"anonymous lifetimes are not permitted here", + replacement: self.replacement() + }) + } + fn self_region(&self, _span: span) -> Result { // 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 { + -> Result { 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(self: &RS) binding_rscope { base: base, anon_bindings: @mut 0 } } impl region_scope for binding_rscope { - fn anon_region(&self, _span: span) -> Result { + fn anon_region(&self, _span: span) -> Result { 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 { + fn self_region(&self, span: span) -> Result { self.base.self_region(span) } fn named_region(&self, span: span, - id: ast::ident) -> Result + id: ast::ident) -> Result { 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 3b4b198e5956..048f4f098bab 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 d3e5bfd3e85b..19dea9a57ee4 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 b399ef8a747f..5c2269977d19 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 <: 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 a7656a53e39e..10d7a921ed03 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 5519a99d4b8d..2397c8f23113 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() {}