-
Notifications
You must be signed in to change notification settings - Fork 1.9k
needless_late_init: ignore if let, let mut and significant drops
#8617
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? @giraffate (rust-highfive has picked a reviewer for you, use r? to override) |
|
☔ The latest upstream changes (presumably #8563) made this pull request unmergeable. Please resolve the merge conflicts. |
bb2a79e to
491d4d5
Compare
|
☔ The latest upstream changes (presumably #8705) made this pull request unmergeable. Please resolve the merge conflicts. |
491d4d5 to
1be706a
Compare
|
Sorry for the late reviewing. I should be able to start the review by next week. |
| seen | ||
| } | ||
|
|
||
| fn contains_let(cond: &Expr<'_>) -> bool { |
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.
Is it possible to replace this with higher::IfLet::hir?
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.
It wouldn't be for let chains
tests/ui/needless_late_init.rs
Outdated
|
|
||
| println!(); |
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.
nits
| println!(); |
tests/ui/needless_late_init.rs
Outdated
|
|
||
| println!(); |
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.
nits
| println!(); |
| h = format!("{}", e); | ||
|
|
||
| println!("{}", a); | ||
| println!(); |
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.
nits
| println!(); |
1be706a to
1d1fecf
Compare
|
@bors r+ Thanks! |
|
📌 Commit 1d1fecf has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
No longer lints
if let, personal taste on this one is pretty split, so it probably shouldn't be warning by default. Fixes #8613No longer lints
let mut, things like the following are not uncommon and look fine as they arehttps://github.com/shepmaster/twox-hash/blob/b169c16d86eb8ea4a296b0acb9d00ca7e3c3005f/src/sixty_four.rs#L88-L93
Avoids changing the drop order in an observable way, where the type of
xhas a drop with side effects and something betweenxand the first use also does, e.g.https://github.com/denoland/rusty_v8/blob/48cc6cb791cac57d22fab1a2feaa92da8ddc9a68/tests/test_api.rs#L159-L167
The implementation of
type_needs_ordered_drop_innerwas changed a bit, it now usesTy::has_significant_dropand reordered the ifs to check diagnostic name before checking the implicit drop implchangelog: [
needless_late_init]: No longer lintsif letstatements,let mutbindings and no longer significantly changes drop order