-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Clippy: Backport needless_return
fix
#131492
Conversation
Don't warn on proc macro generated code in `needless_return` Fixes rust-lang#13458 Fixes rust-lang#13457 Fixes rust-lang#13467 Fixes rust-lang#13479 Fixes rust-lang#13481 Fixes rust-lang#13526 Fixes rust-lang#13486 The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does* fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`. The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context. "Hide whitespace" helps a bit for reviewing the proc macro detection change changelog: none
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
@bors r+ p=1 |
…atthiaskrgr Clippy: Backport `needless_return` fix r? `@Manishearth` This cherry-picks rust-lang/rust-clippy#13464, so that it gets into master and with that into `beta` tomorrow, so that the bug in this lint doesn't hit `beta`. Changes look quite big, but most of them are whitespace changes because of the introduction of an `_inner` function. In reality it only adds 2 checks.
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Build error doesn't look related? @bors retry |
Build failure is #127883, unrelated. |
…=matthiaskrgr Clippy: Backport `needless_return` fix r? `@Manishearth` This cherry-picks rust-lang/rust-clippy#13464, so that it gets into master and with that into `beta` tomorrow, so that the bug in this lint doesn't hit `beta`. Changes look quite big, but most of them are whitespace changes because of the introduction of an `_inner` function. In reality it only adds 2 checks.
…=matthiaskrgr Clippy: Backport `needless_return` fix r? ``@Manishearth`` This cherry-picks rust-lang/rust-clippy#13464, so that it gets into master and with that into `beta` tomorrow, so that the bug in this lint doesn't hit `beta`. Changes look quite big, but most of them are whitespace changes because of the introduction of an `_inner` function. In reality it only adds 2 checks.
☀️ Test successful - checks-actions |
Finished benchmarking commit (d0141af): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 2.9%, secondary 0.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 779.131s -> 781.528s (0.31%) |
r? @Manishearth
This cherry-picks rust-lang/rust-clippy#13464, so that it gets into master and with that into
beta
tomorrow, so that the bug in this lint doesn't hitbeta
.Changes look quite big, but most of them are whitespace changes because of the introduction of an
_inner
function. In reality it only adds 2 checks.