This is an archive of the discontinued LLVM Phabricator instance.

[NFC][clangd] cleanup llvm-else-after-return findings
ClosedPublic

Authored by kuhnel on Nov 15 2021, 7:01 AM.

Details

Summary

Cleanup of clang-tidy findings: removing "else" after a return statement
to improve readability of the code.

This patch was created by applying the clang-tidy fixes automatically.

Diff Detail

Event Timeline

kuhnel created this revision.Nov 15 2021, 7:01 AM
kuhnel published this revision for review.Nov 15 2021, 8:08 AM

Mostly looks good. There's a bunch of ifs that are now trivial (condition and body each fit on one line, no else) and we'd usually drop braces there, but it's probably not worth the hassle.

I have to say that this rule slightly improves "early exit after error condition" type code, but makes it less clear that "there are 3 cases" type code is exhaustive.
So personally I'm not sure the check is a win if we want to follow it everywhere, I'd consider fixing the cases where it improves things and then turning it off. @kadircet am I being too picky here?

clang-tools-extra/clangd/JSONTransport.cpp
255–256

This seems inconsistent: we're elseing after continue.

If we've using this style, i'd invert the if here so the "continue" case just falls off the end of the loop body. (But keep the comment)

284

Looks like there are unrelated formatting changes in a couple of files

clang-tools-extra/clangd/Selection.cpp
352

FWIW I find this one *much* less clear :-(
I'd personally prefer to ignore the rule in this case, but curious what others think

sammccall accepted this revision.Nov 16 2021, 5:24 AM

Discussed offline, I believe the plan is to NOLINT the locations where we're handling several cases exhaustively.

clang-tools-extra/clangd/Selection.cpp
352

I think this is a nolint

This revision is now accepted and ready to land.Nov 16 2021, 5:24 AM
kuhnel updated this revision to Diff 387918.Nov 17 2021, 5:44 AM
kuhnel marked 4 inline comments as done.

addressed Sam's comments

Sorry for the continued mess with automatic code formatting. Somehow VSCode forgot my settings :(

sammccall accepted this revision.Nov 17 2021, 6:08 AM

Thanks!

This revision was automatically updated to reflect the committed changes.