Remove needless returns detected by clippy in the compiler #130114
Conversation
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
rustbot
added
O-unix
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
|
@eduardosm can you split off the library parts into their own PR? |
eduardosm
force-pushed
the
needless-returns
branch
from
339bdc4 to
24ebe34
Compare
eduardosm
changed the title
|
Done, libraries part here |
eduardosm
changed the title
|
It's not obvious that these are all harmless changes - sometimes the explicit return expresses intent, so it should be preserved to account for future possible refactorings even if it does not make a difference right now.
|
eduardosm
force-pushed
the
needless-returns
branch
from
24ebe34 to
20e8fa1
Compare
|
I undid some of the changes, trying to keep only the most obvious ones. |
| @@ -336,7 +336,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |||
| return true; | |||
| } | |||
| } | |||
| return false; | |||
There was a problem hiding this comment.
IMO it is nicer, for symmetry with the return just a few lines above, to use return here.
That's exactly why I don't like just forcing such clippy lints over the entire codebase...
There was a problem hiding this comment.
I have exactly the opposite opinion. I like the consistency that tools like clippy encourage, and in this case I think the change makes the "early return" and "default/fallthrough" visually distinct, which I find easier to read.
There was a problem hiding this comment.
The last two returns here are not distinct in a meaningful sense, so it is not a good idea to make them visually distinct.
There was a problem hiding this comment.
I agree with @saethlin. I think it's just plain odd to have return value; at the end of a block when it's not suggesting exceptional control flow.
I think it's actually very odd that miri has allow(clippy::needless_return) on its codebase, but for the rest of the compiler I think this should be fixed.
eduardosm
force-pushed
the
needless-returns
branch
from
20e8fa1 to
0b20ffc
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Oops, closed the wrong PR !! Meant to close mine which is a duplicate :) |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…iler-errors Remove needless returns detected by clippy in the compiler
…kingjubilee Rollup of 11 pull requests Successful merges: - rust-lang#119286 (show linker output even if the linker succeeds) - rust-lang#129103 (Don't warn empty branches unreachable for now) - rust-lang#129696 (update stdarch) - rust-lang#129835 (enable const-float-classify test, and test_next_up/down on 32bit x86) - rust-lang#129992 (Update compiler-builtins to 0.1.125) - rust-lang#130052 (Don't leave debug locations for constants sitting on the builder indefinitely) - rust-lang#130077 (Fix linking error when compiling for 32-bit watchOS) - rust-lang#130114 (Remove needless returns detected by clippy in the compiler) - rust-lang#130156 (Add test for S_OBJNAME & update test for LF_BUILDINFO cl and cmd) - rust-lang#130168 (maint: update docs for change_time ext and doc links) - rust-lang#130239 (miri: fix overflow detection for unsigned pointer offset) r? `@ghost` `@rustbot` modify labels: rollup
…kingjubilee Rollup of 10 pull requests Successful merges: - rust-lang#129103 (Don't warn empty branches unreachable for now) - rust-lang#129696 (update stdarch) - rust-lang#129835 (enable const-float-classify test, and test_next_up/down on 32bit x86) - rust-lang#130077 (Fix linking error when compiling for 32-bit watchOS) - rust-lang#130114 (Remove needless returns detected by clippy in the compiler) - rust-lang#130168 (maint: update docs for change_time ext and doc links) - rust-lang#130228 (notify Miri when intrinsics are changed) - rust-lang#130239 (miri: fix overflow detection for unsigned pointer offset) - rust-lang#130244 (Use the same span for attributes and Try expansion of ?) - rust-lang#130248 (Limit `libc::link` usage to `nto70` target only, not NTO OS) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#130114 - eduardosm:needless-returns, r=compiler-errors Remove needless returns detected by clippy in the compiler
No description provided.