Skip to content

Commit

Permalink
Rollup merge of rust-lang#24162 - pnkfelix:fsk-detect-duplicate-loop-…
Browse files Browse the repository at this point in the history
…labels, r=nikomatsakis

Check for duplicate loop labels in function bodies.

See also: http://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833

Fix rust-lang#21633
  • Loading branch information
steveklabnik committed Apr 10, 2015
2 parents a2761e7 + e6880ab commit b64b055
Show file tree
Hide file tree
Showing 5 changed files with 427 additions and 12 deletions.
202 changes: 190 additions & 12 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use middle::region;
use middle::subst;
use middle::ty;
use std::fmt;
use std::mem::replace;
use syntax::ast;
use syntax::codemap::Span;
use syntax::parse::token::special_idents;
Expand Down Expand Up @@ -70,6 +71,9 @@ struct LifetimeContext<'a> {

// I'm sorry.
trait_ref_hack: bool,

// List of labels in the function/method currently under analysis.
labels_in_fn: Vec<(ast::Ident, Span)>,
}

enum ScopeChain<'a> {
Expand Down Expand Up @@ -97,13 +101,18 @@ pub fn krate(sess: &Session, krate: &ast::Crate, def_map: &DefMap) -> NamedRegio
scope: &ROOT_SCOPE,
def_map: def_map,
trait_ref_hack: false,
labels_in_fn: vec![],
}, krate);
sess.abort_if_errors();
named_region_map
}

impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
fn visit_item(&mut self, item: &ast::Item) {
// Items save/restore the set of labels. This way innner items
// can freely reuse names, be they loop labels or lifetimes.
let saved = replace(&mut self.labels_in_fn, vec![]);

// Items always introduce a new root scope
self.with(RootScope, |_, this| {
match item.node {
Expand Down Expand Up @@ -137,23 +146,26 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
}
}
});

// Done traversing the item; restore saved set of labels.
replace(&mut self.labels_in_fn, saved);
}

fn visit_fn(&mut self, fk: visit::FnKind<'v>, fd: &'v ast::FnDecl,
b: &'v ast::Block, s: Span, _: ast::NodeId) {
match fk {
visit::FkItemFn(_, generics, _, _) => {
self.visit_early_late(subst::FnSpace, generics, |this| {
visit::walk_fn(this, fk, fd, b, s)
this.walk_fn(fk, fd, b, s)
})
}
visit::FkMethod(_, sig) => {
self.visit_early_late(subst::FnSpace, &sig.generics, |this| {
visit::walk_fn(this, fk, fd, b, s)
this.walk_fn(fk, fd, b, s)
})
}
visit::FkFnBlock(..) => {
visit::walk_fn(self, fk, fd, b, s)
self.walk_fn(fk, fd, b, s)
}
}
}
Expand Down Expand Up @@ -190,13 +202,19 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
}

fn visit_trait_item(&mut self, trait_item: &ast::TraitItem) {
// We reset the labels on every trait item, so that different
// methods in an impl can reuse label names.
let saved = replace(&mut self.labels_in_fn, vec![]);

if let ast::MethodTraitItem(ref sig, None) = trait_item.node {
self.visit_early_late(
subst::FnSpace, &sig.generics,
|this| visit::walk_trait_item(this, trait_item))
} else {
visit::walk_trait_item(self, trait_item);
}

replace(&mut self.labels_in_fn, saved);
}

fn visit_block(&mut self, b: &ast::Block) {
Expand Down Expand Up @@ -286,7 +304,159 @@ impl<'a, 'v> Visitor<'v> for LifetimeContext<'a> {
}
}

enum ShadowKind { Label, Lifetime }
struct Original { kind: ShadowKind, span: Span }
struct Shadower { kind: ShadowKind, span: Span }

fn original_label(span: Span) -> Original {
Original { kind: ShadowKind::Label, span: span }
}
fn shadower_label(span: Span) -> Shadower {
Shadower { kind: ShadowKind::Label, span: span }
}
fn original_lifetime(l: &ast::Lifetime) -> Original {
Original { kind: ShadowKind::Lifetime, span: l.span }
}
fn shadower_lifetime(l: &ast::Lifetime) -> Shadower {
Shadower { kind: ShadowKind::Lifetime, span: l.span }
}

impl ShadowKind {
fn desc(&self) -> &'static str {
match *self {
ShadowKind::Label => "label",
ShadowKind::Lifetime => "lifetime",
}
}
}

fn signal_shadowing_error(
sess: &Session, name: ast::Name, orig: Original, shadower: Shadower) {
sess.span_err(shadower.span,
&format!("{} name `{}` shadows a \
{} name that is already in scope",
shadower.kind.desc(), name, orig.kind.desc()));
sess.span_note(orig.span,
&format!("shadowed {} `{}` declared here",
orig.kind.desc(), name));
}

// Adds all labels in `b` to `ctxt.labels_in_fn`, signalling an error
// if one of the label shadows a lifetime or another label.
fn extract_labels<'v, 'a>(ctxt: &mut LifetimeContext<'a>, b: &'v ast::Block) {

struct GatherLabels<'a> {
sess: &'a Session,
scope: Scope<'a>,
labels_in_fn: &'a mut Vec<(ast::Ident, Span)>,
}

let mut gather = GatherLabels {
sess: ctxt.sess,
scope: ctxt.scope,
labels_in_fn: &mut ctxt.labels_in_fn,
};
gather.visit_block(b);
return;

impl<'v, 'a> Visitor<'v> for GatherLabels<'a> {
fn visit_expr(&mut self, ex: &'v ast::Expr) {
if let Some(label) = expression_label(ex) {
for &(prior, prior_span) in &self.labels_in_fn[..] {
// FIXME (#24278): non-hygienic comparision
if label.name == prior.name {
signal_shadowing_error(self.sess,
label.name,
original_label(prior_span),
shadower_label(ex.span));
}
}

check_if_label_shadows_lifetime(self.sess,
self.scope,
label,
ex.span);

self.labels_in_fn.push((label, ex.span));
}
visit::walk_expr(self, ex)
}

fn visit_item(&mut self, _: &ast::Item) {
// do not recurse into items defined in the block
}
}

fn expression_label(ex: &ast::Expr) -> Option<ast::Ident> {
match ex.node {
ast::ExprWhile(_, _, Some(label)) |
ast::ExprWhileLet(_, _, _, Some(label)) |
ast::ExprForLoop(_, _, _, Some(label)) |
ast::ExprLoop(_, Some(label)) => Some(label),
_ => None,
}
}

fn check_if_label_shadows_lifetime<'a>(sess: &'a Session,
mut scope: Scope<'a>,
label: ast::Ident,
label_span: Span) {
loop {
match *scope {
BlockScope(_, s) => { scope = s; }
RootScope => { return; }

EarlyScope(_, lifetimes, s) |
LateScope(lifetimes, s) => {
for lifetime_def in lifetimes {
// FIXME (#24278): non-hygienic comparision
if label.name == lifetime_def.lifetime.name {
signal_shadowing_error(
sess,
label.name,
original_lifetime(&lifetime_def.lifetime),
shadower_label(label_span));
return;
}
}
scope = s;
}
}
}
}
}

impl<'a> LifetimeContext<'a> {
// This is just like visit::walk_fn, except that it extracts the
// labels of the function body and swaps them in before visiting
// the function body itself.
fn walk_fn<'b>(&mut self,
fk: visit::FnKind,
fd: &ast::FnDecl,
fb: &'b ast::Block,
_span: Span) {
match fk {
visit::FkItemFn(_, generics, _, _) => {
visit::walk_fn_decl(self, fd);
self.visit_generics(generics);
}
visit::FkMethod(_, sig) => {
visit::walk_fn_decl(self, fd);
self.visit_generics(&sig.generics);
self.visit_explicit_self(&sig.explicit_self);
}
visit::FkFnBlock(..) => {
visit::walk_fn_decl(self, fd);
}
}

// After inpsecting the decl, add all labels from the body to
// `self.labels_in_fn`.
extract_labels(self, fb);

self.visit_block(fb);
}

fn with<F>(&mut self, wrap_scope: ScopeChain, f: F) where
F: FnOnce(Scope, &mut LifetimeContext),
{
Expand All @@ -297,6 +467,7 @@ impl<'a> LifetimeContext<'a> {
scope: &wrap_scope,
def_map: self.def_map,
trait_ref_hack: self.trait_ref_hack,
labels_in_fn: self.labels_in_fn.clone(),
};
debug!("entering scope {:?}", this.scope);
f(self.scope, &mut this);
Expand Down Expand Up @@ -494,6 +665,17 @@ impl<'a> LifetimeContext<'a> {
mut old_scope: Scope,
lifetime: &ast::Lifetime)
{
for &(label, label_span) in &self.labels_in_fn {
// FIXME (#24278): non-hygienic comparision
if lifetime.name == label.name {
signal_shadowing_error(self.sess,
lifetime.name,
original_label(label_span),
shadower_lifetime(&lifetime));
return;
}
}

loop {
match *old_scope {
BlockScope(_, s) => {
Expand All @@ -507,15 +689,11 @@ impl<'a> LifetimeContext<'a> {
EarlyScope(_, lifetimes, s) |
LateScope(lifetimes, s) => {
if let Some((_, lifetime_def)) = search_lifetimes(lifetimes, lifetime) {
self.sess.span_err(
lifetime.span,
&format!("lifetime name `{}` shadows another \
lifetime name that is already in scope",
token::get_name(lifetime.name)));
self.sess.span_note(
lifetime_def.span,
&format!("shadowed lifetime `{}` declared here",
token::get_name(lifetime.name)));
signal_shadowing_error(
self.sess,
lifetime.name,
original_lifetime(&lifetime_def),
shadower_lifetime(&lifetime));
return;
}

Expand Down
44 changes: 44 additions & 0 deletions src/test/compile-fail/loops-reject-duplicate-labels-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright 2015 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.

// ignore-tidy-linelength

// Issue #21633: reject duplicate loop labels in function bodies.
//
// This is testing the generalization (to the whole function body)
// discussed here:
// http://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833

fn main() {
{ 'fl: for _ in 0..10 { break; } } //~ NOTE shadowed label `'fl` declared here
{ 'fl: loop { break; } } //~ ERROR label name `'fl` shadows a label name that is already in scope

{ 'lf: loop { break; } } //~ NOTE shadowed label `'lf` declared here
{ 'lf: for _ in 0..10 { break; } } //~ ERROR label name `'lf` shadows a label name that is already in scope

{ 'wl: while 2 > 1 { break; } } //~ NOTE shadowed label `'wl` declared here
{ 'wl: loop { break; } } //~ ERROR label name `'wl` shadows a label name that is already in scope

{ 'lw: loop { break; } } //~ NOTE shadowed label `'lw` declared here
{ 'lw: while 2 > 1 { break; } } //~ ERROR label name `'lw` shadows a label name that is already in scope

{ 'fw: for _ in 0..10 { break; } } //~ NOTE shadowed label `'fw` declared here
{ 'fw: while 2 > 1 { break; } } //~ ERROR label name `'fw` shadows a label name that is already in scope

{ 'wf: while 2 > 1 { break; } } //~ NOTE shadowed label `'wf` declared here
{ 'wf: for _ in 0..10 { break; } } //~ ERROR label name `'wf` shadows a label name that is already in scope

{ 'tl: while let Some(_) = None::<i32> { break; } } //~ NOTE shadowed label `'tl` declared here
{ 'tl: loop { break; } } //~ ERROR label name `'tl` shadows a label name that is already in scope

{ 'lt: loop { break; } } //~ NOTE shadowed label `'lt` declared here
{ 'lt: while let Some(_) = None::<i32> { break; } }
//~^ ERROR label name `'lt` shadows a label name that is already in scope
}
50 changes: 50 additions & 0 deletions src/test/compile-fail/loops-reject-duplicate-labels.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright 2015 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.

// ignore-tidy-linelength

// Issue #21633: reject duplicate loop labels in function bodies.
// This is testing the exact cases that are in the issue description.

fn main() {
'fl: for _ in 0..10 { break; } //~ NOTE shadowed label `'fl` declared here
'fl: loop { break; } //~ ERROR label name `'fl` shadows a label name that is already in scope

'lf: loop { break; } //~ NOTE shadowed label `'lf` declared here
'lf: for _ in 0..10 { break; } //~ ERROR label name `'lf` shadows a label name that is already in scope

'wl: while 2 > 1 { break; } //~ NOTE shadowed label `'wl` declared here
'wl: loop { break; } //~ ERROR label name `'wl` shadows a label name that is already in scope

'lw: loop { break; } //~ NOTE shadowed label `'lw` declared here
'lw: while 2 > 1 { break; } //~ ERROR label name `'lw` shadows a label name that is already in scope

'fw: for _ in 0..10 { break; } //~ NOTE shadowed label `'fw` declared here
'fw: while 2 > 1 { break; } //~ ERROR label name `'fw` shadows a label name that is already in scope

'wf: while 2 > 1 { break; } //~ NOTE shadowed label `'wf` declared here
'wf: for _ in 0..10 { break; } //~ ERROR label name `'wf` shadows a label name that is already in scope

'tl: while let Some(_) = None::<i32> { break; } //~ NOTE shadowed label `'tl` declared here
'tl: loop { break; } //~ ERROR label name `'tl` shadows a label name that is already in scope

'lt: loop { break; } //~ NOTE shadowed label `'lt` declared here
'lt: while let Some(_) = None::<i32> { break; }
//~^ ERROR label name `'lt` shadows a label name that is already in scope
}

// Note however that it is okay for the same label to be reuse in
// different methods of one impl, as illustrated here.

struct S;
impl S {
fn m1(&self) { 'okay: loop { break 'okay; } }
fn m2(&self) { 'okay: loop { break 'okay; } }
}
Loading

0 comments on commit b64b055

Please sign in to comment.