Merge pull request #1672 from Manishearth/lifetime_sugg

Fix various false positives around needless_lifetime
This commit is contained in:
Oliver Schneider 2017-04-12 10:37:57 +02:00 committed by GitHub
commit 7f89fb7a20
4 changed files with 111 additions and 40 deletions

View file

@ -98,8 +98,8 @@ pub fn get_argument_fmtstr_parts<'a, 'b>(cx: &LateContext<'a, 'b>, expr: &'a Exp
}
/// Checks if the expressions matches
/// ```rust
/// { static __STATIC_FMTSTR: s = &["a", "b", c]; __STATIC_FMTSTR }
/// ```rust, ignore
/// { static __STATIC_FMTSTR: &'static[&'static str] = &["a", "b", c]; __STATIC_FMTSTR }
/// ```
fn check_static_str(cx: &LateContext, expr: &Expr) -> bool {
if let Some(expr) = get_argument_fmtstr_parts(cx, expr) {

View file

@ -11,7 +11,6 @@
#![feature(conservative_impl_trait)]
#![allow(indexing_slicing, shadow_reuse, unknown_lints, missing_docs_in_private_items)]
#![allow(needless_lifetimes)]
extern crate syntax;
extern crate syntax_pos;

View file

@ -6,6 +6,7 @@ use rustc::hir::intravisit::{Visitor, walk_ty, walk_ty_param_bound, walk_fn_decl
use std::collections::{HashSet, HashMap};
use syntax::codemap::Span;
use utils::{in_external_macro, span_lint, last_path_segment};
use syntax::symbol::keywords;
/// **What it does:** Checks for lifetime annotations which can be removed by
/// relying on lifetime elision.
@ -58,20 +59,24 @@ impl LintPass for LifetimePass {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LifetimePass {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
if let ItemFn(ref decl, _, _, _, ref generics, _) = item.node {
check_fn_inner(cx, decl, generics, item.span);
if let ItemFn(ref decl, _, _, _, ref generics, id) = item.node {
check_fn_inner(cx, decl, Some(id), generics, item.span);
}
}
fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx ImplItem) {
if let ImplItemKind::Method(ref sig, _) = item.node {
check_fn_inner(cx, &sig.decl, &sig.generics, item.span);
if let ImplItemKind::Method(ref sig, id) = item.node {
check_fn_inner(cx, &sig.decl, Some(id), &sig.generics, item.span);
}
}
fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx TraitItem) {
if let TraitItemKind::Method(ref sig, _) = item.node {
check_fn_inner(cx, &sig.decl, &sig.generics, item.span);
if let TraitItemKind::Method(ref sig, ref body) = item.node {
let body = match *body {
TraitMethod::Required(_) => None,
TraitMethod::Provided(id) => Some(id),
};
check_fn_inner(cx, &sig.decl, body, &sig.generics, item.span);
}
}
}
@ -84,30 +89,32 @@ enum RefLt {
Named(Name),
}
fn bound_lifetimes(bound: &TyParamBound) -> HirVec<&Lifetime> {
if let TraitTyParamBound(ref trait_ref, _) = *bound {
trait_ref.trait_ref
.path
.segments
.last()
.expect("a path must have at least one segment")
.parameters
.lifetimes()
} else {
HirVec::new()
}
}
fn check_fn_inner<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, decl: &'tcx FnDecl, generics: &'tcx Generics, span: Span) {
fn check_fn_inner<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, decl: &'tcx FnDecl, body: Option<BodyId>, generics: &'tcx Generics, span: Span) {
if in_external_macro(cx, span) || has_where_lifetimes(cx, &generics.where_clause) {
return;
}
let bounds_lts = generics.ty_params
.iter()
.flat_map(|typ| typ.bounds.iter().flat_map(bound_lifetimes));
if could_use_elision(cx, decl, &generics.lifetimes, bounds_lts) {
let mut bounds_lts = Vec::new();
for typ in &generics.ty_params {
for bound in &typ.bounds {
if let TraitTyParamBound(ref trait_ref, _) = *bound {
let bounds = trait_ref.trait_ref
.path
.segments
.last()
.expect("a path must have at least one segment")
.parameters
.lifetimes();
for bound in bounds {
if bound.name != "'static" && !bound.is_elided() {
return;
}
bounds_lts.push(bound);
}
}
}
}
if could_use_elision(cx, decl, body, &generics.lifetimes, bounds_lts) {
span_lint(cx,
NEEDLESS_LIFETIMES,
span,
@ -116,11 +123,12 @@ fn check_fn_inner<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, decl: &'tcx FnDecl, gene
report_extra_lifetimes(cx, decl, generics);
}
fn could_use_elision<'a, 'tcx: 'a, T: Iterator<Item = &'tcx Lifetime>>(
fn could_use_elision<'a, 'tcx: 'a>(
cx: &LateContext<'a, 'tcx>,
func: &'tcx FnDecl,
body: Option<BodyId>,
named_lts: &'tcx [LifetimeDef],
bounds_lts: T
bounds_lts: Vec<&'tcx Lifetime>,
) -> bool {
// There are two scenarios where elision works:
// * no output references, all input references have different LT
@ -144,8 +152,22 @@ fn could_use_elision<'a, 'tcx: 'a, T: Iterator<Item = &'tcx Lifetime>>(
output_visitor.visit_ty(ty);
}
let input_lts = lts_from_bounds(input_visitor.into_vec(), bounds_lts);
let output_lts = output_visitor.into_vec();
let input_lts = match input_visitor.into_vec() {
Some(lts) => lts_from_bounds(lts, bounds_lts.into_iter()),
None => return false,
};
let output_lts = match output_visitor.into_vec() {
Some(val) => val,
None => return false,
};
if let Some(body_id) = body {
let mut checker = BodyLifetimeChecker { lifetimes_used_in_body: false };
checker.visit_expr(&cx.tcx.hir.body(body_id).value);
if checker.lifetimes_used_in_body {
return false;
}
}
// check for lifetimes from higher scopes
for lt in input_lts.iter().chain(output_lts.iter()) {
@ -216,6 +238,7 @@ fn unique_lifetimes(lts: &[RefLt]) -> usize {
struct RefVisitor<'a, 'tcx: 'a> {
cx: &'a LateContext<'a, 'tcx>,
lts: Vec<RefLt>,
abort: bool,
}
impl<'v, 't> RefVisitor<'v, 't> {
@ -223,6 +246,7 @@ impl<'v, 't> RefVisitor<'v, 't> {
RefVisitor {
cx: cx,
lts: Vec::new(),
abort: false,
}
}
@ -240,8 +264,12 @@ impl<'v, 't> RefVisitor<'v, 't> {
}
}
fn into_vec(self) -> Vec<RefLt> {
self.lts
fn into_vec(self) -> Option<Vec<RefLt>> {
if self.abort {
None
} else {
Some(self.lts)
}
}
fn collect_anonymous_lifetimes(&mut self, qpath: &QPath, ty: &Ty) {
@ -292,7 +320,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> {
},
TyTraitObject(ref bounds, ref lt) => {
if !lt.is_elided() {
self.record(&Some(*lt));
self.abort = true;
}
for bound in bounds {
self.visit_poly_trait_ref(bound, TraitBoundModifier::None);
@ -329,10 +357,13 @@ fn has_where_lifetimes<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, where_clause: &
walk_ty_param_bound(&mut visitor, bound);
}
// and check that all lifetimes are allowed
for lt in visitor.into_vec() {
if !allowed_lts.contains(&lt) {
return true;
}
match visitor.into_vec() {
None => return false,
Some(lts) => for lt in lts {
if !allowed_lts.contains(&lt) {
return true;
}
},
}
},
WherePredicate::EqPredicate(ref pred) => {
@ -384,3 +415,20 @@ fn report_extra_lifetimes<'a, 'tcx: 'a>(cx: &LateContext<'a, 'tcx>, func: &'tcx
span_lint(cx, UNUSED_LIFETIMES, v, "this lifetime isn't used in the function definition");
}
}
struct BodyLifetimeChecker {
lifetimes_used_in_body: bool,
}
impl<'tcx> Visitor<'tcx> for BodyLifetimeChecker {
// for lifetimes as parameters of generics
fn visit_lifetime(&mut self, lifetime: &'tcx Lifetime) {
if lifetime.name != keywords::Invalid.name() && lifetime.name != "'static" {
self.lifetimes_used_in_body = true;
}
}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None
}
}

View file

@ -128,5 +128,29 @@ fn elided_input_named_output<'a>(_arg: &str) -> &'a str { unimplemented!() }
fn trait_bound_ok<'a, T: WithLifetime<'static>>(_: &'a u8, _: T) { unimplemented!() }
fn trait_bound<'a, T: WithLifetime<'a>>(_: &'a u8, _: T) { unimplemented!() }
// don't warn on these, see #292
fn trait_bound_bug<'a, T: WithLifetime<'a>>() { unimplemented!() }
// #740
struct Test {
vec: Vec<usize>,
}
impl Test {
fn iter<'a>(&'a self) -> Box<Iterator<Item = usize> + 'a> {
unimplemented!()
}
}
trait LintContext<'a> {}
fn f<'a, T: LintContext<'a>>(_: &T) {}
fn test<'a>(x: &'a [u8]) -> u8 {
let y: &'a u8 = &x[5];
*y
}
fn main() {
}