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

Add lint checks for unused loop labels #50763

Merged
merged 15 commits into from
May 19, 2018
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
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,12 @@ declare_lint! {
"detects name collision with an existing but unstable method"
}

declare_lint! {
pub UNUSED_LABELS,
Allow,
"detects labels that are never used"
}

/// 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 @@ -325,6 +331,7 @@ impl LintPass for HardwiredLints {
UNUSED_MUT,
SINGLE_USE_LIFETIME,
UNUSED_LIFETIME,
UNUSED_LABELS,
TYVAR_BEHIND_RAW_POINTER,
ELIDED_LIFETIME_IN_PATH,
BARE_TRAIT_OBJECT,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
UNUSED_DOC_COMMENT,
UNUSED_EXTERN_CRATES,
UNUSED_FEATURES,
UNUSED_LABELS,
UNUSED_PARENS);

add_lint_group!(sess,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_resolve/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
}
}

for (id, span) in resolver.unused_labels.iter() {
resolver.session.buffer_lint(lint::builtin::UNUSED_LABELS, *id, *span, "unused label");
}

let mut visitor = UnusedImportCheckVisitor {
resolver,
unused_imports: NodeMap(),
Expand Down
12 changes: 10 additions & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,10 @@ pub struct Resolver<'a> {
pub maybe_unused_trait_imports: NodeSet,
pub maybe_unused_extern_crates: Vec<(NodeId, Span)>,

/// A list of labels as of yet unused. Labels will be removed from this map when
/// they are used (in a `break` or `continue` statement)
pub unused_labels: FxHashMap<NodeId, Span>,

/// privacy errors are delayed until the end in order to deduplicate them
privacy_errors: Vec<PrivacyError<'a>>,
/// ambiguity errors are delayed for deduplication
Expand Down Expand Up @@ -1752,6 +1756,8 @@ impl<'a> Resolver<'a> {
maybe_unused_trait_imports: NodeSet(),
maybe_unused_extern_crates: Vec::new(),

unused_labels: FxHashMap(),

privacy_errors: Vec::new(),
ambiguity_errors: Vec::new(),
use_injections: Vec::new(),
Expand Down Expand Up @@ -3694,6 +3700,7 @@ impl<'a> Resolver<'a> {
where F: FnOnce(&mut Resolver)
{
if let Some(label) = label {
self.unused_labels.insert(id, label.ident.span);
let def = Def::Label(id);
self.with_label_rib(|this| {
this.label_ribs.last_mut().unwrap().bindings.insert(label.ident, def);
Expand Down Expand Up @@ -3742,9 +3749,10 @@ impl<'a> Resolver<'a> {
ResolutionError::UndeclaredLabel(&label.ident.name.as_str(),
close_match));
}
Some(def @ Def::Label(_)) => {
Some(Def::Label(id)) => {
// Since this def is a label, it is never read.
self.record_def(expr.id, PathResolution::new(def));
self.record_def(expr.id, PathResolution::new(Def::Label(id)));
self.unused_labels.remove(&id);
}
Some(_) => {
span_bug!(expr.span, "label wasn't mapped to a label def!");
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/label_break_value_continue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

#![feature(label_break_value)]
#![allow(unused_labels)]

// Simple continue pointing to an unlabeled break should yield in an error
fn continue_simple() {
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/label_break_value_continue.stderr
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
error[E0695]: unlabeled `continue` inside of a labeled block
--> $DIR/label_break_value_continue.rs:16:9
--> $DIR/label_break_value_continue.rs:17:9
|
LL | continue; //~ ERROR unlabeled `continue` inside of a labeled block
| ^^^^^^^^ `continue` statements that would diverge to or through a labeled block need to bear a label

error[E0696]: `continue` pointing to a labeled block
--> $DIR/label_break_value_continue.rs:23:9
--> $DIR/label_break_value_continue.rs:24:9
|
LL | continue 'b; //~ ERROR `continue` pointing to a labeled block
| ^^^^^^^^^^^ labeled blocks cannot be `continue`'d
|
note: labeled block the continue points to
--> $DIR/label_break_value_continue.rs:22:5
--> $DIR/label_break_value_continue.rs:23:5
|
LL | / 'b: {
LL | | continue 'b; //~ ERROR `continue` pointing to a labeled block
LL | | }
| |_____^

error[E0695]: unlabeled `continue` inside of a labeled block
--> $DIR/label_break_value_continue.rs:31:13
--> $DIR/label_break_value_continue.rs:32:13
|
LL | continue; //~ ERROR unlabeled `continue` inside of a labeled block
| ^^^^^^^^ `continue` statements that would diverge to or through a labeled block need to bear a label
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/label_break_value_unlabeled_break.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

#![feature(label_break_value)]
#![allow(unused_labels)]

// Simple unlabeled break should yield in an error
fn unlabeled_break_simple() {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/label_break_value_unlabeled_break.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error[E0695]: unlabeled `break` inside of a labeled block
--> $DIR/label_break_value_unlabeled_break.rs:16:9
--> $DIR/label_break_value_unlabeled_break.rs:17:9
|
LL | break; //~ ERROR unlabeled `break` inside of a labeled block
| ^^^^^ `break` statements that would diverge to or through a labeled block need to bear a label

error[E0695]: unlabeled `break` inside of a labeled block
--> $DIR/label_break_value_unlabeled_break.rs:24:13
--> $DIR/label_break_value_unlabeled_break.rs:25:13
|
LL | break; //~ ERROR unlabeled `break` inside of a labeled block
| ^^^^^ `break` statements that would diverge to or through a labeled block need to bear a label
Expand Down
96 changes: 96 additions & 0 deletions src/test/ui/lint/unused_labels.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// 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.

// The output should warn when a loop label is not used. However, it
// should also deal with the edge cases where a label is shadowed,
// within nested loops

// compile-pass

#![feature(label_break_value)]
#![warn(unused_labels)]

fn main() {
'unused_while_label: while 0 == 0 {
//~^ WARN unused label
}

let opt = Some(0);
'unused_while_let_label: while let Some(_) = opt {
//~^ WARN unused label
}

'unused_for_label: for _ in 0..10 {
//~^ WARN unused label
}

'used_loop_label: loop {
break 'used_loop_label;
}

'used_loop_label_outer_1: for _ in 0..10 {
'used_loop_label_inner_1: for _ in 0..10 {
break 'used_loop_label_inner_1;
}
break 'used_loop_label_outer_1;
}

'used_loop_label_outer_2: for _ in 0..10 {
'unused_loop_label_inner_2: for _ in 0..10 {
//~^ WARN unused label
break 'used_loop_label_outer_2;
}
}

'unused_loop_label_outer_3: for _ in 0..10 {
//~^ WARN unused label
'used_loop_label_inner_3: for _ in 0..10 {
break 'used_loop_label_inner_3;
}
}

// You should be able to break the same label many times
'many_used: loop {
if true {
break 'many_used;
} else {
break 'many_used;
}
}

// Test breaking many times with the same inner label doesn't break the
// warning on the outer label
'many_used_shadowed: for _ in 0..10 {
//~^ WARN unused label
'many_used_shadowed: for _ in 0..10 {
//~^ WARN label name `'many_used_shadowed` shadows a label name that is already in scope
if 1 % 2 == 0 {
break 'many_used_shadowed;
} else {
break 'many_used_shadowed;
}
}
}

'unused_loop_label: loop {
//~^ WARN unused label
break;
}

// Make sure unused block labels give warnings...
'unused_block_label: {
//~^ WARN unused label
}

// ...and that used ones don't:
'used_block_label: {
break 'used_block_label;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Before merging, can you also add a test case for the following, which should be allowed?

fn foo() {
    'used_many: loop {
        if true {
            break 'used_many;
        } else {
            break 'used_many;
        }
    }
}

}
63 changes: 63 additions & 0 deletions src/test/ui/lint/unused_labels.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
warning: unused label
--> $DIR/unused_labels.rs:21:5
|
LL | 'unused_while_label: while 0 == 0 {
| ^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/unused_labels.rs:18:9
|
LL | #![warn(unused_labels)]
| ^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:26:5
|
LL | 'unused_while_let_label: while let Some(_) = opt {
| ^^^^^^^^^^^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:30:5
|
LL | 'unused_for_label: for _ in 0..10 {
| ^^^^^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:46:9
|
LL | 'unused_loop_label_inner_2: for _ in 0..10 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:52:5
|
LL | 'unused_loop_label_outer_3: for _ in 0..10 {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:70:5
|
LL | 'many_used_shadowed: for _ in 0..10 {
| ^^^^^^^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:82:5
|
LL | 'unused_loop_label: loop {
| ^^^^^^^^^^^^^^^^^^

warning: unused label
--> $DIR/unused_labels.rs:88:5
|
LL | 'unused_block_label: {
| ^^^^^^^^^^^^^^^^^^^

warning: label name `'many_used_shadowed` shadows a label name that is already in scope
--> $DIR/unused_labels.rs:72:9
|
LL | 'many_used_shadowed: for _ in 0..10 {
| ------------------- first declared here
LL | //~^ WARN unused label
LL | 'many_used_shadowed: for _ in 0..10 {
| ^^^^^^^^^^^^^^^^^^^ lifetime 'many_used_shadowed already in scope

1 change: 1 addition & 0 deletions src/test/ui/loops-reject-duplicate-labels-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
// discussed here:
// https://internals.rust-lang.org/t/psa-rejecting-duplicate-loop-labels/1833

#[allow(unused_labels)]
pub fn foo() {
{ 'fl: for _ in 0..10 { break; } }
{ 'fl: loop { break; } } //~ WARN label name `'fl` shadows a label name that is already in scope
Expand Down
18 changes: 9 additions & 9 deletions src/test/ui/loops-reject-duplicate-labels-2.stderr
Original file line number Diff line number Diff line change
@@ -1,69 +1,69 @@
warning: label name `'fl` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:23:7
--> $DIR/loops-reject-duplicate-labels-2.rs:24:7
|
LL | { 'fl: for _ in 0..10 { break; } }
| --- first declared here
LL | { 'fl: loop { break; } } //~ WARN label name `'fl` shadows a label name that is already in scope
| ^^^ lifetime 'fl already in scope

warning: label name `'lf` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:25:7
--> $DIR/loops-reject-duplicate-labels-2.rs:26:7
|
LL | { 'lf: loop { break; } }
| --- first declared here
LL | { 'lf: for _ in 0..10 { break; } } //~ WARN label name `'lf` shadows a label name that is already in scope
| ^^^ lifetime 'lf already in scope

warning: label name `'wl` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:27:7
--> $DIR/loops-reject-duplicate-labels-2.rs:28:7
|
LL | { 'wl: while 2 > 1 { break; } }
| --- first declared here
LL | { 'wl: loop { break; } } //~ WARN label name `'wl` shadows a label name that is already in scope
| ^^^ lifetime 'wl already in scope

warning: label name `'lw` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:29:7
--> $DIR/loops-reject-duplicate-labels-2.rs:30:7
|
LL | { 'lw: loop { break; } }
| --- first declared here
LL | { 'lw: while 2 > 1 { break; } } //~ WARN label name `'lw` shadows a label name that is already in scope
| ^^^ lifetime 'lw already in scope

warning: label name `'fw` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:31:7
--> $DIR/loops-reject-duplicate-labels-2.rs:32:7
|
LL | { 'fw: for _ in 0..10 { break; } }
| --- first declared here
LL | { 'fw: while 2 > 1 { break; } } //~ WARN label name `'fw` shadows a label name that is already in scope
| ^^^ lifetime 'fw already in scope

warning: label name `'wf` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:33:7
--> $DIR/loops-reject-duplicate-labels-2.rs:34:7
|
LL | { 'wf: while 2 > 1 { break; } }
| --- first declared here
LL | { 'wf: for _ in 0..10 { break; } } //~ WARN label name `'wf` shadows a label name that is already in scope
| ^^^ lifetime 'wf already in scope

warning: label name `'tl` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:35:7
--> $DIR/loops-reject-duplicate-labels-2.rs:36:7
|
LL | { 'tl: while let Some(_) = None::<i32> { break; } }
| --- first declared here
LL | { 'tl: loop { break; } } //~ WARN label name `'tl` shadows a label name that is already in scope
| ^^^ lifetime 'tl already in scope

warning: label name `'lt` shadows a label name that is already in scope
--> $DIR/loops-reject-duplicate-labels-2.rs:37:7
--> $DIR/loops-reject-duplicate-labels-2.rs:38:7
|
LL | { 'lt: loop { break; } }
| --- first declared here
LL | { 'lt: while let Some(_) = None::<i32> { break; } }
| ^^^ lifetime 'lt already in scope

error: compilation successful
--> $DIR/loops-reject-duplicate-labels-2.rs:42:1
--> $DIR/loops-reject-duplicate-labels-2.rs:43:1
|
LL | / pub fn main() { //~ ERROR compilation successful
LL | | foo();
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/loops-reject-duplicate-labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// Issue #21633: reject duplicate loop labels in function bodies.
// This is testing the exact cases that are in the issue description.

#[allow(unused_labels)]
fn foo() {
'fl: for _ in 0..10 { break; }
'fl: loop { break; } //~ WARN label name `'fl` shadows a label name that is already in scope
Expand Down
Loading