lebedev.ri added inline comments.
================ Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:56 + auto RecordIsInteresting = + allOf(anyOf(isStruct(), isClass()), hasMethods(), hasNonStaticMethod(), + IgnoreClassesWithAllMemberVariablesBeingPublic ---------------- aaron.ballman wrote: > Neither the C++ Core Guidelines nor HIC++ carve out an exception for union > types. I think you should talk to the guideline authors to see what they > think. Perhaps first try to diagnose unions the same as structs and classes > and see how the results look to you -- if they look bad, you can incorporate > those as examples when discussing with the authors. As discussed, let's just drop HICPP alias, and CPPCG only talks about struct/class, not union. https://github.com/isocpp/CppCoreGuidelines/issues/1281 ================ Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:82 + "member variable '%0' of %1 '%2' has %3 visibility") + << Field->getName() << Record->getKindName() << Record->getName() + << Field->getAccess(); ---------------- aaron.ballman wrote: > Drop the single quotes above and just pass in `Field` and `Record` directly > -- the diagnostic printer will do the right thing. Nice, but that does not seem to dump `getKindName()`. ================ Comment at: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp:1 +// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s misc-non-private-member-variables-in-classes %t +// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' -- ---------------- zinovy.nis wrote: > JonasToth wrote: > > lebedev.ri wrote: > > > JonasToth wrote: > > > > I would prefer multiple test-files instead of mixing them all together. > > > Hmm, no. Then the tests will have to be duplicated in N files, > > > because i really do want to see what each of these 4 configurations do on > > > the *same* input. > > > > > > It only looks ugly because the script only supports `-check-suffix=`, > > > and not `-check-suffixes=`, which would significantly cut down on > > > check-lines. > > Ok, if you prefer this. > Not an issue anymore. You can use `-check-suffixes` since now. Yay, look at this beauty :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
