Add a clang-tidy check in the regular CI run#3620
Conversation
| 'modernize-use-nullptr', | ||
| 'readability-braces-around-statements', | ||
| 'performance-unnecessary-value-param', | ||
| '*-non-private-member-variables-in-classes', |
There was a problem hiding this comment.
In #3619 you converted some struct to class. Was that also due to a clang-tidy rule by any chance? If yes: maybe it's worth adding that here as well. Mea culpa. 👼
There was a problem hiding this comment.
I don't think there is a clang-tidy rule about this (though if there is I'd like to enable it).
The logic here is: plain struct, if ever used, should just be a C-style struct. If it has a constructor or associated logical functions, it should be a class. Also, unless there truly is no logical consistency requirements (which is rare!), it should be a class, and any mutations performed through setters. This allows easier understanding of how/when something is modified. Eg if the member variable is just public, then it's very easy to capture it by mutable reference in a way that is not so obvious:
struct S { int foo; int something; }
int bar(S& s) {
s.something = 9;
return baz(s.foo);
}
/// in some other file far away ...
int baz(int& x) { x = 5; return 9; }
whereas if there are getters and setters than mutations are highly visible/grepable. Also in many cases no setter is required (for example the identity field in Client_PSK) in which case the absence of a setter at all makes it clear this field is never modified.
95e1a5f to
65d3589
Compare
No description provided.