kbobyrev 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>( ---------------- aaron.ballman wrote: > lebedev.ri wrote: > > 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? > > "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? > > More information doesn't always equate into more understanding, especially > when that information causes a distraction. For instance, you could argue > that the type of the declared variables is also information we already have, > but what purpose would it serve to tell it to the user? > > Can you give an example where the specific number of declarations involved > would help you to correct the diagnostic? I can't come up with one, so it > feels to me like having the count is more of a distraction; especially given > that there's no configurable threshold for "now you have too many > declarations". I'd feel differently if there was a config option, because > then the count is truly useful to know. Oh, but that's different: "This translation unit has an error. Can not continue" does not provide enough information for users to fix the issue, pointing out that there are *multiple* declarations per statement is definitely enough. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51949 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
