================
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/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
