[libcxx-commits] [PATCH] D129386: [libc++] Make the naming of private member variables consistent and enforce it through readability-identifier-naming

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this! In general SGTM, but I would like to see the CI pass after rebasing.

In D129386#3639398 <https://reviews.llvm.org/D129386#3639398>, @philnik wrote:

> @Mordante You are currently working on chrono, right? Is there anything that interferes with one of your patches?

Correct, but only on non-member functions. I don't see any conflicts, thanks for asking.



================
Comment at: libcxx/.clang-tidy:46
+  - key:   readability-identifier-naming.PrivateMemberSuffix
+    value: _
 
----------------
Not your change but please verify whether https://reviews.llvm.org/D129503#inline-1244548 is addressed otherwise please rebase and fix that before landing this patch.


================
Comment at: libcxx/.clang-tidy:46
+  - key:   readability-identifier-naming.PrivateMemberSuffix
+    value: _
 
----------------
Mordante wrote:
> Not your change but please verify whether https://reviews.llvm.org/D129503#inline-1244548 is addressed otherwise please rebase and fix that before landing this patch.
Maybe not this patch but can we add a regex that when a member starts with `__` it should end with `_`.


================
Comment at: libcxx/include/__chrono/year.h:105
 bool year::ok() const noexcept
-{ return static_cast<int>(min()) <= __y && __y <= static_cast<int>(max()); }
+{ return static_cast<int>(min()) <= __y_ && __y_ <= static_cast<int>(max()); }
 } // namespace chrono
----------------
This will conflict with D129442, sorry I wasn't aware of this patch.


================
Comment at: libcxx/include/any:315
         void *  __ptr;
         __any_imp::_Buffer __buf;
     };
----------------
This prompted my regex question, since they weren't updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129386