[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343
+  auto Diag =
+      diag(WholeDecl->getBeginLoc(), "this statement declares %0 variables")
+      << static_cast<unsigned int>(
----------------
kbobyrev wrote:
> JonasToth wrote:
> > kbobyrev wrote:
> > > How about `multiple declarations within a single statement hurts readability`?
> > s/hurts/reduces/? hurts sound a bit weird i think.
> > 
> > Lebedev wanted the number of decls in the diagnostic, would you include it or rather now?
> "decreases" is also fine. "hurts" is probably too strong, I agree.
> 
> Up to you. Personally, I don't see any value in having the diagnostic message saying "hey, you have 2 declarations within one statement, that's really bad!" or "hey, you have 5 declarations within one statement..." - in both cases the point is that there are *multiple* declarations. I also don't think it would make debugging easier because you also check the formatting, so you already imply that the correct number of declarations was detected.
> 
> I'm interested to know what @lebedev.ri thinks.
> I'm interested to know what @lebedev.ri thinks.

"This translation unit has an error. Can not continue" is also a diagnostic message.
Why are we not ok with that one, and want compiler to be a bit more specific?

Similarly here, why just point out that this code is bad as per the check,
without giving a little bit more info, that you already have?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949