[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