[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