[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Btw, I'm in WG14 (C) standards meetings all week this week and on vacation next week, so my availability for code reviews is pretty limited for the next while. So despite my comments here, if you don't get a response back from me in a timely manner, don't hold up the review on my account if someone else approves (I have no blocking concerns with this patch).



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:50
 struct ClangTidyStats {
-  ClangTidyStats()
-      : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0),
-        ErrorsIgnoredNonUserCode(0), ErrorsIgnoredLineFilter(0) {}
+  int ErrorsDisplayed{0};
+  int ErrorsIgnoredCheckFilter{0};
----------------
carlosgalvezp wrote:
> whisperity wrote:
> > carlosgalvezp wrote:
> > > salman-javed-nz wrote:
> > > > What's the prevalent style for class member initialization? `=` or `{}`?
> > > > cppcoreguidelines-prefer-member-initializer defaults to `{}` but I have seen both types in the code.
> > > I tried to find this info in the LLVM coding guidelines but didn't find anything, so I assume it's maybe up to developers' discretion.
> > > 
> > > I prefer using braced initialization, since it prevents implicit conversions:
> > > https://godbolt.org/z/4sP4rGsrY
> > > 
> > > More strict guidelines like Autosar enforce this. Also CppCoreGuidelines prefer that style as you point out.
> > I think this is such a new and within LLVM relatively unused feature (remember we are still pegged to C++14...) that we do not have a consensus on style, and perhaps warrants discussing it on the mailing list.
> Just to clarify - this is C++11/14 syntax, nothing really fancy. I'm happy to change to " = 0" if people want, I don't have a strong opinion here.
I've seen both styles but I believe I've seen `=` used more often for scalar initialization and `{}` used more often for ctor initialization. e.g.,
```
int a = 0; // More often
int a{0}; // Less often

ClassFoo c = {1, 2, "three"}; // Less often
ClassFoo c{1, 2, "three"}; // More often
```


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:61-64
-  unsigned errorsIgnored() const {
+  int errorsIgnored() const {
     return ErrorsIgnoredNOLINT + ErrorsIgnoredCheckFilter +
            ErrorsIgnoredNonUserCode + ErrorsIgnoredLineFilter;
   }
----------------
The type changes overflow here from well-defined behavior to undefined behavior. I don't think that's a serious concern (we have error limits already), but in general, I don't think we should be changing types like this without stronger rationale. This particular bit feels like churn without benefit -- what do we gain by introducing the possibility for UB here? (I'm not strongly opposed, but I get worried when well-defined code turns into potentially undefined code in the name of an NFC cleanup.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113847