[PATCH] D56966: [clang-tidy] misc-non-private-member-variables-in-classes: ignore implicit methods

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 19 13:15:12 PST 2019


lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.

Some nits.



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


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


================
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.
----------------
Comment needs updating?


================
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``
----------------
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.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56966





More information about the cfe-commits mailing list