Support lifetime suggestion for method

This includes a change to the way lifetime names are generated. Say we
figure that `[#0, 'a, 'b]` have to be the same lifetimes, then instead
of just generating a new lifetime `'c` like before to replace them, we
would reuse `'a`. This is done so that when the lifetime name comes
from an impl, we don't give something that's completely off, and we
don't have to do much work to figure out where the name came from. For
example, for the following code snippet:

```rust
struct Baz<'x> {
    bar: &'x int
}

impl<'x> Baz<'x> {
    fn baz1(&self) -> &int {
        self.bar
    }
}
```

`[#1, 'x]` (where `#1` is BrAnon(1) and refers to lifetime of `&int`)
have to be marked the same lifetime. With the old method, we would
generate a new lifetime `'a` and suggest `fn baz1(&self) -> &'a int`
or `fn baz1<'a>(&self) -> &'a int`, both of which are wrong.

diff --git a/src/librustc/middle/typeck/infer/error_reporting.rs b/src/librustc/middle/typeck/infer/error_reporting.rs
index 95a1f85c7867da1886647dc8f4b1dbf5303f6d9e..91cc0a20813423bd63d7301af1681bafffb2394a 100644 (file)
--- a/src/librustc/middle/typeck/infer/error_reporting.rs
+++ b/src/librustc/middle/typeck/infer/error_reporting.rs
@@ -82,6 +82,7 @@
 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;
@@ -143,10 +144,12 @@ fn note_region_origin(&self,
                           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> {
@@ -260,6 +263,19 @@ struct FreeRegionsFromSameFn {
             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)
@@ -280,17 +296,14 @@ fn free_regions_from_same_fn(tcx: &ty::ctxt,
             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 => {
@@ -662,21 +675,28 @@ fn give_suggestion(&self, same_regions: &[SameRegions]) {
         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);
     }
 }
 
@@ -694,40 +714,58 @@ 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,
@@ -735,12 +773,39 @@ fn rebuild(&self) -> (Vec<ast::Arg>, ast::P<ast::Ty>, ast::Generics) {
             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)
@@ -849,9 +914,38 @@ fn rebuild_ty_param_bounds(&self,
         })
     }
 
+    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 {
@@ -860,7 +954,7 @@ fn rebuild_generics(&self,
             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());
             }
         }
@@ -1099,29 +1193,17 @@ fn rebuild_path(&self,
 
 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,
@@ -1318,6 +1400,47 @@ fn contains_error(&self) -> bool {
     }
 }
 
+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>,
@@ -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() {

diff --git a/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param-2.rs b/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param-2.rs
index a6bf5a4b653d509d48f5c0f830e27385e872d259..a210c5d2f0e40c3e5569320f9834cc6d6439837b 100644 (file)
--- a/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param-2.rs
+++ b/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param-2.rs
@@ -22,7 +22,7 @@ fn iter(&'r self) -> Range<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| {

diff --git a/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param.rs b/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param.rs
index 33b849f346a057f72d6eb178f4672a934e983fff..3328a5d23e62ec1923b89f71cd187d0acdba03ee 100644 (file)
--- a/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param.rs
+++ b/src/test/compile-fail/lifetime-inference-give-expl-lifetime-param.rs
@@ -17,7 +17,7 @@ fn foo1(x: &Foo) -> &int {
 }
 
 fn foo2<'a, 'b>(x: &'a Foo) -> &'b int {
-//~^ NOTE: consider using an explicit lifetime parameter as shown: fn foo2<'c>(x: &'c Foo) -> &'c int
+//~^ NOTE: consider using an explicit lifetime parameter as shown: fn foo2<'a>(x: &'a Foo) -> &'a int
     &x.bar //~ ERROR: cannot infer
 }
 
@@ -28,7 +28,7 @@ fn foo3(x: &Foo) -> (&int, &int) {
 }
 
 fn foo4<'a, 'b>(x: &'a Foo) -> (&'b int, &'a int, &int) {
-//~^ NOTE: consider using an explicit lifetime parameter as shown: fn foo4<'c>(x: &'c Foo) -> (&'c int, &'c int, &'c int)
+//~^ NOTE: consider using an explicit lifetime parameter as shown: fn foo4<'a>(x: &'a Foo) -> (&'a int, &'a int, &'a int)
     (&x.bar, &x.bar, &x.bar) //~ ERROR: cannot infer
     //~^ ERROR: cannot infer
 }
@@ -48,7 +48,7 @@ fn bar1(x: &Bar) -> (&int, &int, &int) {
 }
 
 fn bar2<'a, 'b, 'c>(x: &Bar<'a, 'b, 'c>) -> (&int, &int, &int) {
-//~^ NOTE: consider using an explicit lifetime parameter as shown: fn bar2<'d, 'e, 'a, 'c>(x: &'e Bar<'a, 'd, 'c>) -> (&'d int, &'e int, &'e int)
+//~^ NOTE: consider using an explicit lifetime parameter as shown: fn bar2<'d, 'a, 'b, 'c>(x: &'d Bar<'a, 'b, 'c>) -> (&'b int, &'d int, &'d int)
     (x.bar, &x.baz, &x.baz) //~ ERROR: mismatched types
     //~^ ERROR: cannot infer
     //~^^ ERROR: cannot infer
@@ -62,8 +62,28 @@ fn cat<'x>(x: Cat<'x, Dog>) -> &int {
 }
 
 fn cat2<'x, 'y>(x: Cat<'x, Dog<'y>>) -> &int {
-//~^ NOTE: consider using an explicit lifetime parameter as shown: fn cat2<'a, 'x>(x: Cat<'x, Dog<'a>>) -> &'a int
+//~^ NOTE: consider using an explicit lifetime parameter as shown: fn cat2<'x, 'y>(x: Cat<'x, Dog<'y>>) -> &'y int
     x.t.dog //~ ERROR: mismatched types
 }
 
+struct Baz<'x> {
+    bar: &'x int
+}
+
+impl<'x> Baz<'x> {
+    fn baz1(&self) -> &int {
+    //~^ NOTE: consider using an explicit lifetime parameter as shown: fn baz1(&self) -> &'x int
+        self.bar //~ ERROR: mismatched types
+    }
+}
+
+impl<'a> Baz<'a> {
+    fn baz2(&self, x: &int) -> (&int, &int) {
+    //~^ NOTE: consider using an explicit lifetime parameter as shown: fn baz2<'b>(&self, x: &'b int) -> (&'a int, &'b int)
+        (self.bar, x) //~ ERROR: cannot infer
+        //~^ ERROR: mismatched types
+        //~^^ ERROR: mismatched types
+    }
+}
+
 fn main() {}