vmiklos marked 8 inline comments as done.
vmiklos added a comment.

I also noticed I forgot to clang-format the testcase, done now.



================
Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:21
 
 AST_MATCHER(CXXRecordDecl, hasMethods) {
+  for (const auto &Method : Node.methods()) {
----------------
lebedev.ri wrote:
> let's keep this one as-is.
Done.


================
Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:31-34
 AST_MATCHER(CXXRecordDecl, hasNonStaticMethod) {
   return hasMethod(unless(isStaticStorageClass()))
       .matches(Node, Finder, Builder);
 }
----------------
lebedev.ri wrote:
> And change this to something like
> ```
> AST_MATCHER(CXXRecordDecl, hasNonStaticNonImplicitMethod) {
>   return hasMethod(unless(anyOf(isStaticStorageClass(), isImplicit())))
>       .matches(Node, Finder, Builder);
> }
> ```
Done.


================
Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:74-76
   // We only want the records that not only contain the mutable data 
(non-static
   // member variables), but also have some logic (non-static member functions).
   // We may optionally ignore records where all the member variables are 
public.
----------------
lebedev.ri wrote:
> Comment needs updating?
Done.


================
Comment at: 
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:9
 
-Finds classes that contain non-static data members in addition to non-static
+Finds classes that contain non-static data members in addition to explicit 
non-static
 member functions and diagnose all data members declared with a non-``public``
----------------
lebedev.ri wrote:
> I don't like the wording. We are not looking for `explicit` methods,
> we are looking for user-provided methods, as opposed to compiler-provided 
> methods.
OK, used "user-provided" instead.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56966



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to