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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

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

> Aha, I see this is consistent with the existing naming convention for this, so I'm ok with it.

Thanks!

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

I see that the discussion thread from last year regarding C++ API (lack of) stability didn't end up into something that was committed (https://lists.llvm.org/pipermail/llvm-dev/2020-July/143613.html); changing ADT APIs can probably cause a fair bit of churn for e.g. out of tree users. Then again, I guess these particular methods are a bit less frequently used than the rest of ADT, so it's maybe not a biggie? Or should we be kind and do it as a two-stage change, adding new methods with new names and flagging the old ones as deprecated, and removing the deprecated ones in a week or two?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104218