[PATCH] D104218: [ADT] Add StringRef consume_front_lower and consume_back_lower

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 14 23:19:07 PDT 2021


mstorsjo added a comment.

In D104218#2817946 <https://reviews.llvm.org/D104218#2817946>, @MaskRay wrote:

> Can you show a patch which will benefit from this? Otherwise this looks good.

I have one existing use with a local helper function in llvm-rc here: https://github.com/llvm/llvm-project/blob/d8c5a4d6b6efad405c71ead8997276d8d3a7c5ad/llvm/tools/llvm-rc/llvm-rc.cpp#L259-L277 And I want to use it similarly in D104212 <https://reviews.llvm.org/D104212>. (llvm-ar does case insensitive switching between different modes of the same binary based on `argv[0]` this way, https://github.com/llvm/llvm-project/blob/d8c5a4d6b6efad405c71ead8997276d8d3a7c5ad/llvm/tools/llvm-ar/llvm-ar.cpp#L1273-L1291, so therefore I suppose it's good to keep the rest of the parsing of `argv[0]` similarly case insensitive.)

In D104218#2818302 <https://reviews.llvm.org/D104218#2818302>, @lattner wrote:

> I don't think this is general enough requirement to add a specialized method for it.  Did you consider adding a consumes_front/back overload that takes a lambda?

I didn't, do you mean something like this?

  bool consume_front(function_ref<bool(StringRef)> F) {
    if (!F(*this))
      return false;
  
    *this = drop_front(/*how much?*/);
    return true;
  }

Then the info about how much to drop has to be passed separately somehow, maybe by having the lambda return a `std::pair<bool, size_t>`? I guess that'd work and keep it generic, but it loses all the convenience of the function in such string parsing code - right now it's pretty straightforward to write compact and efficient parsing code with the other StringRef functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104218



More information about the llvm-commits mailing list