[PATCH] D113892: [NFC][clangd] cleanup llvm-else-after-return findings

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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?



================
Comment at: clang-tools-extra/clangd/JSONTransport.cpp:260
       continue;
     } else {
       // An empty line indicates the end of headers.
----------------
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)


================
Comment at: clang-tools-extra/clangd/JSONTransport.cpp:283
+    Read = retryAfterSignalUnlessShutdown(
+        0, [&] { return std::fread(&JSON[Pos], 1, ContentLength - Pos, In); });
     if (Read == 0) {
----------------
Looks like there are unrelated formatting changes in a couple of files


================
Comment at: clang-tools-extra/clangd/Selection.cpp:350
       }
+      /* fall through and treat as part of the macro body */
     }
----------------
FWIW I find this one *much* less clear :-(
I'd personally prefer to ignore the rule in this case, but curious what others think


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113892/new/

https://reviews.llvm.org/D113892