From e48973eb9faa1043de3c1dc5a5a86d709180c992 Mon Sep 17 00:00:00 2001 From: Florian Hartwig Date: Wed, 11 Nov 2015 00:12:45 +0100 Subject: [PATCH 1/2] Track elided lifetimes in types and trait objects --- src/lifetimes.rs | 86 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 24 deletions(-) diff --git a/src/lifetimes.rs b/src/lifetimes.rs index 206424de9b58..229d13401c67 100644 --- a/src/lifetimes.rs +++ b/src/lifetimes.rs @@ -3,6 +3,7 @@ use reexport::*; use rustc::lint::*; use syntax::codemap::Span; use rustc_front::visit::{Visitor, walk_ty, walk_ty_param_bound}; +use rustc::middle::def::Def::{DefTy, DefTrait}; use std::collections::HashSet; use utils::{in_external_macro, span_lint}; @@ -53,16 +54,16 @@ use self::RefLt::*; fn check_fn_inner(cx: &LateContext, decl: &FnDecl, slf: Option<&ExplicitSelf>, generics: &Generics, span: Span) { - if in_external_macro(cx, span) || has_where_lifetimes(&generics.where_clause) { + if in_external_macro(cx, span) || has_where_lifetimes(cx, &generics.where_clause) { return; } - if could_use_elision(decl, slf, &generics.lifetimes) { + if could_use_elision(cx, decl, slf, &generics.lifetimes) { span_lint(cx, NEEDLESS_LIFETIMES, span, "explicit lifetimes given in parameter types where they could be elided"); } } -fn could_use_elision(func: &FnDecl, slf: Option<&ExplicitSelf>, +fn could_use_elision(cx: &LateContext, func: &FnDecl, slf: Option<&ExplicitSelf>, named_lts: &[LifetimeDef]) -> bool { // There are two scenarios where elision works: // * no output references, all input references have different LT @@ -74,8 +75,8 @@ fn could_use_elision(func: &FnDecl, slf: Option<&ExplicitSelf>, let allowed_lts = allowed_lts_from(named_lts); // these will collect all the lifetimes for references in arg/return types - let mut input_visitor = RefVisitor(Vec::new()); - let mut output_visitor = RefVisitor(Vec::new()); + let mut input_visitor = RefVisitor::new(cx); + let mut output_visitor = RefVisitor::new(cx); // extract lifetime in "self" argument for methods (there is a "self" argument // in func.inputs, but its type is TyInfer) @@ -88,14 +89,11 @@ fn could_use_elision(func: &FnDecl, slf: Option<&ExplicitSelf>, } // extract lifetimes in input argument types for arg in &func.inputs { - walk_ty(&mut input_visitor, &arg.ty); - if let TyRptr(None, _) = arg.ty.node { - input_visitor.record(&None); - } + input_visitor.visit_ty(&arg.ty); } // extract lifetimes in output type if let Return(ref ty) = func.output { - walk_ty(&mut output_visitor, ty); + output_visitor.visit_ty(ty); } let input_lts = input_visitor.into_vec(); @@ -159,35 +157,75 @@ fn unique_lifetimes(lts: &[RefLt]) -> usize { } /// A visitor usable for rustc_front::visit::walk_ty(). -struct RefVisitor(Vec); +struct RefVisitor<'v, 't: 'v> { + cx: &'v LateContext<'v, 't>, // context reference + lts: Vec +} + +impl <'v, 't> RefVisitor<'v, 't> { + fn new(cx: &'v LateContext<'v, 't>) -> RefVisitor<'v, 't> { + RefVisitor { cx: cx, lts: Vec::new() } + } -impl RefVisitor { fn record(&mut self, lifetime: &Option) { if let &Some(ref lt) = lifetime { if lt.name.as_str() == "'static" { - self.0.push(Static); + self.lts.push(Static); } else { - self.0.push(Named(lt.name)); + self.lts.push(Named(lt.name)); } } else { - self.0.push(Unnamed); + self.lts.push(Unnamed); } } fn into_vec(self) -> Vec { - self.0 + self.lts + } + + fn collect_anonymous_lifetimes(&mut self, path: &Path, ty: &Ty) { + let last_path_segment = path.segments.last().map(|s| &s.parameters); + if let Some(&AngleBracketedParameters(ref params)) = last_path_segment { + if params.lifetimes.is_empty() { + let def = self.cx.tcx.def_map.borrow().get(&ty.id).map(|r| r.full_def()); + match def { + Some(DefTy(def_id, _)) => { + if let Some(ty_def) = self.cx.tcx.adt_defs.borrow().get(&def_id) { + let scheme = ty_def.type_scheme(self.cx.tcx); + for _ in scheme.generics.regions.as_slice() { + self.record(&None); + } + } + }, + Some(DefTrait(def_id)) => { + let trait_def = self.cx.tcx.trait_defs.borrow()[&def_id]; + for _ in &trait_def.generics.regions { + self.record(&None); + } + }, + _ => {} + } + } + } } } -impl<'v> Visitor<'v> for RefVisitor { +impl<'v, 't> Visitor<'v> for RefVisitor<'v, 't> { + // for lifetimes as parameters of generics fn visit_lifetime(&mut self, lifetime: &'v Lifetime) { self.record(&Some(*lifetime)); } fn visit_ty(&mut self, ty: &'v Ty) { - if let TyRptr(None, _) = ty.node { - self.record(&None); + match ty.node { + TyRptr(None, _) => { + self.record(&None); + }, + TyPath(_, ref path) => { + self.collect_anonymous_lifetimes(path, ty); + }, + _ => {} } walk_ty(self, ty); } @@ -195,16 +233,16 @@ impl<'v> Visitor<'v> for RefVisitor { /// Are any lifetimes mentioned in the `where` clause? If yes, we don't try to /// reason about elision. -fn has_where_lifetimes(where_clause: &WhereClause) -> bool { +fn has_where_lifetimes(cx: &LateContext, where_clause: &WhereClause) -> bool { for predicate in &where_clause.predicates { match *predicate { WherePredicate::RegionPredicate(..) => return true, WherePredicate::BoundPredicate(ref pred) => { // a predicate like F: Trait or F: for<'a> Trait<'a> - let mut visitor = RefVisitor(Vec::new()); + let mut visitor = RefVisitor::new(cx); // walk the type F, it may not contain LT refs walk_ty(&mut visitor, &pred.bounded_ty); - if !visitor.0.is_empty() { return true; } + if !visitor.lts.is_empty() { return true; } // if the bounds define new lifetimes, they are fine to occur let allowed_lts = allowed_lts_from(&pred.bound_lifetimes); // now walk the bounds @@ -219,9 +257,9 @@ fn has_where_lifetimes(where_clause: &WhereClause) -> bool { } } WherePredicate::EqPredicate(ref pred) => { - let mut visitor = RefVisitor(Vec::new()); + let mut visitor = RefVisitor::new(cx); walk_ty(&mut visitor, &pred.ty); - if !visitor.0.is_empty() { return true; } + if !visitor.lts.is_empty() { return true; } } } } From 6046edbc234272dd2eeb7a98028e499e57f2a255 Mon Sep 17 00:00:00 2001 From: Florian Hartwig Date: Wed, 11 Nov 2015 00:26:22 +0100 Subject: [PATCH 2/2] Add some tests for lifetime elision lint with types and traits with lifetimes --- tests/compile-fail/lifetimes.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/compile-fail/lifetimes.rs b/tests/compile-fail/lifetimes.rs index a654c4523797..f5d95aacc9a3 100755 --- a/tests/compile-fail/lifetimes.rs +++ b/tests/compile-fail/lifetimes.rs @@ -85,5 +85,26 @@ fn already_elided<'a>(_: &u8, _: &'a u8) -> &'a u8 { unimplemented!() } +fn struct_with_lt<'a>(_foo: Foo<'a>) -> &'a str { unimplemented!() } //~ERROR explicit lifetimes given + +// no warning, two input lifetimes (named on the reference, anonymous on Foo) +fn struct_with_lt2<'a>(_foo: &'a Foo) -> &'a str { unimplemented!() } + +// no warning, two input lifetimes (anonymous on the reference, named on Foo) +fn struct_with_lt3<'a>(_foo: &Foo<'a> ) -> &'a str { unimplemented!() } + +// no warning, two input lifetimes +fn struct_with_lt4<'a, 'b>(_foo: &'a Foo<'b> ) -> &'a str { unimplemented!() } + +trait WithLifetime<'a> {} +type WithLifetimeAlias<'a> = WithLifetime<'a>; + +// should not warn because it won't build without the lifetime +fn trait_obj_elided<'a>(_arg: &'a WithLifetime) -> &'a str { unimplemented!() } + +// this should warn because there is no lifetime on Drop, so this would be +// unambiguous if we elided the lifetime +fn trait_obj_elided2<'a>(_arg: &'a Drop) -> &'a str { unimplemented!() } //~ERROR explicit lifetimes given + fn main() { }