[PATCH] D131886: Use llvm::all_of (NFC)

Kazu Hirata via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 15 13:31:24 PDT 2022


kazu added inline comments.


================
Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:1266-1272
+    if (llvm::all_of(Data, [](Node *P) { return P->ArrayCache == Cache::No; }))
       ArrayCache = Cache::No;
-    if (std::all_of(Data.begin(), Data.end(), [](Node* P) {
-          return P->FunctionCache == Cache::No;
-        }))
+    if (llvm::all_of(Data,
+                     [](Node *P) { return P->FunctionCache == Cache::No; }))
       FunctionCache = Cache::No;
-    if (std::all_of(Data.begin(), Data.end(), [](Node* P) {
-          return P->RHSComponentCache == Cache::No;
-        }))
+    if (llvm::all_of(Data,
+                     [](Node *P) { return P->RHSComponentCache == Cache::No; }))
----------------
I am not sure if it's a good idea to modify this file without making corresponding changes to `libcxxabi/src/demangle/ItaniumDemangle.h`.  Note that the comment at the beginning of the file says:

```
// Generic itanium demangler library.
// There are two copies of this file in the source tree.  The one under
// libcxxabi is the original and the one under llvm is the copy.  Use
// cp-to-llvm.sh to update the copy.  See README.txt for more details.
```

Also, we probably don't want to introduce dependence on llvm headers in `libcxxabi/src/demangle/ItaniumDemangle.h`.  In turn, we probably shouldn't include llvm headers in `llvm/include/llvm/Demangle/ItaniumDemangle.h` either.





================
Comment at: llvm/lib/ProfileData/InstrProfReader.cpp:165
   return count == 0 ||
-         std::all_of(buffer.begin(), buffer.begin() + count,
-                     [](char c) { return isPrint(c) || isSpace(c); });
+         llvm::all_of(drop_end(buffer, buffer.size() - count),
+                      [](char c) { return isPrint(c) || isSpace(c); });
----------------
I am not sure if this change improves readability.  `drop_end(buffer, buffer.size() - count)` makes it harder to see the original intention that we iterate on the first `count` elements.  That is, we would be asking readers to compute `buffer.size() - (buffer.size() - count)`, which evaluates to `count`.

I would be inclined to wait for `std::ranges::views::take` in C++20.  If you really wanted to, you could implement an equivalent of `std::ranges::take_view` or something in `llvm/include/llvm/ADT/STLExtras.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131886



More information about the llvm-commits mailing list