fix(while_let_loop): detect the pattern when the loop has a label - #17614
fix(while_let_loop): detect the pattern when the loop has a label#17614durationextender wants to merge 1 commit into
Conversation
rustbot
added
S-waiting-on-community-reviews
|
Thanks for the pull request. A reviewer will take a look after it receives 2 community reviews. In the meantime, we would highly appreciate if you could try to review any of PRs waiting on community reviews. |
This comment has been minimized.
This comment has been minimized.
| /// Checks if `block` contains a single unlabeled `break` expression or statement, possibly embedded | ||
| /// inside other blocks. | ||
| fn is_simple_break_block(block: &Block<'_>) -> bool { | ||
| /// Checks if `block` contains a single unlabeled and importantly now also a labeled `break` |
There was a problem hiding this comment.
| /// Checks if `block` contains a single unlabeled and importantly now also a labeled `break` | |
| /// Checks if `block` contains a single (labeled or unlabeled) `break` |
the "now" part is confusing, since we don't normally reference older versions of the code outside of regression tests; unclear why this is marked as "importantly"
| /// Checks if `expr` contains a single unlabeled `break` expression or statement, possibly embedded | ||
| /// inside other blocks. | ||
| fn is_simple_break_expr(expr: &Expr<'_>) -> bool { | ||
| /// Checks if `expr` contains a single unlabeled and importantly now also a labeled `break` |
There was a problem hiding this comment.
| /// Checks if `expr` contains a single unlabeled and importantly now also a labeled `break` | |
| /// Checks if `expr` contains a single (labeled or unlabeled ) `break` |
per above
|
Should be good now i hope. @rustbot ready |
3d03892 to
98939c0
Compare
This comment has been minimized.
This comment has been minimized.
|
As I mentioned in #17590, a while-let loop is allowed to be labeled, so the label should be preserved even if it's only a "comment" about what the loop is doing. |
Sorry, you still have @rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
98939c0 to
1292570
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
rustbot
removed
the
S-waiting-on-community-reviews
|
r? @blyxyas rustbot has assigned @blyxyas for the project review. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
Could you add tests with blocks inside those loops? And with those blocks labeled? What about if they share the same label? And only breaking from the inner/outer block?
Try to find the weirdest examples that you can think of. Lints don't usually break from run-of-the-mill common expressions, but from weird, sometimes macro-generated code. (=^-ω-^=)
rustbot
added
S-waiting-on-author
7 participants
while_let_loopwasn't firing when the loop had a label, since the break check assumed no label. Now it also acceptsbreak 'labelwhen it matches the loop's own label, while still ignoring breaks meant for a different outer loop.Covers
if let/else,let else, andmatchversions.fixes #17590
changelog: [
while_let_loop]: detect the pattern even when the enclosing loop has a label