Skip to content

Commit

Permalink
Rollup merge of rust-lang#65830 - Quantumplation:master, r=davidtwco
Browse files Browse the repository at this point in the history
Use ident.span instead of def_span in dead-code pass

Hello! First time contributor! :)

This should fix rust-lang#58729.

According to @estebank in the duplicate rust-lang#63064, def_span scans forward on the line until it finds a {,
and if it can't find one, falls back to the span for the whole item. This
was apparently written before the identifier span was explicitly tracked on
each node.

This means that if an unused function signature spans multiple lines, the
entire function (potentially hundreds of lines) gets flagged as dead code.
This could, for example, cause IDEs to add error squiggly's to the whole
function.

By using the span from the ident instead, we narrow the scope of this in
most cases. In a wider sense, it's probably safe to use ident.span
instead of def_span in most locations throughout the whole code base,
but since this is my first contribution, I kept it small.

Some interesting points that came up while I was working on this:
- I reorganized the tests a bit to bring some of the dead code ones all
into the same location
- A few tests were for things unrelated to dead code (like the
path-lookahead for parens), so I added #![allow(dead_code)] and
cleaned up the stderr file to reduce noise in the future
- The same fix doesn't apply to const and static declarations. I tried
adding these cases to the match expression, but that created a much
wider change to tests and error messages, so I left it off until I
could get some code review to validate the approach.
  • Loading branch information
Centril authored Oct 26, 2019
2 parents 8a5f08f + 94890d2 commit 838ed89
Show file tree
Hide file tree
Showing 50 changed files with 128 additions and 112 deletions.
2 changes: 1 addition & 1 deletion src/librustc_passes/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ impl Visitor<'tcx> for DeadVisitor<'tcx> {
hir::ItemKind::Struct(..) |
hir::ItemKind::Union(..) |
hir::ItemKind::Trait(..) |
hir::ItemKind::Impl(..) => self.tcx.sess.source_map().def_span(item.span),
hir::ItemKind::Impl(..) => item.ident.span,
_ => item.span,
};
let participle = match item.kind {
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui-fulldeps/lint-plugin-cmdline-allow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ LL | #![plugin(lint_plugin_test)]
= note: `#[warn(deprecated)]` on by default

warning: function is never used: `lintme`
--> $DIR/lint-plugin-cmdline-allow.rs:10:1
--> $DIR/lint-plugin-cmdline-allow.rs:10:4
|
LL | fn lintme() { }
| ^^^^^^^^^^^
| ^^^^^^
|
note: lint level defined here
--> $DIR/lint-plugin-cmdline-allow.rs:7:9
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui-fulldeps/lint-tool-cmdline-allow.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ LL | fn lintme() {}
= note: `#[warn(clippy::test_lint)]` on by default

warning: function is never used: `lintme`
--> $DIR/lint-tool-cmdline-allow.rs:10:1
--> $DIR/lint-tool-cmdline-allow.rs:10:4
|
LL | fn lintme() {}
| ^^^^^^^^^^^
| ^^^^^^
|
note: lint level defined here
--> $DIR/lint-tool-cmdline-allow.rs:7:9
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: function is never used: `foo`
--> $DIR/fail-no-dead-code.rs:4:1
--> $DIR/basic.rs:4:4
|
LL | fn foo() {
| ^^^^^^^^
| ^^^
|
note: lint level defined here
--> $DIR/fail-no-dead-code.rs:1:9
--> $DIR/basic.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: enum is never used: `E`
--> $DIR/lint-dead-code-empty-unused-enum.rs:3:1
--> $DIR/empty-unused-enum.rs:3:6
|
LL | enum E {}
| ^^^^^^
| ^
|
note: lint level defined here
--> $DIR/lint-dead-code-empty-unused-enum.rs:1:9
--> $DIR/empty-unused-enum.rs:1:9
|
LL | #![deny(unused)]
| ^^^^^^
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: type alias is never used: `Unused`
--> $DIR/lint-dead-code-impl-trait.rs:12:1
--> $DIR/impl-trait.rs:12:1
|
LL | type Unused = ();
| ^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-impl-trait.rs:1:9
--> $DIR/impl-trait.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: struct is never constructed: `Bar`
--> $DIR/lint-dead-code-1.rs:12:5
--> $DIR/lint-dead-code-1.rs:12:16
|
LL | pub struct Bar;
| ^^^^^^^^^^^^^^^
| ^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-1.rs:5:9
Expand All @@ -23,16 +23,16 @@ LL | const priv_const: isize = 0;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: struct is never constructed: `PrivStruct`
--> $DIR/lint-dead-code-1.rs:35:1
--> $DIR/lint-dead-code-1.rs:35:8
|
LL | struct PrivStruct;
| ^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^

error: enum is never used: `priv_enum`
--> $DIR/lint-dead-code-1.rs:64:1
--> $DIR/lint-dead-code-1.rs:64:6
|
LL | enum priv_enum { foo2, bar2 }
| ^^^^^^^^^^^^^^
| ^^^^^^^^^

error: variant is never constructed: `bar3`
--> $DIR/lint-dead-code-1.rs:67:5
Expand All @@ -41,28 +41,28 @@ LL | bar3
| ^^^^

error: function is never used: `priv_fn`
--> $DIR/lint-dead-code-1.rs:88:1
--> $DIR/lint-dead-code-1.rs:88:4
|
LL | fn priv_fn() {
| ^^^^^^^^^^^^
| ^^^^^^^

error: function is never used: `foo`
--> $DIR/lint-dead-code-1.rs:93:1
--> $DIR/lint-dead-code-1.rs:93:4
|
LL | fn foo() {
| ^^^^^^^^
| ^^^

error: function is never used: `bar`
--> $DIR/lint-dead-code-1.rs:98:1
--> $DIR/lint-dead-code-1.rs:98:4
|
LL | fn bar() {
| ^^^^^^^^
| ^^^

error: function is never used: `baz`
--> $DIR/lint-dead-code-1.rs:102:1
--> $DIR/lint-dead-code-1.rs:102:4
|
LL | fn baz() -> impl Copy {
| ^^^^^^^^^^^^^^^^^^^^^
| ^^^

error: aborting due to 10 previous errors

File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: function is never used: `dead_fn`
--> $DIR/lint-dead-code-2.rs:22:1
--> $DIR/lint-dead-code-2.rs:22:4
|
LL | fn dead_fn() {}
| ^^^^^^^^^^^^
| ^^^^^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-2.rs:2:9
Expand All @@ -11,16 +11,16 @@ LL | #![deny(dead_code)]
| ^^^^^^^^^

error: function is never used: `dead_fn2`
--> $DIR/lint-dead-code-2.rs:25:1
--> $DIR/lint-dead-code-2.rs:25:4
|
LL | fn dead_fn2() {}
| ^^^^^^^^^^^^^
| ^^^^^^^^

error: function is never used: `main`
--> $DIR/lint-dead-code-2.rs:38:1
--> $DIR/lint-dead-code-2.rs:38:4
|
LL | fn main() {
| ^^^^^^^^^
| ^^^^

error: aborting due to 3 previous errors

File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: struct is never constructed: `Foo`
--> $DIR/lint-dead-code-3.rs:13:1
--> $DIR/lint-dead-code-3.rs:13:8
|
LL | struct Foo;
| ^^^^^^^^^^^
| ^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-3.rs:3:9
Expand All @@ -17,16 +17,16 @@ LL | fn foo(&self) {
| ^^^^^^^^^^^^^

error: function is never used: `bar`
--> $DIR/lint-dead-code-3.rs:20:1
--> $DIR/lint-dead-code-3.rs:20:4
|
LL | fn bar() {
| ^^^^^^^^
| ^^^

error: enum is never used: `c_void`
--> $DIR/lint-dead-code-3.rs:59:1
--> $DIR/lint-dead-code-3.rs:59:6
|
LL | enum c_void {}
| ^^^^^^^^^^^
| ^^^^^^

error: foreign function is never used: `free`
--> $DIR/lint-dead-code-3.rs:61:5
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ LL | | },
| |_____^

error: enum is never used: `ABC`
--> $DIR/lint-dead-code-4.rs:24:1
--> $DIR/lint-dead-code-4.rs:24:6
|
LL | enum ABC {
| ^^^^^^^^
| ^^^

error: variant is never constructed: `I`
--> $DIR/lint-dead-code-4.rs:36:5
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ LL | Variant6(isize),
| ^^^^^^^^^^^^^^^

error: enum is never used: `Enum3`
--> $DIR/lint-dead-code-5.rs:18:1
--> $DIR/lint-dead-code-5.rs:18:6
|
LL | enum Enum3 {
| ^^^^^^^^^^
| ^^^^^

error: aborting due to 4 previous errors

19 changes: 19 additions & 0 deletions src/test/ui/lint/dead-code/newline-span.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![deny(dead_code)]

fn unused() { //~ error: function is never used:
println!("blah");
}

fn unused2(var: i32) { //~ error: function is never used:
println!("foo {}", var);
}

fn unused3( //~ error: function is never used:
var: i32,
) {
println!("bar {}", var);
}

fn main() {
println!("Hello world!");
}
26 changes: 26 additions & 0 deletions src/test/ui/lint/dead-code/newline-span.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error: function is never used: `unused`
--> $DIR/newline-span.rs:3:4
|
LL | fn unused() {
| ^^^^^^
|
note: lint level defined here
--> $DIR/newline-span.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^

error: function is never used: `unused2`
--> $DIR/newline-span.rs:7:4
|
LL | fn unused2(var: i32) {
| ^^^^^^^

error: function is never used: `unused3`
--> $DIR/newline-span.rs:11:4
|
LL | fn unused3(
| ^^^^^^^

error: aborting due to 3 previous errors

File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: type alias is never used: `Unused`
--> $DIR/lint-dead-code-type-alias.rs:4:1
--> $DIR/type-alias.rs:4:1
|
LL | type Unused = u8;
| ^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-type-alias.rs:1:9
--> $DIR/type-alias.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
error: struct is never constructed: `F`
--> $DIR/lint-dead-code-unused-enum.rs:3:1
--> $DIR/unused-enum.rs:3:8
|
LL | struct F;
| ^^^^^^^^^
| ^
|
note: lint level defined here
--> $DIR/lint-dead-code-unused-enum.rs:1:9
--> $DIR/unused-enum.rs:1:9
|
LL | #![deny(unused)]
| ^^^^^^
= note: `#[deny(dead_code)]` implied by `#[deny(unused)]`

error: struct is never constructed: `B`
--> $DIR/lint-dead-code-unused-enum.rs:4:1
--> $DIR/unused-enum.rs:4:8
|
LL | struct B;
| ^^^^^^^^^
| ^

error: enum is never used: `E`
--> $DIR/lint-dead-code-unused-enum.rs:6:1
--> $DIR/unused-enum.rs:6:6
|
LL | enum E {
| ^^^^^^
| ^

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: variant is never constructed: `Bar`
--> $DIR/lint-dead-code-unused-variant.rs:8:5
--> $DIR/unused-struct-variant.rs:8:5
|
LL | Bar(B),
| ^^^^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-unused-variant.rs:1:9
--> $DIR/unused-struct-variant.rs:1:9
|
LL | #![deny(unused)]
| ^^^^^^
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: variant is never constructed: `Variant1`
--> $DIR/lint-dead-code-variant.rs:5:5
--> $DIR/unused-variant.rs:5:5
|
LL | Variant1,
| ^^^^^^^^
|
note: lint level defined here
--> $DIR/lint-dead-code-variant.rs:1:9
--> $DIR/unused-variant.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
error: function is never used: `foo`
--> $DIR/fail-no-dead-code-core.rs:7:1
--> $DIR/with-core-crate.rs:7:4
|
LL | fn foo() {
| ^^^^^^^^
| ^^^
|
note: lint level defined here
--> $DIR/fail-no-dead-code-core.rs:1:9
--> $DIR/with-core-crate.rs:1:9
|
LL | #![deny(dead_code)]
| ^^^^^^^^^
Expand Down
File renamed without changes.
8 changes: 4 additions & 4 deletions src/test/ui/path-lookahead.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// run-pass

#![warn(unused)]
#![allow(dead_code)]
#![warn(unused_parens)]

// Parser test for #37765

fn with_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `with_parens`
fn with_parens<T: ToString>(arg: T) -> String {
return (<T as ToString>::to_string(&arg)); //~WARN unnecessary parentheses around `return` value
}

fn no_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `no_parens`
fn no_parens<T: ToString>(arg: T) -> String {
return <T as ToString>::to_string(&arg);
}

Expand Down
Loading

0 comments on commit 838ed89

Please sign in to comment.