Skip to content

Support lifetime suggestion for method #13164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 174 additions & 52 deletions src/librustc/middle/typeck/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ use syntax::ast_map;
use syntax::ast_util;
use syntax::ast_util::name_to_dummy_lifetime;
use syntax::owned_slice::OwnedSlice;
use syntax::codemap;
use syntax::parse::token;
use syntax::print::pprust;
use util::ppaux::UserString;
Expand Down Expand Up @@ -143,10 +144,12 @@ trait ErrorReportingHelpers {
origin: SubregionOrigin);

fn give_expl_lifetime_param(&self,
inputs: Vec<ast::Arg>,
output: ast::P<ast::Ty>,
item: ast::P<ast::Item>,
generics: ast::Generics);
decl: &ast::FnDecl,
fn_style: ast::FnStyle,
ident: ast::Ident,
opt_explicit_self: Option<ast::ExplicitSelf_>,
generics: &ast::Generics,
span: codemap::Span);
}

impl<'a> ErrorReporting for InferCtxt<'a> {
Expand Down Expand Up @@ -260,6 +263,19 @@ impl<'a> ErrorReporting for InferCtxt<'a> {
scope_id: ast::NodeId
}

impl FreeRegionsFromSameFn {
fn new(sub_fr: ty::FreeRegion,
sup_fr: ty::FreeRegion,
scope_id: ast::NodeId)
-> FreeRegionsFromSameFn {
FreeRegionsFromSameFn {
sub_fr: sub_fr,
sup_fr: sup_fr,
scope_id: scope_id
}
}
}

fn free_regions_from_same_fn(tcx: &ty::ctxt,
sub: Region,
sup: Region)
Expand All @@ -280,17 +296,14 @@ impl<'a> ErrorReporting for InferCtxt<'a> {
match parent_node {
Some(node) => match node {
ast_map::NodeItem(item) => match item.node {
// FIXME: handle method
ast::ItemFn(..) => {
let fr_from_same_fn = FreeRegionsFromSameFn {
sub_fr: fr1,
sup_fr: fr2,
scope_id: scope_id
};
Some(fr_from_same_fn)
Some(FreeRegionsFromSameFn::new(fr1, fr2, scope_id))
},
_ => None
},
ast_map::NodeMethod(..) => {
Some(FreeRegionsFromSameFn::new(fr1, fr2, scope_id))
},
_ => None
},
None => {
Expand Down Expand Up @@ -662,21 +675,28 @@ impl<'a> ErrorReporting for InferCtxt<'a> {
let node_inner = match parent_node {
Some(node) => match node {
ast_map::NodeItem(item) => match item.node {
// FIXME: handling method
ast::ItemFn(ref fn_decl, _, _, ref gen, _) => {
Some((item, fn_decl, gen))
ast::ItemFn(ref fn_decl, ref pur, _, ref gen, _) => {
Some((fn_decl, gen, *pur, item.ident, None, item.span))
},
_ => None
},
ast_map::NodeMethod(m) => {
Some((&m.decl, &m.generics, m.fn_style,
m.ident, Some(m.explicit_self.node), m.span))
},
_ => None
},
None => None
};
let (item, fn_decl, generics) = node_inner.expect("expect item fn");
let rebuilder = Rebuilder::new(self.tcx, *fn_decl,
generics, same_regions);
let (inputs, output, generics) = rebuilder.rebuild();
self.give_expl_lifetime_param(inputs, output, item, generics);
let (fn_decl, generics, fn_style, ident, expl_self, span)
= node_inner.expect("expect item fn");
let taken = lifetimes_in_scope(self.tcx, scope_id);
let life_giver = LifeGiver::with_taken(taken.as_slice());
let rebuilder = Rebuilder::new(self.tcx, *fn_decl, expl_self,
generics, same_regions, &life_giver);
let (fn_decl, expl_self, generics) = rebuilder.rebuild();
self.give_expl_lifetime_param(&fn_decl, fn_style, ident,
expl_self, &generics, span);
}
}

Expand All @@ -694,53 +714,98 @@ struct RebuildPathInfo<'a> {
struct Rebuilder<'a> {
tcx: &'a ty::ctxt,
fn_decl: ast::P<ast::FnDecl>,
expl_self_opt: Option<ast::ExplicitSelf_>,
generics: &'a ast::Generics,
same_regions: &'a [SameRegions],
life_giver: LifeGiver,
life_giver: &'a LifeGiver,
cur_anon: Cell<uint>,
inserted_anons: RefCell<HashSet<uint>>,
}

enum FreshOrKept {
Fresh,
Kept
}

impl<'a> Rebuilder<'a> {
fn new(tcx: &'a ty::ctxt,
fn_decl: ast::P<ast::FnDecl>,
expl_self_opt: Option<ast::ExplicitSelf_>,
generics: &'a ast::Generics,
same_regions: &'a [SameRegions])
same_regions: &'a [SameRegions],
life_giver: &'a LifeGiver)
-> Rebuilder<'a> {
Rebuilder {
tcx: tcx,
fn_decl: fn_decl,
expl_self_opt: expl_self_opt,
generics: generics,
same_regions: same_regions,
life_giver: LifeGiver::with_taken(generics.lifetimes.as_slice()),
life_giver: life_giver,
cur_anon: Cell::new(0),
inserted_anons: RefCell::new(HashSet::new()),
}
}

fn rebuild(&self) -> (Vec<ast::Arg>, ast::P<ast::Ty>, ast::Generics) {
fn rebuild(&self)
-> (ast::FnDecl, Option<ast::ExplicitSelf_>, ast::Generics) {
let mut expl_self_opt = self.expl_self_opt;
let mut inputs = self.fn_decl.inputs.clone();
let mut output = self.fn_decl.output;
let mut ty_params = self.generics.ty_params.clone();
let mut kept_lifetimes = HashSet::new();
for sr in self.same_regions.iter() {
self.cur_anon.set(0);
self.offset_cur_anon();
let (anon_nums, region_names) =
self.extract_anon_nums_and_names(sr);
let lifetime = self.life_giver.give_lifetime();
let (lifetime, fresh_or_kept) = self.pick_lifetime(&region_names);
match fresh_or_kept {
Kept => { kept_lifetimes.insert(lifetime.name); }
_ => ()
}
expl_self_opt = self.rebuild_expl_self(expl_self_opt, lifetime,
&anon_nums, &region_names);
inputs = self.rebuild_args_ty(inputs.as_slice(), lifetime,
&anon_nums, &region_names);
output = self.rebuild_arg_ty_or_output(output, lifetime,
&anon_nums, &region_names);
ty_params = self.rebuild_ty_params(ty_params, lifetime,
&region_names);
}
let generated_lifetimes = self.life_giver.get_generated_lifetimes();
let fresh_lifetimes = self.life_giver.get_generated_lifetimes();
let all_region_names = self.extract_all_region_names();
let generics = self.rebuild_generics(self.generics,
generated_lifetimes,
&all_region_names, ty_params);
(inputs, output, generics)
&fresh_lifetimes,
&kept_lifetimes,
&all_region_names,
ty_params);
let new_fn_decl = ast::FnDecl {
inputs: inputs,
output: output,
cf: self.fn_decl.cf,
variadic: self.fn_decl.variadic
};
(new_fn_decl, expl_self_opt, generics)
}

fn pick_lifetime(&self,
region_names: &HashSet<ast::Name>)
-> (ast::Lifetime, FreshOrKept) {
if region_names.len() > 0 {
// It's not necessary to convert the set of region names to a
// vector of string and then sort them. However, it makes the
// choice of lifetime name deterministic and thus easier to test.
let mut names = Vec::new();
for rn in region_names.iter() {
let lt_name = token::get_name(*rn).get().to_owned();
names.push(lt_name);
}
names.sort();
let name = token::str_to_ident(names.get(0).as_slice()).name;
return (name_to_dummy_lifetime(name), Kept);
}
return (self.life_giver.give_lifetime(), Fresh);
}

fn extract_anon_nums_and_names(&self, same_regions: &SameRegions)
Expand Down Expand Up @@ -849,9 +914,38 @@ impl<'a> Rebuilder<'a> {
})
}

fn rebuild_expl_self(&self,
expl_self_opt: Option<ast::ExplicitSelf_>,
lifetime: ast::Lifetime,
anon_nums: &HashSet<uint>,
region_names: &HashSet<ast::Name>)
-> Option<ast::ExplicitSelf_> {
match expl_self_opt {
Some(expl_self) => match expl_self {
ast::SelfRegion(lt_opt, muta) => match lt_opt {
Some(lt) => if region_names.contains(&lt.name) {
return Some(ast::SelfRegion(Some(lifetime), muta));
},
None => {
let anon = self.cur_anon.get();
self.inc_and_offset_cur_anon(1);
if anon_nums.contains(&anon) {
self.track_anon(anon);
return Some(ast::SelfRegion(Some(lifetime), muta));
}
}
},
_ => ()
},
None => ()
}
expl_self_opt
}

fn rebuild_generics(&self,
generics: &ast::Generics,
add: Vec<ast::Lifetime>,
add: &Vec<ast::Lifetime>,
keep: &HashSet<ast::Name>,
remove: &HashSet<ast::Name>,
ty_params: OwnedSlice<ast::TyParam>)
-> ast::Generics {
Expand All @@ -860,7 +954,7 @@ impl<'a> Rebuilder<'a> {
lifetimes.push(*lt);
}
for lt in generics.lifetimes.iter() {
if !remove.contains(&lt.name) {
if keep.contains(&lt.name) || !remove.contains(&lt.name) {
lifetimes.push((*lt).clone());
}
}
Expand Down Expand Up @@ -1099,29 +1193,17 @@ impl<'a> Rebuilder<'a> {

impl<'a> ErrorReportingHelpers for InferCtxt<'a> {
fn give_expl_lifetime_param(&self,
inputs: Vec<ast::Arg>,
output: ast::P<ast::Ty>,
item: ast::P<ast::Item>,
generics: ast::Generics) {
let (fn_decl, fn_style, ident) = match item.node {
// FIXME: handling method
ast::ItemFn(ref fn_decl, ref fn_style, _, _, _) => {
(fn_decl, fn_style, item.ident)
},
_ => fail!("Expect function or method")

};
let fd = ast::FnDecl {
inputs: inputs,
output: output,
cf: fn_decl.cf,
variadic: fn_decl.variadic
};
let suggested_fn =
pprust::fun_to_str(&fd, *fn_style, ident, None, &generics);
decl: &ast::FnDecl,
fn_style: ast::FnStyle,
ident: ast::Ident,
opt_explicit_self: Option<ast::ExplicitSelf_>,
generics: &ast::Generics,
span: codemap::Span) {
let suggested_fn = pprust::fun_to_str(decl, fn_style, ident,
opt_explicit_self, generics);
let msg = format!("consider using an explicit lifetime \
parameter as shown: {}", suggested_fn);
self.tcx.sess.span_note(item.span, msg);
self.tcx.sess.span_note(span, msg);
}

fn report_inference_failure(&self,
Expand Down Expand Up @@ -1318,6 +1400,47 @@ impl Resolvable for @ty::TraitRef {
}
}

fn lifetimes_in_scope(tcx: &ty::ctxt,
scope_id: ast::NodeId)
-> Vec<ast::Lifetime> {
let mut taken = Vec::new();
let parent = tcx.map.get_parent(scope_id);
let method_id_opt = match tcx.map.find(parent) {
Some(node) => match node {
ast_map::NodeItem(item) => match item.node {
ast::ItemFn(_, _, _, ref gen, _) => {
taken.push_all(gen.lifetimes.as_slice());
None
},
_ => None
},
ast_map::NodeMethod(m) => {
taken.push_all(m.generics.lifetimes.as_slice());
Some(m.id)
},
_ => None
},
None => None
};
if method_id_opt.is_some() {
let method_id = method_id_opt.unwrap();
let parent = tcx.map.get_parent(method_id);
match tcx.map.find(parent) {
Some(node) => match node {
ast_map::NodeItem(item) => match item.node {
ast::ItemImpl(ref gen, _, _, _) => {
taken.push_all(gen.lifetimes.as_slice());
}
_ => ()
},
_ => ()
},
None => ()
}
}
return taken;
}

// LifeGiver is responsible for generating fresh lifetime names
struct LifeGiver {
taken: HashSet<~str>,
Expand All @@ -1326,7 +1449,6 @@ struct LifeGiver {
}

impl LifeGiver {
// FIXME: `taken` needs to include names from higher scope, too
fn with_taken(taken: &[ast::Lifetime]) -> LifeGiver {
let mut taken_ = HashSet::new();
for lt in taken.iter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl<'r> Itble<'r, uint, Range<uint>> for (uint, uint) {
}

fn check<'r, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &T) -> bool {
//~^ NOTE: consider using an explicit lifetime parameter as shown: fn check<'a, I: Iterator<uint>, T: Itble<'a, uint, I>>(cont: &'a T) -> bool
//~^ NOTE: consider using an explicit lifetime parameter as shown: fn check<'r, I: Iterator<uint>, T: Itble<'r, uint, I>>(cont: &'r T) -> bool
let cont_iter = cont.iter(); //~ ERROR: cannot infer
let result = cont_iter.fold(Some(0u16), |state, val| {
state.map_or(None, |mask| {
Expand Down
Loading