Sorry for the long delay. Here are some initial comments. It will be easier to
review once you reformat the code and place consts in front (or remove, if
needed) to make the code closer to the LLVM style.
================
Comment at: clang-tidy/readability/RemoveVoidArg.cpp:170
@@ +169,3 @@
+ std::string const Text = getText(Result, *Typedef);
+ removeVoidArgumentTokens(Result, Typedef->getLocStart(), Text, "Redundant
void argument list in typedef.");
+}
----------------
1. The "Redundant void argument list in" part is repeated everywhere. Maybe
just pass the description of the location (here - "typedef") as an argument to
`removeVoidArgumentTokens`?
2. Clang-tidy messages should use the same style as Clang diagnostics: start
with a lower-case letter, and no trailing period.
================
Comment at: clang-tidy/readability/RemoveVoidArg.h:26
@@ +25,3 @@
+/// int foo(void); ==> int foo();
+class RemoveVoidArg : public ClangTidyCheck
+{
----------------
Formatting is off in many places. Could you clang-format the code so I don't
need to point to every nit?
================
Comment at: clang-tidy/readability/RemoveVoidArg.h:38
@@ +37,3 @@
+private:
+ void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult
&Result, FunctionDecl const *const Function);
+
----------------
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);
================
Comment at: test/clang-tidy/readability-remove-void-arg.cpp:28
@@ +27,3 @@
+int (*returns_fn_void_int(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: Redundant void argument list in
function declaration. [readability-remove-void-arg]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: Redundant void argument list in
function declaration. [readability-remove-void-arg]
----------------
Repeating the whole message each time makes the test harder to read. Please
leave the whole message once and shorten all other occurrences, e.g.:
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function declaration
================
Comment at: test/clang-tidy/readability-remove-void-arg.cpp:145
@@ +144,3 @@
+// CHECK-FIXES: {{^}}typedef void (function_ptr2){{$}}
+// CHECK-FIXES-NEXT: {{^ }}({{$}}
+// CHECK-FIXES-NEXT: {{^ $}}
----------------
It would be more readable with a single regular expression:
{{^ \($}}
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
