implicit_return improvements
#6951
Conversation
|
r? @flip1995 (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
Jarcho
force-pushed
the
implicit_return_fp
branch
6 times, most recently
from
11916da to
ffa25ec
Compare
|
☔ The latest upstream changes (presumably #6971) made this pull request unmergeable. Please resolve the merge conflicts. |
Jarcho
force-pushed
the
implicit_return_fp
branch
from
ffa25ec to
edb76df
Compare
|
Sorry for taking so long with my reviews recently... Is there any chance you can split this PR in multiple commits? |
Jarcho
force-pushed
the
implicit_return_fp
branch
from
edb76df to
267834e
Compare
|
Ended up doing a bit of a rewrite at the same time. The code would be simpler to add a return to the top level expression rather to each individual sub expression (e.g. conditionals get a return added to each branch), but that would change the behaviour of the lint. |
Jarcho
force-pushed
the
implicit_return_fp
branch
from
267834e to
f6e7a5a
Compare
|
☔ The latest upstream changes (presumably #6931) made this pull request unmergeable. Please resolve the merge conflicts. |
Jarcho
force-pushed
the
implicit_return_fp
branch
from
f6e7a5a to
88dc123
Compare
|
☔ The latest upstream changes (presumably #7043) made this pull request unmergeable. Please resolve the merge conflicts. |
Jarcho
force-pushed
the
implicit_return_fp
branch
from
88dc123 to
6d6f933
Compare
|
☔ The latest upstream changes (presumably #7046) made this pull request unmergeable. Please resolve the merge conflicts. |
Jarcho
force-pushed
the
implicit_return_fp
branch
from
6d6f933 to
a7ddaf6
Compare
|
☔ The latest upstream changes (presumably #7047) made this pull request unmergeable. Please resolve the merge conflicts. |
flip1995
left a comment
flip1995
left a comment
There was a problem hiding this comment.
Thanks! This LGTM overall. I still have to double check the lint_implicit_returns function, because I only skimmed it in this review round. I only found two documentation/style NITs.
|
You don't have to keep rebasing your branch. Best thing to do here is to just add commits and rebase/squash it once before merge. It's easier for me to review and you don't have to resolve conflicts every other day. |
flip1995
added
S-waiting-on-author
|
@Jarcho I'd still like to see this change: #6951 (comment) Otherwise this LGTM. |
|
Looks like I forgot to change that. |
Better suggestions when returning macro calls. Suggest changeing all the break expressions in a loop, not just the final statement. Don't lint divergent functions. Don't suggest returning the result of any divergent fuction.
Jarcho
force-pushed
the
implicit_return_fp
branch
from
7154e32 to
3d793f3
Compare
|
@bors r+ Thanks! |
|
📌 Commit 3d793f3 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
4 participants
fixes: #6940
changelog: Fix
implicit_returnsuggestion for async functionschangelog: Improve
implicit_returnsuggestions when returning the result of a macrochangelog: Check for
breakexpressions inside a loop which are then implicitly returnedchangelog: Allow all diverging functions in
implicit_return, not just panic functions