Allow hoisting else block stmts for while_let_loop
#16404
Conversation
rustbot
added
the
S-waiting-on-review
|
rustbot has assigned @samueltardieu. Use |
|
Is it okay to eliminate |
Yeah it's usually fine... but if you are able to come up with a refactoring, then by all means! |
tests/ui/while_let_loop.rs
Outdated
| // The suggestion is: | ||
| // ```rust | ||
| // while let Some(x) = std::hint::black_box(None::<i32>) { | ||
| // println!("x = {x}"); | ||
| // } | ||
| // println!("fail"); | ||
| // ``` |
There was a problem hiding this comment.
This should be fine to remove now, as the suggestion is contained in the .stderr file
| LL ~ while let Some(x) = std::hint::black_box(None::<i32>) { .. } | ||
| LL + println!("fail"); |
There was a problem hiding this comment.
One problem with this is that, since on of the statements is moved out, it's a bit unclear what exactly should the ellipses be replaced with. Not sure how to fix this, short of reintroducing body reconstruction in the suggestion
There was a problem hiding this comment.
what do you think about adding a help note to explain what the user should do with the { .. }? like so:
help: try
|
LL ~ while let Some(x) = y { .. }
LL + println!("fail");
|
= note: the statements above are hoisted from the `else` block; move the original loop body into the `while let`
| println!("x = {x}"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
it would be nice to add tests for:
- multiple statements
- a semicolon-less statement, like
if true { whatever; }
| "\n{indent} let {pat_str}{ty_str} = {init_str};\n{indent} ..\n{indent}", | ||
| indent = snippet_indent(cx, expr.span).unwrap_or_default(), | ||
| ) | ||
| format!("\n{indent} let {pat_str}{ty_str} = {init_str};\n{indent} ..\n{indent}",) |
There was a problem hiding this comment.
| format!("\n{indent} let {pat_str}{ty_str} = {init_str};\n{indent} ..\n{indent}",) | |
| format!("\n{indent} let {pat_str}{ty_str} = {init_str};\n{indent} ..\n{indent}") |
I wish rustfmt would handle this...
borngraced
force-pushed
the
while-let-loop-hoist-stmts
branch
from
67edf9b to
ea92abc
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustbot
added
has-merge-commits
borngraced
force-pushed
the
while-let-loop-hoist-stmts
branch
from
5d7b991 to
ea92abc
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
rustbot
removed
S-waiting-on-author
add test cases for semicolon_less and multiple stmts extract could_be_while_let fn param into a struct fix clippy warnings
borngraced
force-pushed
the
while-let-loop-hoist-stmts
branch
from
ea92abc to
b4bc45c
Compare
4 participants
fixes: #16393
changelog: [
while_let_loop]: Allow hoisting else block stmts