[PATCH] Add readability-redundant-void-arg check to clang-tidy

[Richard]

> 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.".


In clang-tidy discussions, "false positive" means that the check incorrectly analyzed the code and that the resulting application of an automated fix would change the code to syntactically incorrect or semantically incorrect.  Under this definition, the only false positive would be a case where the fix was applied to C code, where (void) is necessary to distinguish a function with no arguments as opposed to a function with an unspecified number and type of arguments.

There are plenty of readability checks in clang-tidy that impose a certain point of view.  A good example is the LLVM namespace comment check.  Here, a style guideline is being imposed on the code.  If you don't want your code subjected to that style guideline, then you shouldn't be running that check and applying the suggested fixes to your code.  clang-tidy is not part of the compiler, nothing is forcing you to run clang-tidy and even if you use clang-tidy, nothing is forcing you to run this check.  You only get what you ask for.

> 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.


I created this check because I was working on C++ teams where people kept bringing their C habits into the C++ code.  This is purely a C-ism and people coming from C routinely bring this into their C++ code because they operate under the (erroneous IMO) opinion that "C++ is just C with classes".

> Does anybody know that some C++ developers use void because they think it's safer?


There is no issue of safety here.  In C, function prototypes are underspecified if they don't write `(void)`.  In C++, no such thing occurs.

> When you try this on real code.. what is the false positive ratio?


Again, there are no "false positives" in the clang-tidy sense.  I believe I have added sufficient C oriented test cases to cover that.  If you don't want the results of this check, or any other clang-tidy check, applied to your code, then don't ask for it.


================
Comment at: clang-tidy/readability/RedundantVoidArgCheck.cpp:50
@@ +49,3 @@
+void RedundantVoidArgCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(functionDecl(isExpansionInMainFile(), parameterCountIs(0),
+                                  unless(isImplicit()),
----------------
alexfh wrote:
> So why do we need `isExpansionInMainFile()` here and in all other matchers? ClangTidy has its own mechanism to filter warnings by location.
The lexer crashes with an assert if I don't limit it to the main file.  Why, I don't know, but that's what happens.

http://reviews.llvm.org/D7639

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/