[libcxx-commits] [PATCH] D129051: [libc++] Make parameter names consistent and enforce the naming style using readability-identifier-naming

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__format/formatter.h:76
+_LIBCPP_HIDE_FROM_ABI constexpr char __hex_to_upper(char __c) {
+  switch (__c) {
   case 'a':
----------------
Mordante wrote:
> Nice catch! Can you undo this change? It will be removed in D128846.
I'd rather keep this change in, since clang-tidy would fail otherwise or we couldn't enforce the proper naming.


================
Comment at: libcxx/include/regex:2971
 
-    bool __test_back_ref(_CharT c);
+    bool __test_back_ref(_CharT);
 
----------------
Mordante wrote:
> I have a preference to use `__c`, without a named argument it almost sounds like the argument is unused.
My general approach is that if the name isn't useful to the reader in the declaration just don't add one. Saying "this char is really a character" isn't exactly useful IMO. I've also removed some `path p` in filesystem somewhere. In general I'm assuming that arguments are used, especially in implementation-detail functions. Our code-base isn't so large that it would be impractical to remove an argument everywhere in a single patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129051