mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Emit warning when lifetime names are shadowed.

Browse Source 
This is not technically a [breaking-change], but it will be soon, so
you should update your code. Typically, shadowing is accidental, and
the shadowing lifetime can simply be removed. This frequently occurs
in constructor patterns:

```rust
// Old:
impl<'a> SomeStruct<'a> { fn new<'a>(..) -> SomeStruct<'a> { ... } }

// Should be:
impl<'a> SomeStruct<'a> { fn new(..) -> SomeStruct<'a> { ... } }
```

Otherwise, you should rename the inner lifetime to something
else. Note though that lifetime elision frequently applies:

```rust
// Old
impl<'a> SomeStruct<'a> {
    fn get<'a>(x: &'a self) -> &'a T { &self.field }
}

// Should be:
impl<'a> SomeStruct<'a> {
    fn get(x: &self) -> &T { &self.field }
}
``
This commit is contained in:
Niko Matsakis 2014-12-12 11:09:24 -05:00
parent ef0bc464af
commit b60de4bfc2
2 changed files with 144 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

154
src/librustc/middle/resolve_lifetime.rs
View File

@ -90,38 +90,41 @@ pub fn krate(sess: &Session, krate: &ast::Crate, def_map: &DefMap) -> NamedRegio
impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
fn visit_item(&mut self, item: &ast::Item) {
match item.node {
ast::ItemFn(..) => {
// Fn lifetimes get added in visit_fn below:
self.with(RootScope, |this| visit::walk_item(this, item));
}
ast::ItemMod(..) |
ast::ItemMac(..) |
ast::ItemForeignMod(..) |
ast::ItemStatic(..) |
ast::ItemConst(..) => {
// These sorts of items have no lifetime parameters at all.
self.with(RootScope, |this| visit::walk_item(this, item));
}
ast::ItemTy(_, ref generics) |
ast::ItemEnum(_, ref generics) |
ast::ItemStruct(_, ref generics) |
ast::ItemTrait(_, ref generics, _, _, _) => {
// These kinds of items have only early bound lifetime parameters.
let lifetimes = &generics.lifetimes;
self.with(EarlyScope(subst::TypeSpace, lifetimes, &ROOT_SCOPE), |this| {
this.check_lifetime_defs(lifetimes);
// Items always introduce a new root scope
self.with(RootScope, |_, this| {
match item.node {
ast::ItemFn(..) => {
// Fn lifetimes get added in visit_fn below:
visit::walk_item(this, item);
});
}
ast::ItemImpl(_, ref generics, _, _, _) => {
// Impls have both early- and late-bound lifetimes.
self.visit_early_late(subst::TypeSpace, generics, |this| {
this.check_lifetime_defs(&generics.lifetimes);
}
ast::ItemMod(..) |
ast::ItemMac(..) |
ast::ItemForeignMod(..) |
ast::ItemStatic(..) |
ast::ItemConst(..) => {
// These sorts of items have no lifetime parameters at all.
visit::walk_item(this, item);
})
}
ast::ItemTy(_, ref generics) |
ast::ItemEnum(_, ref generics) |
ast::ItemStruct(_, ref generics) |
ast::ItemTrait(_, ref generics, _, _, _) => {
// These kinds of items have only early bound lifetime parameters.
let lifetimes = &generics.lifetimes;
let early_scope = EarlyScope(subst::TypeSpace, lifetimes, &ROOT_SCOPE);
this.with(early_scope, |old_scope, this| {
this.check_lifetime_defs(old_scope, lifetimes);
visit::walk_item(this, item);
});
}
ast::ItemImpl(_, ref generics, _, _, _) => {
// Impls have both early- and late-bound lifetimes.
this.visit_early_late(subst::TypeSpace, generics, |this| {
visit::walk_item(this, item);
})
}
}
}
});
}
fn visit_fn(&mut self, fk: visit::FnKind<'v>, fd: &'v ast::FnDecl,
@ -129,9 +132,9 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
match fk {
visit::FkItemFn(_, generics, _, _) |
visit::FkMethod(_, generics, _) => {
self.visit_early_late(
subst::FnSpace, generics,
|this| visit::walk_fn(this, fk, fd, b, s))
self.visit_early_late(subst::FnSpace, generics, |this| {
visit::walk_fn(this, fk, fd, b, s)
})
}
visit::FkFnBlock(..) => {
visit::walk_fn(self, fk, fd, b, s)
@ -145,8 +148,8 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
// Careful, the bounds on a closure/proc are *not* within its binder.
visit::walk_ty_param_bounds_helper(self, &c.bounds);
visit::walk_lifetime_decls_helper(self, &c.lifetimes);
self.with(LateScope(&c.lifetimes, self.scope), |this| {
this.check_lifetime_defs(&c.lifetimes);
self.with(LateScope(&c.lifetimes, self.scope), |old_scope, this| {
this.check_lifetime_defs(old_scope, &c.lifetimes);
for argument in c.decl.inputs.iter() {
this.visit_ty(&*argument.ty)
}
@ -155,10 +158,10 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
}
ast::TyBareFn(ref c) => {
visit::walk_lifetime_decls_helper(self, &c.lifetimes);
self.with(LateScope(&c.lifetimes, self.scope), |this| {
self.with(LateScope(&c.lifetimes, self.scope), |old_scope, this| {
// a bare fn has no bounds, so everything
// contained within is scoped within its binder.
this.check_lifetime_defs(&c.lifetimes);
this.check_lifetime_defs(old_scope, &c.lifetimes);
visit::walk_ty(this, ty);
});
}
@ -167,7 +170,7 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
// a trait ref, which introduces a binding scope.
match self.def_map.borrow().get(&id) {
Some(&def::DefTrait(..)) => {
self.with(LateScope(&Vec::new(), self.scope), |this| {
self.with(LateScope(&Vec::new(), self.scope), |_, this| {
this.visit_path(path, id);
});
}
@ -190,7 +193,7 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
fn visit_block(&mut self, b: &ast::Block) {
self.with(BlockScope(region::CodeExtent::from_node_id(b.id), self.scope),
|this| visit::walk_block(this, b));
|_, this| visit::walk_block(this, b));
}
fn visit_lifetime_ref(&mut self, lifetime_ref: &ast::Lifetime) {
@ -232,8 +235,8 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
fn visit_poly_trait_ref(&mut self, trait_ref: &ast::PolyTraitRef) {
debug!("visit_poly_trait_ref trait_ref={}", trait_ref);
self.with(LateScope(&trait_ref.bound_lifetimes, self.scope), |this| {
this.check_lifetime_defs(&trait_ref.bound_lifetimes);
self.with(LateScope(&trait_ref.bound_lifetimes, self.scope), |old_scope, this| {
this.check_lifetime_defs(old_scope, &trait_ref.bound_lifetimes);
for lifetime in trait_ref.bound_lifetimes.iter() {
this.visit_lifetime_def(lifetime);
}
@ -248,7 +251,7 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
impl<'a> LifetimeContext<'a> {
fn with<F>(&mut self, wrap_scope: ScopeChain, f: F) where
F: FnOnce(&mut LifetimeContext),
F: FnOnce(Scope, &mut LifetimeContext),
{
let LifetimeContext {sess, ref mut named_region_map, ..} = *self;
let mut this = LifetimeContext {
@ -258,7 +261,7 @@ impl<'a> LifetimeContext<'a> {
def_map: self.def_map,
};
debug!("entering scope {}", this.scope);
f(&mut this);
f(self.scope, &mut this);
debug!("exiting scope {}", this.scope);
}
@ -294,9 +297,9 @@ impl<'a> LifetimeContext<'a> {
let (early, late) = generics.lifetimes.clone().partition(
|l| referenced_idents.iter().any(|&i| i == l.lifetime.name));
self.with(EarlyScope(early_space, &early, self.scope), move |this| {
this.with(LateScope(&late, this.scope), move |this| {
this.check_lifetime_defs(&generics.lifetimes);
self.with(EarlyScope(early_space, &early, self.scope), move |old_scope, this| {
this.with(LateScope(&late, this.scope), move |_, this| {
this.check_lifetime_defs(old_scope, &generics.lifetimes);
walk(this);
});
});
@ -323,7 +326,8 @@ impl<'a> LifetimeContext<'a> {
EarlyScope(space, lifetimes, s) => {
match search_lifetimes(lifetimes, lifetime_ref) {
Some((index, decl_id)) => {
Some((index, lifetime_def)) => {
let decl_id = lifetime_def.id;
let def = DefEarlyBoundRegion(space, index, decl_id);
self.insert_lifetime(lifetime_ref, def);
return;
@ -336,7 +340,8 @@ impl<'a> LifetimeContext<'a> {
LateScope(lifetimes, s) => {
match search_lifetimes(lifetimes, lifetime_ref) {
Some((_index, decl_id)) => {
Some((_index, lifetime_def)) => {
let decl_id = lifetime_def.id;
let debruijn = ty::DebruijnIndex::new(late_depth + 1);
let def = DefLateBoundRegion(debruijn, decl_id);
self.insert_lifetime(lifetime_ref, def);
@ -388,8 +393,8 @@ impl<'a> LifetimeContext<'a> {
}
match search_result {
Some((_depth, decl_id)) => {
let def = DefFreeRegion(scope_data, decl_id);
Some((_depth, lifetime)) => {
let def = DefFreeRegion(scope_data, lifetime.id);
self.insert_lifetime(lifetime_ref, def);
}
@ -407,7 +412,7 @@ impl<'a> LifetimeContext<'a> {
token::get_name(lifetime_ref.name)).as_slice());
}
fn check_lifetime_defs(&mut self, lifetimes: &Vec<ast::LifetimeDef>) {
fn check_lifetime_defs(&mut self, old_scope: Scope, lifetimes: &Vec<ast::LifetimeDef>) {
for i in range(0, lifetimes.len()) {
let lifetime_i = &lifetimes[i];
@ -422,6 +427,7 @@ impl<'a> LifetimeContext<'a> {
}
}
// It is a hard error to shadow a lifetime within the same scope.
for j in range(i + 1, lifetimes.len()) {
let lifetime_j = &lifetimes[j];
@ -435,12 +441,54 @@ impl<'a> LifetimeContext<'a> {
}
}
// It is a soft error to shadow a lifetime within a parent scope.
self.check_lifetime_def_for_shadowing(old_scope, &lifetime_i.lifetime);
for bound in lifetime_i.bounds.iter() {
self.resolve_lifetime_ref(bound);
}
}
}
fn check_lifetime_def_for_shadowing(&self,
mut old_scope: Scope,
lifetime: &ast::Lifetime)
{
loop {
match *old_scope {
BlockScope(_, s) => {
old_scope = s;
}
RootScope => {
return;
}
EarlyScope(_, lifetimes, s) |
LateScope(lifetimes, s) => {
if let Some((_, lifetime_def)) = search_lifetimes(lifetimes, lifetime) {
self.sess.span_warn(
lifetime.span,
format!("lifetime name `{}` shadows another \
lifetime name that is already in scope",
token::get_name(lifetime.name)).as_slice());
self.sess.span_help(
lifetime_def.span,
format!("shadowed lifetime `{}` declared here",
token::get_name(lifetime.name)).as_slice());
self.sess.span_help(
lifetime.span,
"shadowed lifetimes are deprecated \
and will become a hard error before 1.0");
return;
}
old_scope = s;
}
}
}
}
fn insert_lifetime(&mut self,
lifetime_ref: &ast::Lifetime,
def: DefRegion) {
@ -458,12 +506,12 @@ impl<'a> LifetimeContext<'a> {
}
}
fn search_lifetimes(lifetimes: &Vec<ast::LifetimeDef>,
fn search_lifetimes<'a>(lifetimes: &'a Vec<ast::LifetimeDef>,
lifetime_ref: &ast::Lifetime)
-> Option<(uint, ast::NodeId)> {
-> Option<(uint, &'a ast::Lifetime)> {
for (i, lifetime_decl) in lifetimes.iter().enumerate() {
if lifetime_decl.lifetime.name == lifetime_ref.name {
return Some((i, lifetime_decl.lifetime.id));
return Some((i, &lifetime_decl.lifetime));
}
}
return None;

43
src/test/compile-fail/shadowed-lifetime.rs Normal file
View File

@ -0,0 +1,43 @@
// Copyright 2013 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.
// Test that shadowed lifetimes generate an error.
struct Foo<'a>(&'a int);
impl<'a> Foo<'a> {
//~^ HELP shadowed lifetime `'a` declared here
fn shadow_in_method<'a>(&'a self) -> &'a int {
//~^ WARNING lifetime name `'a` shadows another lifetime name that is already in scope
//~| HELP deprecated
self.0
}
fn shadow_in_type<'b>(&'b self) -> &'b int {
//~^ HELP shadowed lifetime `'b` declared here
let x: for<'b> fn(&'b int) = panic!();
//~^ WARNING lifetime name `'b` shadows another lifetime name that is already in scope
//~| HELP deprecated
self.0
}
fn not_shadow_in_item<'b>(&'b self) {
struct Bar<'a, 'b>(&'a int, &'b int); // not a shadow, separate item
fn foo<'a, 'b>(x: &'a int, y: &'b int) { } // same
}
}
fn main() {
// intentional error that occurs after `resolve_lifetime` runs,
// just to ensure that this test fails to compile; when shadowed
// lifetimes become either an error or a proper lint, this will
// not be needed.
let x: int = 3u; //~ ERROR mismatched types
}