================
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);
----------------
LegalizeAdulthood wrote:
> 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.
It doesn't have to be a perfect matcher, but it should try to reject the most
common false positives early.
Just by saying that it is a functionType() you get most of the varDecls out of
the way without going into the callback.
This should be good for now.
================
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:
> > 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.
================
Comment at: clang-tidy/readability/RemoveVoidArg.h:38
@@ +37,3 @@
+private:
+ void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult
&Result, FunctionDecl const *const Function);
+
----------------
LegalizeAdulthood wrote:
> 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.
They could still be free functions, that return some data instead of taking
action directly.
But I understand now.
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
