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

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 24 11:54:02 PDT 2021


dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

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? Just be sure to clang-format when you do the mechanical changes in the follow up patches.)



================
Comment at: llvm/include/llvm/ADT/StringRef.h:198
 
+    LLVM_NODISCARD
+    bool equals_lower(StringRef RHS) const { return equals_insensitive(RHS); }
----------------
You could probably drop the `LLVM_NODISCARD` from the soon-to-be-deleted `_lower` variants, but maybe it's not worth fussing with (up to you).


================
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 {
----------------
You can drop these duplicate doxygen comments entirely from the `.cpp` file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104819



More information about the cfe-commits mailing list