[PATCH] D104218: [ADT] Add StringRef consume_front_lower and consume_back_lower
Duncan P. N. Exon Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 22 10:54:44 PDT 2021
dexonsmith added a comment.
In D104218#2832463 <https://reviews.llvm.org/D104218#2832463>, @mstorsjo wrote:
> In D104218#2832251 <https://reviews.llvm.org/D104218#2832251>, @lattner wrote:
>
>> That said, `xxx_lower()` is a really terrible name here for insensitive comparisons. We have `StringRef::lower()` which returns a lower case string - it isn't insensitive. It seems that the whole family needs a rename. Could you take a crack at this as a follow-up? It should be a mechanical change.
>
> Sure, I can give that a shot. Any suggestion on what the name pattern should be? `xxx_caseinsensitive()` is a bit of a mouthful, `xxx_i()` is terse but less understandable and discoverable.
A few ideas to consider:
- `X_casei()`
- `X_nocase()`
- `X_ignorecase()`
- `X_foldcase()` (term from text search)
(Looks like `X_lower()` has been with us for over a decade (68e4945c03eeeb04773587f3b265b0888bb22549 / r87020), but I think equal_lower and compare_lower were less ambiguous than the newer ones.)
In D104218#2832677 <https://reviews.llvm.org/D104218#2832677>, @foad wrote:
>> Btw, if you happen to be able to check easily in the diff between your downstream fork vs upstream, do you happen to have a lot use of these `_lower()` method calls, outside of code in upstream?
>
> No, none actually! This was more of a general comment about how to do incompatible changes in a downstream-friendly way.
Right, no real need for a buffer in time. Once the change is split in two, it's easy for downstreams to temporarily revert the second commit while they update out-of-tree code at their own pace.
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