aaron.ballman added a comment.

In https://reviews.llvm.org/D52771#1263404, @lebedev.ri wrote:

> FIXME: `IgnoreClassesWithAllMemberVariablesBeingPublic` needs to be somehow 
> enabled for cppcoreguidelines check.
>  I don't know if it is possible, and how.


IIRC, the idea is to override `getModuleOptions()` and specify different 
default configuration options for the alias.



================
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();
----------------
lebedev.ri wrote:
> 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()`.
Correct - it only works for automatically quoting `NamedDecl` objects


================
Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:90
+  diag(Field->getLocation(), "member variable %0 of %1 %2 has %3 visibility")
+      << Field << Record->getKindName() << Record << Field->getAccess();
+}
----------------
I cannot think of a situation in which the class name is actually useful 
information to present to the user, so I'd recommend dropping it (and the kind 
name as well). However, if you have a specific case in mind where the class 
name is useful for understanding how to fix the issue, I'd love to see it.


================
Comment at: 
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:9
+
+Finds classes that not only contain the data (non-static member variables),
+but also have logic (non-static member functions), and diagnoses all member
----------------
I'd reword this a bit to:
```
Finds classes that contain non-static data members in addition to non-static 
member functions and diagnose all data members declared with a non-``public`` 
access specifier. The data members should be declared as ``private`` and 
accessed through member functions instead of exposed to derived classes or 
class consumers.
```


================
Comment at: 
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:14-15
+
+Optionally, classes with all member variables being ``public`` could be
+ignored, and optionally all ``public`` member variables could be ignored.
+
----------------
I'd drop this paragraph entirely.


================
Comment at: 
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst:28
+   Allows to ignore (not diagnose) **all** the member variables with ``public``
+   visibility scope.
----------------
with public visibility scope -> declared with a public access specifier


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

Reply via email to