From e741dad62990660f2f9a3378e695dfb5e03320ef Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 23 Nov 2017 08:05:58 -0500 Subject: [PATCH] adding lint for single use lifetime names --- src/librustc/lint/builtin.rs | 9 +- src/librustc/middle/resolve_lifetime.rs | 94 ++++++++++++++++--- .../single_use_lifetimes-2.rs | 16 ++++ .../single_use_lifetimes-2.stderr | 14 +++ .../single_use_lifetimes-3.rs | 20 ++++ .../single_use_lifetimes-3.stderr | 20 ++++ .../single_use_lifetimes-4.rs | 20 ++++ .../single_use_lifetimes-4.stderr | 20 ++++ .../single_use_lifetimes-5.rs | 16 ++++ .../single_use_lifetimes-5.stderr | 14 +++ .../in-band-lifetimes/single_use_lifetimes.rs | 16 ++++ .../single_use_lifetimes.stderr | 14 +++ 12 files changed, 258 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-2.rs create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-2.stderr create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-3.rs create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-3.stderr create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-4.rs create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-4.stderr create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-5.rs create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes-5.stderr create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes.rs create mode 100644 src/test/ui/in-band-lifetimes/single_use_lifetimes.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index d352d359e2021..de0bd9d029597 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -234,6 +234,12 @@ declare_lint! { "detect coercion to !" } +declare_lint! { + pub SINGLE_USE_LIFETIME, + Allow, + "detects single use lifetimes" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// which are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -277,7 +283,8 @@ impl LintPass for HardwiredLints { DEPRECATED, UNUSED_UNSAFE, UNUSED_MUT, - COERCE_NEVER + COERCE_NEVER, + SINGLE_USE_LIFETIME ) } } diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index d9b2bf62841f1..dfa739e69ca7d 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -31,6 +31,7 @@ use syntax_pos::Span; use errors::DiagnosticBuilder; use util::nodemap::{DefIdMap, FxHashMap, FxHashSet, NodeMap, NodeSet}; use std::slice; +use rustc::lint; use hir; use hir::intravisit::{self, NestedVisitorMap, Visitor}; @@ -56,6 +57,13 @@ impl LifetimeDefOrigin { } } +// This counts the no of times a lifetime is used +#[derive(Clone, Copy, Debug)] +pub enum LifetimeUseSet<'tcx> { + One(&'tcx hir::Lifetime), + Many, +} + #[derive(Clone, Copy, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, Debug)] pub enum Region { Static, @@ -245,6 +253,8 @@ struct LifetimeContext<'a, 'tcx: 'a> { // Cache for cross-crate per-definition object lifetime defaults. xcrate_object_lifetime_defaults: DefIdMap>, + + lifetime_uses: DefIdMap>, } #[derive(Debug)] @@ -407,6 +417,7 @@ fn krate<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>) -> NamedRegionMap { is_in_fn_syntax: false, labels_in_fn: vec![], xcrate_object_lifetime_defaults: DefIdMap(), + lifetime_uses: DefIdMap(), }; for (_, item) in &krate.items { visitor.visit_item(item); @@ -443,8 +454,11 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { fn visit_item(&mut self, item: &'tcx hir::Item) { match item.node { hir::ItemFn(ref decl, _, _, _, ref generics, _) => { - self.visit_early_late(None, decl, generics, |this| { - intravisit::walk_item(this, item); + self.visit_early_late(None, + decl, + generics, + |this| { + intravisit::walk_item(this, item); }); } hir::ItemExternCrate(_) @@ -498,9 +512,12 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { fn visit_foreign_item(&mut self, item: &'tcx hir::ForeignItem) { match item.node { hir::ForeignItemFn(ref decl, _, ref generics) => { - self.visit_early_late(None, decl, generics, |this| { - intravisit::walk_foreign_item(this, item); - }) + self.visit_early_late(None, + decl, + generics, + |this| { + intravisit::walk_foreign_item(this, item); + }) } hir::ForeignItemStatic(..) => { intravisit::walk_foreign_item(self, item); @@ -1142,12 +1159,41 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { is_in_fn_syntax: self.is_in_fn_syntax, labels_in_fn, xcrate_object_lifetime_defaults, + lifetime_uses: DefIdMap(), }; debug!("entering scope {:?}", this.scope); f(self.scope, &mut this); debug!("exiting scope {:?}", this.scope); self.labels_in_fn = this.labels_in_fn; self.xcrate_object_lifetime_defaults = this.xcrate_object_lifetime_defaults; + + for (def_id, lifetimeuseset) in &this.lifetime_uses { + match lifetimeuseset { + &LifetimeUseSet::One(_) => { + let node_id = this.tcx.hir.as_local_node_id(*def_id).unwrap(); + debug!("node id first={:?}", node_id); + if let hir::map::NodeLifetime(hir_lifetime) = this.tcx.hir.get(node_id) { + let span = hir_lifetime.span; + let id = hir_lifetime.id; + debug!("id ={:?} span = {:?} hir_lifetime = {:?}", + node_id, + span, + hir_lifetime); + + this.tcx + .struct_span_lint_node(lint::builtin::SINGLE_USE_LIFETIME, + id, + span, + &format!("lifetime name `{}` only used once", + hir_lifetime.name.name())) + .emit(); + } + } + _ => { + debug!("Not one use lifetime"); + } + } + } } /// Visits self by adding a scope and handling recursive walk over the contents with `walk`. @@ -1239,9 +1285,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } } - fn resolve_lifetime_ref(&mut self, lifetime_ref: &hir::Lifetime) { + fn resolve_lifetime_ref(&mut self, lifetime_ref: &'tcx hir::Lifetime) { debug!("resolve_lifetime_ref(lifetime_ref={:?})", lifetime_ref); - // Walk up the scope chain, tracking the number of fn scopes // that we pass through, until we find a lifetime with the // given name or we run out of scopes. @@ -1533,8 +1578,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } // Foreign functions, `fn(...) -> R` and `Trait(...) -> R` (both types and bounds). - hir::map::NodeForeignItem(_) | hir::map::NodeTy(_) | hir::map::NodeTraitRef(_) => None, - + hir::map::NodeForeignItem(_) | hir::map::NodeTy(_) | hir::map::NodeTraitRef(_) => + None, // Everything else (only closures?) doesn't // actually enjoy elision in return types. _ => { @@ -1710,7 +1755,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } } - fn resolve_elided_lifetimes(&mut self, lifetime_refs: &[hir::Lifetime]) { + fn resolve_elided_lifetimes(&mut self, lifetime_refs: &'tcx [hir::Lifetime]) { if lifetime_refs.is_empty() { return; } @@ -1865,7 +1910,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } } - fn resolve_object_lifetime_default(&mut self, lifetime_ref: &hir::Lifetime) { + fn resolve_object_lifetime_default(&mut self, lifetime_ref: &'tcx hir::Lifetime) { let mut late_depth = 0; let mut scope = self.scope; let lifetime = loop { @@ -1887,7 +1932,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { self.insert_lifetime(lifetime_ref, lifetime.shifted(late_depth)); } - fn check_lifetime_defs(&mut self, old_scope: ScopeRef, lifetimes: &[hir::LifetimeDef]) { + fn check_lifetime_defs(&mut self, old_scope: ScopeRef, lifetimes: &'tcx [hir::LifetimeDef]) { for i in 0..lifetimes.len() { let lifetime_i = &lifetimes[i]; @@ -1971,7 +2016,9 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } } - fn check_lifetime_def_for_shadowing(&self, mut old_scope: ScopeRef, lifetime: &hir::Lifetime) { + fn check_lifetime_def_for_shadowing(&self, + mut old_scope: ScopeRef, + lifetime: &'tcx hir::Lifetime) { for &(label, label_span) in &self.labels_in_fn { // FIXME (#24278): non-hygienic comparison if lifetime.name.name() == label { @@ -2020,7 +2067,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { } } - fn insert_lifetime(&mut self, lifetime_ref: &hir::Lifetime, def: Region) { + fn insert_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime, def: Region) { if lifetime_ref.id == ast::DUMMY_NODE_ID { span_bug!( lifetime_ref.span, @@ -2036,6 +2083,25 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { self.tcx.sess.codemap().span_to_string(lifetime_ref.span) ); self.map.defs.insert(lifetime_ref.id, def); + + match def { + Region::LateBoundAnon(..) | + Region::Static => { + // These are anonymous lifetimes or lifetimes that are not declared. + } + + Region::Free(_, def_id) | + Region::LateBound(_, def_id, _) | + Region::EarlyBound(_, def_id, _) => { + // A lifetime declared by the user. + if !self.lifetime_uses.contains_key(&def_id) { + self.lifetime_uses + .insert(def_id, LifetimeUseSet::One(lifetime_ref)); + } else { + self.lifetime_uses.insert(def_id, LifetimeUseSet::Many); + } + } + } } } diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-2.rs b/src/test/ui/in-band-lifetimes/single_use_lifetimes-2.rs new file mode 100644 index 0000000000000..005f1f033b6eb --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-2.rs @@ -0,0 +1,16 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![deny(single_use_lifetime)] +// FIXME(#44752) -- this scenario should not be warned +fn deref<'x>() -> &'x u32 { //~ ERROR lifetime name `'x` only used once + 22 +} + +fn main() { } diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-2.stderr b/src/test/ui/in-band-lifetimes/single_use_lifetimes-2.stderr new file mode 100644 index 0000000000000..a90add79b76aa --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-2.stderr @@ -0,0 +1,14 @@ +error: lifetime name `'x` only used once + --> $DIR/single_use_lifetimes-2.rs:12:10 + | +12 | fn deref<'x>() -> &'x u32 { //~ ERROR lifetime name `'x` only used once + | ^^ + | +note: lint level defined here + --> $DIR/single_use_lifetimes-2.rs:10:9 + | +10 | #![deny(single_use_lifetime)] + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-3.rs b/src/test/ui/in-band-lifetimes/single_use_lifetimes-3.rs new file mode 100644 index 0000000000000..263548ca7f4dd --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-3.rs @@ -0,0 +1,20 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![deny(single_use_lifetime)] +struct Foo<'x> { //~ ERROR lifetime name `'x` only used once + x: &'x u32 // no warning! +} + +// Once #44524 is fixed, this should issue a warning. +impl<'y> Foo<'y> { //~ ERROR lifetime name `'y` only used once + fn method() { } +} + +fn main() { } diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-3.stderr b/src/test/ui/in-band-lifetimes/single_use_lifetimes-3.stderr new file mode 100644 index 0000000000000..8595ce5effb78 --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-3.stderr @@ -0,0 +1,20 @@ +error: lifetime name `'x` only used once + --> $DIR/single_use_lifetimes-3.rs:11:12 + | +11 | struct Foo<'x> { //~ ERROR lifetime name `'x` only used once + | ^^ + | +note: lint level defined here + --> $DIR/single_use_lifetimes-3.rs:10:9 + | +10 | #![deny(single_use_lifetime)] + | ^^^^^^^^^^^^^^^^^^^ + +error: lifetime name `'y` only used once + --> $DIR/single_use_lifetimes-3.rs:16:6 + | +16 | impl<'y> Foo<'y> { //~ ERROR lifetime name `'y` only used once + | ^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-4.rs b/src/test/ui/in-band-lifetimes/single_use_lifetimes-4.rs new file mode 100644 index 0000000000000..ead987a09ab97 --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-4.rs @@ -0,0 +1,20 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![deny(single_use_lifetime)] + // Neither should issue a warning, as explicit lifetimes are mandatory in this case +struct Foo<'x> { //~ ERROR lifetime name `'x` only used once + x: &'x u32 +} + +enum Bar<'x> { //~ ERROR lifetime name `'x` only used once + Variant(&'x u32) +} + +fn main() { } \ No newline at end of file diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-4.stderr b/src/test/ui/in-band-lifetimes/single_use_lifetimes-4.stderr new file mode 100644 index 0000000000000..1b952c8db09d7 --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-4.stderr @@ -0,0 +1,20 @@ +error: lifetime name `'x` only used once + --> $DIR/single_use_lifetimes-4.rs:12:12 + | +12 | struct Foo<'x> { //~ ERROR lifetime name `'x` only used once + | ^^ + | +note: lint level defined here + --> $DIR/single_use_lifetimes-4.rs:10:9 + | +10 | #![deny(single_use_lifetime)] + | ^^^^^^^^^^^^^^^^^^^ + +error: lifetime name `'x` only used once + --> $DIR/single_use_lifetimes-4.rs:16:10 + | +16 | enum Bar<'x> { //~ ERROR lifetime name `'x` only used once + | ^^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-5.rs b/src/test/ui/in-band-lifetimes/single_use_lifetimes-5.rs new file mode 100644 index 0000000000000..cef904c48962c --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-5.rs @@ -0,0 +1,16 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![deny(single_use_lifetime)] +// Should not issue a warning, as explicit lifetimes are mandatory in this case: +trait Foo<'x> { //~ ERROR lifetime name `'x` only used once + fn foo(&self, arg: &'x u32); +} + +fn main() { } diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes-5.stderr b/src/test/ui/in-band-lifetimes/single_use_lifetimes-5.stderr new file mode 100644 index 0000000000000..59e1254836ba9 --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes-5.stderr @@ -0,0 +1,14 @@ +error: lifetime name `'x` only used once + --> $DIR/single_use_lifetimes-5.rs:12:11 + | +12 | trait Foo<'x> { //~ ERROR lifetime name `'x` only used once + | ^^ + | +note: lint level defined here + --> $DIR/single_use_lifetimes-5.rs:10:9 + | +10 | #![deny(single_use_lifetime)] + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes.rs b/src/test/ui/in-band-lifetimes/single_use_lifetimes.rs new file mode 100644 index 0000000000000..96aaf8923f140 --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes.rs @@ -0,0 +1,16 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +#![deny(single_use_lifetime)] + +fn deref<'x>(v: &'x u32) -> u32 { //~ ERROR lifetime name `'x` only used once + *v +} + +fn main() {} \ No newline at end of file diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes.stderr b/src/test/ui/in-band-lifetimes/single_use_lifetimes.stderr new file mode 100644 index 0000000000000..bdcce1f22ecee --- /dev/null +++ b/src/test/ui/in-band-lifetimes/single_use_lifetimes.stderr @@ -0,0 +1,14 @@ +error: lifetime name `'x` only used once + --> $DIR/single_use_lifetimes.rs:12:10 + | +12 | fn deref<'x>(v: &'x u32) -> u32 { //~ ERROR lifetime name `'x` only used once + | ^^ + | +note: lint level defined here + --> $DIR/single_use_lifetimes.rs:10:9 + | +10 | #![deny(single_use_lifetime)] + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error +