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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits