dead_code lint highlights too much on functions with multi-line signatures

[alvinhochun]

With the following example code:

fn unused() {
    println!("blah");
}

fn unused2(var: i32) {
    println!("foo {}", var);
}

fn unused3(
    var: i32,
) {
    println!("bar {}", var);
}

fn main() {
    println!("Hello world!");
}

(Playground)

The compiler produces these warnings:

warning: function is never used: `unused`
 --> src/main.rs:1:1
  |
1 | fn unused() {
  | ^^^^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: function is never used: `unused2`
 --> src/main.rs:5:1
  |
5 | fn unused2(var: i32) {
  | ^^^^^^^^^^^^^^^^^^^^

warning: function is never used: `unused3`
  --> src/main.rs:9:1
   |
9  | / fn unused3(
10 | |     var: i32,
11 | | ) {
12 | |     println!("bar {}", var);
13 | | }
   | |_^

Notice that for fn unused and fn unused2, dead_code is reported on only the first line signature of the function, however for fn unused3 dead_code is reported on the whole function from start to finish.

This is mostly annoying when writing new functions in an editor like vscode that rls will cause the whole function to be orange-wavy-underlined, making a huge distraction.

[jakubadamw]

I've seen this as well.

It's been years since I hacked on the compiler, so I'll claim this as a bug to fix so as to refresh my memory of what's where. :)

[estebank]

There's a method SourceMap::def_span that explicitly looks for the next { token. If the resulting span is a single line, it uses that, otherwise it uses the full span under the assumption that the multiline span would look weird:

warning: function is never used: `unused3`
  --> src/main.rs:9:1
   |
9  | / fn unused3(
10 | |     var: i32,
11 | | ) {
   | |_^

With that, most of the places where we are calling def_span could be changed so that they get the Ident for the function and use its span instead (we used not to encode Spans in Idents due to size concerns). With this change, the resulting output would be:

warning: function is never used: `unused`
 --> src/main.rs:1:1
  |
1 | fn unused() {
  |    ^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: function is never used: `unused2`
 --> src/main.rs:5:1
  |
5 | fn unused2(var: i32) {
  |    ^^^^^^^

warning: function is never used: `unused3`
  --> src/main.rs:9:1
   |
9  | fn unused3(
   |    ^^^^^^^

The reason I haven't pushed for that change is because in some cases the extra information from the other lines is useful to give context, but for errors like this one we just need to identify the function and nothing more.

[jakubadamw]

@estebank, thanks for explaining. This would be in line with what I'd already figured. 🙂 By the way, that function assumes that items would have a leading { in their definition, whereas that's not the case for top-level modules. I filed #63091 with an example of that breaking (and that's regardless of whether this gets fixed).

[jakubadamw]

Oh, this has been reported before: #58729.

[estebank]

Closing as duplicate of #58729