[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D82825#2123125 <https://reviews.llvm.org/D82825#2123125>, @njames93 wrote:

> In D82825#2122789 <https://reviews.llvm.org/D82825#2122789>, @aaron.ballman wrote:
>
> > Other than the documentation and settling on the option name in the parent revision, I think this LGTM (the changes will be mechanical and can be done without further review once the parent commit is in, unless you want more eyes on it).
>
>
> Are you happy with the reduced warnings in the this alias, when running locally over llvm I noticed a lot of warnings about else after return where condition variables are used. usually of the format
>
>   if (llvm::Expected<T> X = ... ) {
>     return *X;
>   }
>   else {
>     handle(X.takeError());
>   }


I can see arguments either way on this. Personally, I would not want the check to diagnose this code because that would encourage people to widen the scope and lifetime of `X` or require them to introduce a new compound scope to get the same behavior, and I think that's more problematic than the `else` following a `return`. I am not certain if others feel the same way though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82825