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

[Samuel Benzaquen]

================
Comment at: clang-tidy/readability/RemoveVoidArg.cpp:65
@@ +64,3 @@
+    Finder->addMatcher(typedefDecl(isExpansionInMainFile()).bind(TypedefId), this);
+    auto FunctionOrMemberPointer = anyOf(hasType(pointerType()), hasType(memberPointerType()));
+    Finder->addMatcher(fieldDecl(FunctionOrMemberPointer, isExpansionInMainFile()).bind(FieldId), this);
----------------
The callbacks are not cheap. They generate string copies and then rerun the tokenizer on them.
These matchers should be more restricted to what we are looking for to avoid running the callbacks on uninteresting nodes.

================
Comment at: clang-tidy/readability/RemoveVoidArg.cpp:78
@@ +77,3 @@
+    BoundNodes const &Nodes = Result.Nodes;
+    if (FunctionDecl const *const Function = Nodes.getNodeAs<FunctionDecl>(FunctionId)) {
+        processFunctionDecl(Result, Function);
----------------
why not auto in this one? You used it on all other cases below.

================
Comment at: clang-tidy/readability/RemoveVoidArg.cpp:174
@@ +173,3 @@
+void RemoveVoidArg::processFieldDecl(const MatchFinder::MatchResult &Result, FieldDecl const *const Member) {
+    std::string const Text = getText(Result, *Member);
+    removeVoidArgumentTokens(Result, Member->getLocStart(), Text, "Redundant void argument list in field declaration.");
----------------
Any reason all of these convert the StringRef into a std::string?
Couldn't they work with the StringRef directly?

================
Comment at: clang-tidy/readability/RemoveVoidArg.h:38
@@ +37,3 @@
+private:
+    void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result, FunctionDecl const *const Function);
+
----------------
alexfh wrote:
> Here and below: I'm against top-level `const` in function parameter types. It's just an implementation detail whether the method can modify its parameter. It doesn't make sense to pollute the interface with this.
> 
> Also, it's more common in Clang/LLVM code to put the const related to the pointed/referenced object in front:
> 
>   void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result, const FunctionDecl *Function);
Any reason why these are members?
Making them free functions in the .cpp file would avoid filling the header with these implementation details.
It would also remove the need to include Lexer.h here.

http://reviews.llvm.org/D7639

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