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

[Samuel Benzaquen]

================
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.");
----------------
LegalizeAdulthood wrote:
> sbenza wrote:
> > LegalizeAdulthood wrote:
> > > sbenza wrote:
> > > > LegalizeAdulthood wrote:
> > > > > 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.
> > > > Twine::str() constructs the string.
> > > > I don't mind constructing a string when a match happens. Those are rare.
> > > > But we should avoid allocating memory when no match happens.
> > > > Right now temporary memory allocations are a considerable part of clang-tidy's runtime. We should try to minimize that.
> > > I think we are at that stage now; I do all early-out testing before constructing the replacement string.
> > You are still generating a copy of the code before re-parsing it.
> > Those will happen for anything that passes the matcher, even if they have no 'void' argument.
> > removeVoidArgumentTokens should take the code as a StringRef directly from getText(), without converting to std::string first. GrammarLocation should also be a StringRef.
> > Basically, avoid std::string until you are about to call diag().
> I'll look into having getText return the StringRef and using that.  I don't understand the fuss over GrammarLocation, however.  Before I had it as a constant string that was just passed through and then another review wanted the duplication in the message eliminated, so I pulled that out.  Now you're saying that the string joining induced by the elimination of duplication is bad.  It doesn't seem possible to satisfy both comments.
The problem with GrammarLocation is that it is declared as a 'const std::string &' and you are always passing literals.
This is what StringRef is for. It will avoid the unnecessary temporary string.
The signature would read like:

    void RemoveVoidArg::removeVoidArgumentTokens(
        const MatchFinder::MatchResult &Result, SourceLocation StartLoc,
        StringRef DeclText, StringRef GrammarLocation);

Then you can construct the Lexer with an std::string, like:

    clang::Lexer PrototypeLexer(StartLoc, Result.Context->getLangOpts(),
                                DeclText.data(), DeclText.data(),
                                DeclText.data() + DeclText.length());

http://reviews.llvm.org/D7639

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