Message suggesting prefixing with underscore is unhelpful

[Kixunil]

I believe that in the vast majority of cases when an unused variable is present, the author of the code intended to use it, but forgot. Thus the message suggesting to effectively turn off the warning is unhelpful.

I suggest changing it to something like:

help: you probably wanted to use it somehow, check the code around to see if it's the case
help: if you really intend to create the variable and not use it, prefix it with underscore: `_var`

While this doesn't concern me much personally, I believe it'd be much more helpful to novice programmers. To put it in extreme, imagine this kind of lint message: "Reference outlives borrowed value. Consider putting #![allow(undefined_behavior)] at the top of your module."

And I must admit the current warning feels a bit strange to me too. ("Dear compiler, the content of your message is completely useless, but its presence saved my ass twice today, so thank you for that!")

Disclaimer: this is entirely based on my personal experience and may not be true in general. Feel free to close if there's a good evidence that I'm broken, not the compiler. :)

This issue has been assigned to @loris-intergalactique via this comment.

[loris-intergalactique]

Hi, I'm a novice and would like to help ! I feel the same way about this message :)

[loris-intergalactique]

What do you think about mentioning the lifetime of the variable ?, like the following snippet from the Rust book :

warning: unused variable `i` in the lifetime starting at 1:1 and ending at 5:1
 --> src/main.rs:2:9
1 |  / fn main() {
...  |
2 |  |    let i = 3; // Lifetime for `i` starts. ────────────────┐
3 |  |     //                                                    │
...  |
4 |  |     //                                                    │
5 |  | }   // Lifetime ends. ────────────────────────────────────┘  
  |  |_^
  |  help: if the variable is meant to be unused, consider prefixing with an underscore: `_i`
  |
  = note: `#[warn(unused_variables)]` on by default

Or maybe something like :

note: ...the reference/variable is valid for the lifetime 'a as defined on the impl at 27:1...
  --> src\lib.rs:27:1
   |
27 | / impl<'a> MyImpl<'a>{
28 | |
29 | |     pub fn myfunc(i: &String) -> MyImpl{
30 | |         ...
...  |
89 | |     }
90 | | }
   | |_^

What's your opinion ? Am I too novice ?

[soumyalahiri]

Hello! I'm new to this repo and I'd like to take up this issue.

[Kixunil]

@loris-intergalactique what does it have to do with lifetimes?

[loris-intergalactique]

@Kixunil When I wrote the message, I didn't think about encapsulated lifetimes, my bad - This could have made the hints maybe even less helpful. What I think could give an insight to programmers about what they tried to accomplish is actually mentionning the function name, not the lifetime.

I think that in a huge code base this could make save some time, and help checking the code around.

I imagine that if you had something like :

warning: unused variable `bark` in the function new_dog()
--> src/main.rs:2:9
1 |  / fn new_dog() {
...  |
2 |  |    let bark = Bark::from("bark"); 
  |  |        ^^^^ help: you probably wanted to use it somehow, check the code around to see if it's the case
  |  |             help: if the variable is meant to be unused, consider prefixing with an underscore: `_bark`
  |
  = note: `#[warn(unused_variables)]` on by default

You'd have more hints about what's going on. With only weeks of practicing Rust, I don't know every use cases yet. But I think that combining the nice message that you gave, as well as another hint on where this variable is could be nice++

What do you think about that ?

[Kixunil]

Ah I think you mean to mention the scope in which the variable is introduced, right? That would be very interesting, I think.

[loris-intergalactique]

Yes, that's right !

I'm gonna try implenting it and testing it. I'm happy to make my first contribution ! :)
@rustbot claim

Hi, just a quick edit to say that I am still on it. I have more work than usual, but I do want to make this contrib ! :)

[shika-blyat]

Are you still working on it @loris-intergalactique ?

[loris-intergalactique]

Hi @shika-blyat, I do but I have been having some issues working with the compiler. Maybe I should let this issue go, unassign myself and take the time to learn step by step how the compiler works before.

If you feel like you can take this issue, do it. I'll contribute somewhere else when I'll be maturer with rust 😄

[aleksator]

I think the suggested message is overly verbose. If we are to change it, let's do with something succinct: if this is intentional prefix it with an underscore.
The whole message would then look like:

warning: unused variable: `not_used`
 --> src/main.rs:2:9
  |
2 |     let not_used = 5;
  |         ^^^^^^^^ help: if this is intentional prefix it with an underscore: `_not_used`
  |
  = note: `#[warn(unused_variables)]` on by default

This has the advantages of:

• conveying that not using the variable was most likely not intentional;
• suggesting a fix if it was;
• being brief at the same time.

Feedback on keeping it short in general and on the exact wording is welcome.
I can submit a PR with this change if this seems like an improvement.

[shika-blyat]

rust/src/librustc_passes/liveness.rs

Line 1568 in 5f13820

"consider prefixing with an underscore",

I think this is the involved line.

[shika-blyat]

I'd be interested to make this contribution (i think i'm able to do it 😄), but i don't know if i should wait for some kind of confirmation.

[aleksator]

I'd be interested to make this contribution (i think i'm able to do it smile), but i don't know if i should wait for some kind of confirmation.

Yeah, that's why I asked about the exact wording before submitting the PR

[Kixunil]

@aleksator I like that wording. It's not as helpful for beginners in programming as the other was, but maybe it's safe to assume that most Rust devs aren't beginners in programming. Might be fun to have an opt-in plugin/wrapper/mode/whatever for the complete beginners one day in the future.

[aleksator]

Submitted a PR , adding a comma that should have been there:
if this is intentional, prefix it with an underscore

[loris-intergalactique]

Hi @aleksator, thanks for this ! May I ask how did you test it, and how did you use x.py ?

My problem when working on this issue was not to find the location of the message that was solved with a quick grep, but on the underlying tests. Each time I compiled it in order to make tests, I stumbled upon multiple errors that were not really linked to what I changed. it really slowed me down..

[aleksator]

@loris-intergalactique My whole process to solve this issue was:

I searched for all occurrences of previous warning message using search and replace feature of my editor in the project folder, and then pressed "replace all".

For testing things locally, I run:
./x.py build -i stage1
The resulting compiler was added to my toolchain list like this:
rustup toolchain link stage1 build/<host-triple>/stage1
I found this and other x.py related commands in the rustc developer's guide, link to which I noticed at the top of this project's README.

I created a simple project with cargo new and added an unused variable to the main. I then run
cargo +stage1 build to test it with the newly built compiler, and saw my new message appear.

I did not run the whole test suite for this PR (since I thought it shouldn't break anything after changing the output of all related test cases to use the new message), and submitted my changes to be tested by CI here.

I did test the other PR locally (if you are interested, it was this one to help resolve build issues here), and the same technique could be used for this one as well. For that I run:
./x.py build && ./x.py test --stage 2

If you notice unrelated build errors at this point, something like./x.py clean (preserves built LLVM) or cargo clean should help by removing old build artifacts.

[loris-intergalactique]

Awesome, thank you ! I know what I did wrong now 👍

[aleksator]

@loris-intergalactique What was that? Maybe we can submit a PR to the documentation to make things clearer.

[loris-intergalactique]

I think that my issues are on me on this one ! hahaha

• I built the wrong things, spending my time watching LLVM compilation logs
• I wanted to use HIR data types and got too far in the docs
• I mistakenly thought that the section "creating a rustup toolchain" was not useful for my case

I learned a lot of things along the road though, so I'm still happy !😄

Edit : And I also became acquainted with the message that was secretly haunting me in some cases ! :

> dmesg
Out of memory: Kill process 10331 (rustc) score 624 or sacrifice child

[Kixunil]

Good job guys!