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

Martin Storsjö via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 24 02:39:31 PDT 2021


mstorsjo added a comment.

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

> Given how large this is, would it be reasonable to split this up a bit more?
>
> What I might do if this were my patch: get a review of the API change + the manual changes in one patch (assuming there aren't many manual changes), then land the remaining mechanical changes in chunks, perhaps vaguely by component, likely using post-commit review. The benefit of committing by chunks is that if there is some problem that comes up (even mechanical changes fail) there's more granularity for bisection (and less churn on revert). WDYT?

That sounds like a good plan to me, thanks for the suggestion!

> Also: is there a reference to this API in the ProgrammersManual? (I honestly don't know, but if there is, please be sure to update it as well.)

I don't think so, at least grepping for `_lower` didn't hit anything in llvm/docs (and manually browsing it now, the section on StringRef doesn't mention those methods).



================
Comment at: llvm/include/llvm/ADT/StringRef.h:192
 
-    /// equals_lower - Check for string equality, ignoring case.
+    /// equals_insensitive - Check for string equality, ignoring case.
     LLVM_NODISCARD
----------------
MaskRay wrote:
> https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
> 
> "Don’t duplicate function or class name at the beginning of the comment."
Thanks, I'll fix those for the methods I'm renaming here, but I'll refrain from touching the other methods in this patch.


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