[Lldb-commits] [PATCH] D104819: [ADT] Rename StringRef case insensitive methods for clarity

Thu Jun 24 12:09:03 PDT 2021

mstorsjo added a comment.

In D104819#2839263 <https://reviews.llvm.org/D104819#2839263>, @dexonsmith wrote:

> LGTM, once @MaskRay is happy. I have a couple of minor comments inline too.
>
> (I also see that there are some clang-format suggestions in the unit tests; not sure any of them are actually improvements so maybe you left those out intentionally?

Yeah those seem to be intentionally vertically aligned that way, no big opinion either way

> Just be sure to clang-format when you do the mechanical changes in the follow up patches.)

Hmm, those would have to be manually reviewed, but I guess I can try to do that (discarding changes where the surrounding code isn't too well formatted now already so it does more extensive reformattings other than for the renaming).

================
Comment at: llvm/lib/Support/StringRef.cpp:37

-/// compare_lower - Compare strings, ignoring case.
-int StringRef::compare_lower(StringRef RHS) const {
+/// compare_insensitive - Compare strings, ignoring case.
+int StringRef::compare_insensitive(StringRef RHS) const {
----------------
dexonsmith wrote:
> You can drop these duplicate doxygen comments entirely from the `.cpp` file.
Sure, yeah doxygen in implementation files make little sense anyway.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104819