Skip to content

Commit

Permalink
Auto merge of #50763 - KyleStach1678:unused-loop-label, r=petrochenkov
Browse files Browse the repository at this point in the history
Add lint checks for unused loop labels

Previously no warning was generated when a loop label was written out but never used (i.e. in a `break` or `continue` statement):
```rust
fn main() {
    'unused_label: loop {}
}
```
would compile without complaint.

This fix introduces `-W unused_loop_label`, which generates the following warning message for the above snippet:
```
warning: unused loop label
 --> main.rs:2:5
  |
2 |     'unused_label: loop {}
  |     ^^^^^^^^^^^^^
  |
  = note: #[warn(unused_loop_label)] on by default
```

Fixes: #50751.
  • Loading branch information
bors committed May 19, 2018
2 parents ef8ee64 + 6da64a7 commit 1b240da
Show file tree
Hide file tree
Showing 15 changed files with 210 additions and 27 deletions.
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,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 @@ -318,6 +324,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 @@ -1468,6 +1468,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 @@ -1747,6 +1751,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 @@ -3682,6 +3688,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 @@ -3730,9 +3737,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;
}
}
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

0 comments on commit 1b240da

Please sign in to comment.