In http://reviews.llvm.org/D7639#193527, @alexfh wrote:
> In http://reviews.llvm.org/D7639#193443, @LegalizeAdulthood wrote: > > > In http://reviews.llvm.org/D7639#192668, @danielmarjamaki wrote: > > > > > sorry but I am personally skeptic about this checker. > > > > > > why is the void removed? > > > > > > it does not cause any wrong behaviour to keep it. > > > > > > the void is not likely added there by mistake, is it? the developer > > > probably wrote it by intention and this checker thinks that the developer > > > intentions are wrong.. > > > > > > how about moving it to clang-modernize? > > > > > > It is a readability check, it isn't designed to detect "wrong" behavior. > > (void) is a C-ism and is a holdover from C-style coding. It is completely > > redundant and unnecessary in C++. If a developer wants to keep unnecessary > > and redundant tokens in their code, then they can turn this check off or > > not run it. > I know it is redundant in C++. And probably the developer knows it too? I expect that nearly 100% of the warnings will be false positives because as I said: "this checker thinks that the developer intentions are wrong.". Stylistic checkers are problematic because people have different taste so the warnings are seen by both those who wants to see it and those who don't, however in this case I have the feeling that your warning will only be seen by those who don't want to have the warning. I doubt people add void by mistake. Does anybody know that some C++ developers use void because they think it's safer? When you try this on real code.. what is the false positive ratio? > Agree. Also, we want to migrate clang-modernize transformations to clang-tidy > some day (it's an extremely low priority task though). Wrt this check, it > might fit better in a new module named "legacy". Not extremely important and > definitely not necessary for this patch. > > What's necessary, is to address my comments > (http://reviews.llvm.org/D7639?id=28202#inline-85173 and > http://reviews.llvm.org/D7639?id=28202#inline-85176). I am afraid I don't understand. I had the impression that checkers should try to warn about stuff that is not intentional. Imho a warning about intentional code that does not do anything wrong is a false positive. I can understand that some checkers could be highly useful in some projects but noisy in most projects. Can we try to separate these so they can be suppressed easily and/or must be enabled specifically? Instead of the other way around where most projects that get the warning must disable it manually. If many such checkers are added it will be a pain to disable them if they are not separated. http://reviews.llvm.org/D7639 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
