Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D7639#275504, @sbenza wrote: > Just fyi, I am looking at this diff. It is very large with a lot of rounds of > comments and I didn't have the context. > I don't know if I should giving comments at this point of the change, but > here it is. > Have you considered matching on typeLoc() instead of having a large list of > different cases? > For example, if you match `typeLoc(loc(functionType()))` it will match all > the places where a function type is mentioned, including things like > `static_cast`, variable declarations, lambda return type declarations, > etc. Might help remove redundancy in the check. That occurred to me and I did an experiment and it didn't work out. I forget the exact details now as it was months ago and this review has been sitting here languishing with a correct implementation as-is. I really just want to get this committed and make incremental improvement, instead of re-evaluating the entire thing from scratch at this time. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
sbenza accepted this revision. sbenza added a comment. This revision is now accepted and ready to land. Just wanted to know it was considered, and it was. It looks good to me then. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
LegalizeAdulthood added a comment. I mean, look at this review. I created it on Feb 13th 2015. It's been getting ground through the review process for 8 months. Why can't we move forward? http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
This revision was automatically updated to reflect the committed changes.
Closed by commit rL251475: Add modernize-redundant-void-arg check to clang-tidy
(authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D7639?vs=37717=38617#toc
Repository:
rL LLVM
http://reviews.llvm.org/D7639
Files:
clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.h
clang-tools-extra/trunk/test/clang-tidy/modernize-redundant-void-arg.c
clang-tools-extra/trunk/test/clang-tidy/modernize-redundant-void-arg.cpp
Index: clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.h
+++ clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.h
@@ -0,0 +1,76 @@
+//===--- RedundantVoidArgCheck.h - clang-tidy *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANT_VOID_ARG_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANT_VOID_ARG_CHECK_H
+
+#include "../ClangTidy.h"
+#include "clang/Lex/Token.h"
+
+#include
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// \brief Find and remove redundant void argument lists.
+///
+/// Examples:
+/// `int f(void);`becomes `int f();`
+/// `int (*f(void))(void);` becomes `int (*f())();`
+/// `typedef int (*f_t(void))(void);` becomes `typedef int (*f_t())();`
+/// `void (C::*p)(void);` becomes `void (C::*p)();`
+/// `C::C(void) {}` becomes `C::C() {}`
+/// `C::~C(void) {}` becomes `C::~C() {}`
+///
+class RedundantVoidArgCheck : public ClangTidyCheck {
+public:
+ RedundantVoidArgCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+ void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult ,
+ const FunctionDecl *Function);
+
+ void processTypedefDecl(const ast_matchers::MatchFinder::MatchResult ,
+ const TypedefDecl *Typedef);
+
+ void processFieldDecl(const ast_matchers::MatchFinder::MatchResult ,
+const FieldDecl *Member);
+
+ void processVarDecl(const ast_matchers::MatchFinder::MatchResult ,
+ const VarDecl *Var);
+
+ void
+ processNamedCastExpr(const ast_matchers::MatchFinder::MatchResult ,
+ const CXXNamedCastExpr *NamedCast);
+
+ void
+ processExplicitCastExpr(const ast_matchers::MatchFinder::MatchResult ,
+ const ExplicitCastExpr *ExplicitCast);
+
+ void processLambdaExpr(const ast_matchers::MatchFinder::MatchResult ,
+ const LambdaExpr *Lambda);
+
+ void
+ removeVoidArgumentTokens(const ast_matchers::MatchFinder::MatchResult ,
+ SourceRange Range, StringRef GrammarLocation);
+
+ void removeVoidToken(Token VoidToken, StringRef Diagnostic);
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANT_VOID_ARG_CHECK_H
Index: clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/RedundantVoidArgCheck.cpp
@@ -0,0 +1,254 @@
+//===- RedundantVoidArgCheck.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RedundantVoidArgCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+
+namespace {
+
+// Determine if the given QualType is a nullary function or pointer to same.
+bool protoTypeHasNoParms(QualType QT) {
+ if (auto PT = QT->getAs()) {
+QT = PT->getPointeeType();
+ }
+ if (auto *MPT = QT->getAs()) {
+QT = MPT->getPointeeType();
+ }
+ if (auto FP = QT->getAs()) {
+return FP->getNumParams() == 0;
+
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
alexfh accepted this revision. alexfh added a comment. In http://reviews.llvm.org/D7639#276641, @LegalizeAdulthood wrote: > I mean, look at this review. I created it on Feb 13th 2015. It's been > getting ground through the review process for 8 months. Why can't we move > forward? Yes, I know the long reviews may be annoying. And I apologize for missing the latest updates on this patch from June. Please feel free to ping the reviews when there are no updates on them for a few days. There are still a few comments on this patch that were not completely addressed, but it seems fine to submit the patch at this stage and address the concerns after the patch is in. I'll commit the patch for you. Comment at: clang-tidy/readability/RedundantVoidArgCheck.cpp:51 @@ +50,3 @@ + unless(isImplicit()), + unless(isExternC())).bind(FunctionId), + this); I'm pretty sure the lexer is able to work on header files, if used correctly. If you need help figuring out what's wrong, please provide more information: which assert fails, call stack, and a reduced test case where the assert fails. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
sbenza added a comment. Just fyi, I am looking at this diff. It is very large with a lot of rounds of comments and I didn't have the context. I don't know if I should giving comments at this point of the change, but here it is. Have you considered matching on typeLoc() instead of having a large list of different cases? For example, if you match `typeLoc(loc(functionType()))` it will match all the places where a function type is mentioned, including things like `static_cast`, variable declarations, lambda return type declarations, etc. Might help remove redundancy in the check. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
LegalizeAdulthood marked 17 inline comments as done. LegalizeAdulthood added a comment. Can we get this committed? I've addressed all comments. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
sbenza added a comment. I'm sorry. This change fell through the cracks. I'll take a look today. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
LegalizeAdulthood added a comment. In http://reviews.llvm.org/D7639#266332, @Eugene.Zelenko wrote: > What is preventing to add this check to Clang-tidy? Just found another piece > of fresh C++ code in LLDB with (void) as argument list... To be honest, I don't know. This review had taken SO long to get approved. At this point, the code is correct and I'd really like to get it committed and available for use instead of fussing with it any longer. http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
LegalizeAdulthood updated this revision to Diff 37717.
http://reviews.llvm.org/D7639
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantVoidArgCheck.cpp
clang-tidy/readability/RedundantVoidArgCheck.h
test/clang-tidy/readability-redundant-void-arg.c
test/clang-tidy/readability-redundant-void-arg.cpp
Index: test/clang-tidy/readability-redundant-void-arg.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-void-arg.cpp
@@ -0,0 +1,419 @@
+// RUN: %python %S/check_clang_tidy.py %s readability-redundant-void-arg %t
+
+#include
+
+int foo();
+
+void bar();
+
+void bar2();
+
+extern "C" void ecfoo(void);
+
+extern "C" void ecfoo(void) {
+}
+
+extern int i;
+
+int j = 1;
+
+int foo(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant void argument list in function definition [readability-redundant-void-arg]
+// CHECK-FIXES: {{^}}int foo() {{{$}}
+return 0;
+}
+
+typedef unsigned int my_uint;
+
+typedef void my_void;
+
+// A function taking void and returning a pointer to function taking void
+// and returning int.
+int (*returns_fn_void_int(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function declaration
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: {{.*}} in function declaration
+// CHECK-FIXES: {{^}}int (*returns_fn_void_int())();{{$}}
+
+typedef int (*returns_fn_void_int_t(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-2]]:44: warning: {{.*}} in typedef
+// CHECK-FIXES: {{^}}typedef int (*returns_fn_void_int_t())();{{$}}
+
+int (*returns_fn_void_int(void))(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} in function definition
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}int (*returns_fn_void_int())() {{{$}}
+ return nullptr;
+}
+
+// A function taking void and returning a pointer to a function taking void
+// and returning a pointer to a function taking void and returning void.
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: {{.*}} in function declaration
+// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: {{.*}} in function declaration
+// CHECK-MESSAGES: :[[@LINE-3]]:56: warning: {{.*}} in function declaration
+// CHECK-FIXES: {{^}}void (*(*returns_fn_returns_fn_void_void())())();{{$}}
+
+typedef void (*(*returns_fn_returns_fn_void_void_t(void))(void))(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:52: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-2]]:59: warning: {{.*}} in typedef
+// CHECK-MESSAGES: :[[@LINE-3]]:66: warning: {{.*}} in typedef
+// CHECK-FIXES: {{^}}typedef void (*(*returns_fn_returns_fn_void_void_t())())();{{$}}
+
+void (*(*returns_fn_returns_fn_void_void(void))(void))(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:42: warning: {{.*}} in function definition
+// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: {{.*}} in function definition
+// CHECK-MESSAGES: :[[@LINE-3]]:56: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}void (*(*returns_fn_returns_fn_void_void())())() {{{$}}
+return nullptr;
+}
+
+void bar(void) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: {{.*}} in function definition
+// CHECK-FIXES: {{^}}void bar() {{{$}}
+}
+
+void op_fn(int i) {
+}
+
+class gronk {
+public:
+ gronk();
+ ~gronk();
+
+void foo();
+void bar();
+void bar2
+();
+void operation(int i) { }
+
+private:
+int m_i;
+int *m_pi;
+float m_f;
+float *m_pf;
+double m_d;
+double *m_pd;
+
+void (*f1)(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*}} in field declaration
+// CHECK-FIXES: {{^}}void (*f1)();{{$}}
+
+ void (*op)(int i);
+
+ void (gronk::*p1)(void);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in field declaration
+ // CHECK-FIXES: {{^ }}void (gronk::*p1)();{{$}}
+
+ int (gronk::*p_mi);
+
+ void (gronk::*p2)(int);
+
+ void (*(*returns_fn_returns_fn_void_void(void))(void))(void);
+ // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: {{.*}} in function declaration
+ // CHECK-MESSAGES: :[[@LINE-2]]:51: warning: {{.*}} in function declaration
+ // CHECK-MESSAGES: :[[@LINE-3]]:58: warning: {{.*}} in function declaration
+ // CHECK-FIXES: {{^}} void (*(*returns_fn_returns_fn_void_void())())();{{$}}
+
+ void (*(*(gronk::*returns_fn_returns_fn_void_void_mem)(void))(void))(void);
+ // CHECK-MESSAGES: :[[@LINE-1]]:58: warning: {{.*}} in field declaration
+ // CHECK-MESSAGES: :[[@LINE-2]]:65: warning: {{.*}} in field declaration
+ // CHECK-MESSAGES: :[[@LINE-3]]:72: warning: {{.*}} in field declaration
+ // CHECK-FIXES: {{^}} void (*(*(gronk::*returns_fn_returns_fn_void_void_mem)())())();{{$}}
+};
+
+int i;
+int *pi;
+void *pv = (void *) pi;
+float f;
+float *fi;
+double d;
+double *pd;
+
+void (*f1)(void);
+// CHECK-MESSAGES:
Re: [PATCH] D7639: Add readability-redundant-void-arg check to clang-tidy
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. What is preventing to add this check to Clang-tidy? Just found another piece of fresh C++ code in LLDB with (void) as argument list... http://reviews.llvm.org/D7639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
