[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D77572#1967956 <https://reviews.llvm.org/D77572#1967956>, @mgehre wrote:

> In D77572#1965143 <https://reviews.llvm.org/D77572#1965143>, @njames93 wrote:
>
> > I'm struggling to see the value of this check though. If it was reworked to check for loop in the middle of a function it would have a lot more value.
>
>
> This is (just) a first step. The next step is to detect the local variable case as you also described it. Note that this also catches functions
>  that do some preprocessing and end with a any_of-style loop.
>  I also have a local branch that generates fixits, but they add quite some code, so I wanted to put them in a separate PR.
>
> In LLVM & clang, the check in this PR already emits 370 unique warnings.


Take this example from TableGen/Record.cpp:

  bool CondOpInit::isConcrete() const {
    for (const Init *Case : getConds())
      if (!Case->isConcrete())
        return false;
  
    for (const Init *Val : getVals())
      if (!Val->isConcrete())
        return false;
  
    return true;
  }

Firstly, the warning is only emitted on the second loop.
Secondly how does your fix it code handle temporaries?
Would it transform to

  bool CondOpInit::isConcrete() const {
    for (const Init *Case : getConds())
      if (!Case->isConcrete())
        return false;
  
    return std::all_of(getVals().begin(), getVals().end(),
                       [](const Init *Val) { return Val->isConcrete(); });
  }

I'd argue that it actually makes the code less readable as there are 2 different constructs for the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572