Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint against single-use lifetime names #46441

Merged
merged 1 commit into from
Dec 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -277,7 +283,8 @@ impl LintPass for HardwiredLints {
DEPRECATED,
UNUSED_UNSAFE,
UNUSED_MUT,
COERCE_NEVER
COERCE_NEVER,
SINGLE_USE_LIFETIME
)
}
}
Expand Down
94 changes: 80 additions & 14 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(_)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
_ => {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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];

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
}
}
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-2.rs
Original file line number Diff line number Diff line change
@@ -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() { }
14 changes: 14 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-2.stderr
Original file line number Diff line number Diff line change
@@ -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

20 changes: 20 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-3.rs
Original file line number Diff line number Diff line change
@@ -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() { }
20 changes: 20 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-3.stderr
Original file line number Diff line number Diff line change
@@ -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

20 changes: 20 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-4.rs
Original file line number Diff line number Diff line change
@@ -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() { }
20 changes: 20 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-4.stderr
Original file line number Diff line number Diff line change
@@ -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

16 changes: 16 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-5.rs
Original file line number Diff line number Diff line change
@@ -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() { }
14 changes: 14 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes-5.stderr
Original file line number Diff line number Diff line change
@@ -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

16 changes: 16 additions & 0 deletions src/test/ui/in-band-lifetimes/single_use_lifetimes.rs
Original file line number Diff line number Diff line change
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test

fn deref<'x>() -> &'x u32 {
    22
}

Ideally, this would not warn, but I suspect your code will. I think that's ok, we can just add a comment like:

// FIXME(#44752) -- this scenario should not be warned

Here are some other scenarios. I will put down the behavior I think we eventually want, but it may not be the behavior that we currently have.

struct Foo<'x> {
    x: &'x u32 // no warning!
}

// Once #44524 is fixed, this should issue a warning. Hence, it's ok if it does so now, though perhaps a bit premature. 
impl<'y> Foo<'y> {
    fn method() { }
}
// Neither should issue a warning, as explicit lifetimes are mandatory in these cases:
struct Foo<'x> {
    x: &'x u32
}

enum Bar<'x> {
    Variant(&'x u32)
}
// Should not issue a warning, as explicit lifetimes are mandatory in these cases:
trait Foo<'x> {
    fn foo(&self, arg: &'x u32);
}

*v
}

fn main() {}
Loading