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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for the cleanup! Some small comments.
Did you use clang-tidy to find these? If so maybe add an attribution in the commit message.

Since the CI fails I would like to have a look at the next version.



================
Comment at: libcxx/include/__format/formatter.h:76
+_LIBCPP_HIDE_FROM_ABI constexpr char __hex_to_upper(char __c) {
+  switch (__c) {
   case 'a':
----------------
Nice catch! Can you undo this change? It will be removed in D128846.


================
Comment at: libcxx/include/__format/formatter_output.h:36
 
-_LIBCPP_HIDE_FROM_ABI constexpr char __hex_to_upper(char c) {
-  switch (c) {
+_LIBCPP_HIDE_FROM_ABI constexpr char __hex_to_upper(char __c) {
+  switch (__c) {
----------------
This is the duplicate and this version will be the only one after D128846.


================
Comment at: libcxx/include/charconv:619
 inline _LIBCPP_HIDE_FROM_ABI from_chars_result
-__from_chars_atoi(const char* __first, const char* __last, _Tp& __value)
+__from_chars_atoi(const char* __f, const char* __l, _Tp& __value)
 {
----------------
I don't feel this aids the readability, so I would prefer to keep it as-is.
Instead rename `_Last` to `__l`.
In general it's good to use longer names for the larger scope.


================
Comment at: libcxx/include/charconv:654
 __from_chars_integral(const char* __first, const char* __last, _Tp& __value,
-                      int __base)
+                      int __b)
 {
----------------
Likewise please keep this `__base`.


================
Comment at: libcxx/include/regex:2971
 
-    bool __test_back_ref(_CharT c);
+    bool __test_back_ref(_CharT);
 
----------------
I have a preference to use `__c`, without a named argument it almost sounds like the argument is unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129051