Auto merge of #46441 - gaurikholkar:single_lifetimes, r=nikomatsakis

bors · bors · commit edbd7d232ee1 · 2017-12-20T01:18:17.000Z
Lint against single-use lifetime names This is a fix for #44752 TO-DO - [x] change lint message - [x] add ui tests r? @nikomatsakis
diff --git 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
@@ -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<Vec<ObjectLifetimeDefault>>,
+
+    lifetime_uses: DefIdMap<LifetimeUseSet<'tcx>>,
 }
 
 #[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(_)
@@ -499,9 +513,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);
@@ -1190,12 +1207,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`.
@@ -1287,9 +1333,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.
@@ -1581,8 +1626,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.
             _ => {
@@ -1758,7 +1803,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;
         }
@@ -1913,7 +1958,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 {
@@ -1935,7 +1980,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];
 
@@ -2019,7 +2064,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 {
@@ -2068,7 +2115,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,
@@ -2084,6 +2131,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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
@@ -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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
@@ -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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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() { }
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
@@ -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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
@@ -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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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() {}
diff --git a/src/test/ui/in-band-lifetimes/single_use_lifetimes.stderr b/src/test/ui/in-band-lifetimes/single_use_lifetimes.stderr

Original file line number	Diff line number	Diff line change
`@@ -234,6 +234,12 @@ declare_lint! {`
`234`	`234`	`"detect coercion to !"`
`235`	`235`	`}`
`236`	`236`
	`237`	`+declare_lint! {`
	`238`	`+ pub SINGLE_USE_LIFETIME,`
	`239`	`+ Allow,`
	`240`	`+ "detects single use lifetimes"`
	`241`	`+}`
	`242`	`+`
`237`	`243`	/// Does nothing as a lint pass, but registers some `Lint`s
`238`	`244`	`/// which are used by other parts of the compiler.`
`239`	`245`	`#[derive(Copy, Clone)]`
`@@ -277,7 +283,8 @@ impl LintPass for HardwiredLints {`
`277`	`283`	`DEPRECATED,`
`278`	`284`	`UNUSED_UNSAFE,`
`279`	`285`	`UNUSED_MUT,`
`280`		`- COERCE_NEVER`
	`286`	`+ COERCE_NEVER,`
	`287`	`+ SINGLE_USE_LIFETIME`
`281`	`288`	`)`
`282`	`289`	`}`
`283`	`290`	`}`