[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