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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
