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

[Richard]

================
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);
----------------
sbenza wrote:
> 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.
So... I need to write an expression that matches not just a pointer to function but a pointer to function with zero arguments?  I'll play with clang-query and see what I can figure out.

I managed to get some narrowing for everything but typedefDecl; I couldn't find any narrowing matchers for it.

================
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.");
----------------
sbenza wrote:
> Any reason all of these convert the StringRef into a std::string?
> Couldn't they work with the StringRef directly?
I tried switching it to a StringRef, but when I tried to combine it with other text to build a replacement for diag(), it got turned into Twine and then failed to match any signature for diag().  I'm open to suggestions for what you'd like to see instead.

================
Comment at: clang-tidy/readability/RemoveVoidArg.h:26
@@ +25,3 @@
+///   int foo(void); ==> int foo();
+class RemoveVoidArg : public ClangTidyCheck
+{
----------------
alexfh wrote:
> Formatting is off in many places. Could you clang-format the code so I don't need to point to every nit?
I just assumed a final pass through clang-format before being committed; I haven't gone through CLion trying to make it follow clang's/llvm's indent rules.

================
Comment at: clang-tidy/readability/RemoveVoidArg.h:38
@@ +37,3 @@
+private:
+    void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult &Result, FunctionDecl const *const Function);
+
----------------
sbenza wrote:
> 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.
They're members because ultimately on detection they ultimately call diag which is a member function.  Otherwise, I would have made them free functions.

http://reviews.llvm.org/D7639

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