Page MenuHomePhabricator

Use llvm::all_of (NFC)

Authored by Miss_Grape on Aug 15 2022, 6:07 AM.


Diff Detail

Event Timeline

Miss_Grape created this revision.Aug 15 2022, 6:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 15 2022, 6:07 AM
Miss_Grape requested review of this revision.Aug 15 2022, 6:07 AM
Herald added a project: Restricted Project. · View Herald Transcript
kazu added inline comments.Aug 15 2022, 1:31 PM
1266–1272 ↗(On Diff #452646)

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
// 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.

165 ↗(On Diff #452646)

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.

Miss_Grape edited the summary of this revision. (Show Details)
Miss_Grape marked 2 inline comments as done.
kazu accepted this revision.Aug 15 2022, 10:42 PM

LGTM. Thank you for updating the patch!

This revision is now accepted and ready to land.Aug 15 2022, 10:42 PM
This revision was automatically updated to reflect the committed changes.