-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Reduce span highlighted code in unused_variables lint #50675
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
Conversation
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
r? @estebank |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some nits
src/librustc/middle/liveness.rs
Outdated
@@ -1539,8 +1545,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { | |||
self.assigned_on_exit(ln, var).is_some() | |||
}; | |||
|
|||
let suggest_underscore_msg = format!("consider using `_{}` instead", | |||
name); | |||
let (suggest_underscore_msg, app) = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this to two separate let bindings
@@ -35,6 +35,10 @@ fn main() { | |||
endless_and_singing: true | |||
}; | |||
|
|||
let mut mut_unused_var = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if that works, but you can try adding a // run-rustfix
comment to the top of the file. That'll show us the file after applying the suggestions
@@ -1398,7 +1399,11 @@ fn check_local<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, local: &'tcx hir::Local) | |||
}, | |||
None => { | |||
this.pat_bindings(&local.pat, |this, ln, var, sp, id| { | |||
this.warn_about_unused(sp, id, ln, var); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the if let you could prepend this line with let sp = local.pat.simple_span.unwrap_or(sp);
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@killercup what actions are needed here? |
@oli-obk I had talked with @killercup on discord, the failed test will be fixed once a new version rustfix published. |
In addition to the We should land this right now, but without the rustfix test mode and the applicability flag. After a rustfix update, we can then add the applicability flag and "prove" it's working by adding the rustfix tests. |
This reverts commit a880971.
makes sense. @bors r+ |
📌 Commit c4e99dd has been approved by |
Reduce span highlighted code in unused_variables lint Fixes #50472 - [X] reduce var span - [ ] mark applicable Before: ``` mut unused_mut_var ^^^^^^^^^^^^^^^^^^ ``` After: ``` mut unused_mut_var ^^^^^^^^^^^^^^ ```
☀️ Test successful - status-appveyor, status-travis |
Fixes #50472
Before:
After: