[clang-tools-extra] fff59f4 - [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 12 12:52:16 PST 2022
Author: Richard
Date: 2022-01-12T13:51:50-07:00
New Revision: fff59f48173dc2f2a3ce867fa0c7836b23214110
URL: https://github.com/llvm/llvm-project/commit/fff59f48173dc2f2a3ce867fa0c7836b23214110
DIFF: https://github.com/llvm/llvm-project/commit/fff59f48173dc2f2a3ce867fa0c7836b23214110.diff
LOG: [clang-tidy] Improve modernize-redundant-void-arg to recognize macro uses
Sometimes a macro invocation will look like an argument list
declaration. Improve the check to detect this situation and not
try to modify the macro invocation.
Thanks to Nathan James for the fix.
- Ignore implicit typedefs (e.g. compiler builtins)
- Improve lexing state machine to locate void argument tokens
- Add additional return_t() macro tests
- clang-format control in the test case file
- remove braces around single statements per LLVM style guide
Fixes #43791
Differential Revision: https://reviews.llvm.org/D116425
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp b/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
index 3214ae84b74de..4f791d404d9da 100644
--- a/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
@@ -20,15 +20,12 @@ namespace {
// Determine if the given QualType is a nullary function or pointer to same.
bool protoTypeHasNoParms(QualType QT) {
- if (const auto *PT = QT->getAs<PointerType>()) {
+ if (const auto *PT = QT->getAs<PointerType>())
QT = PT->getPointeeType();
- }
- if (auto *MPT = QT->getAs<MemberPointerType>()) {
+ if (auto *MPT = QT->getAs<MemberPointerType>())
QT = MPT->getPointeeType();
- }
- if (const auto *FP = QT->getAs<FunctionProtoType>()) {
+ if (const auto *FP = QT->getAs<FunctionProtoType>())
return FP->getNumParams() == 0;
- }
return false;
}
@@ -48,7 +45,8 @@ void RedundantVoidArgCheck::registerMatchers(MatchFinder *Finder) {
unless(isInstantiated()), unless(isExternC()))
.bind(FunctionId),
this);
- Finder->addMatcher(typedefNameDecl().bind(TypedefId), this);
+ Finder->addMatcher(typedefNameDecl(unless(isImplicit())).bind(TypedefId),
+ this);
auto ParenFunctionType = parenType(innerType(functionType()));
auto PointerToFunctionType = pointee(ParenFunctionType);
auto FunctionOrMemberPointer =
@@ -72,34 +70,36 @@ void RedundantVoidArgCheck::registerMatchers(MatchFinder *Finder) {
void RedundantVoidArgCheck::check(const MatchFinder::MatchResult &Result) {
const BoundNodes &Nodes = Result.Nodes;
- if (const auto *Function = Nodes.getNodeAs<FunctionDecl>(FunctionId)) {
+ if (const auto *Function = Nodes.getNodeAs<FunctionDecl>(FunctionId))
processFunctionDecl(Result, Function);
- } else if (const auto *TypedefName =
- Nodes.getNodeAs<TypedefNameDecl>(TypedefId)) {
+ else if (const auto *TypedefName =
+ Nodes.getNodeAs<TypedefNameDecl>(TypedefId))
processTypedefNameDecl(Result, TypedefName);
- } else if (const auto *Member = Nodes.getNodeAs<FieldDecl>(FieldId)) {
+ else if (const auto *Member = Nodes.getNodeAs<FieldDecl>(FieldId))
processFieldDecl(Result, Member);
- } else if (const auto *Var = Nodes.getNodeAs<VarDecl>(VarId)) {
+ else if (const auto *Var = Nodes.getNodeAs<VarDecl>(VarId))
processVarDecl(Result, Var);
- } else if (const auto *NamedCast =
- Nodes.getNodeAs<CXXNamedCastExpr>(NamedCastId)) {
+ else if (const auto *NamedCast =
+ Nodes.getNodeAs<CXXNamedCastExpr>(NamedCastId))
processNamedCastExpr(Result, NamedCast);
- } else if (const auto *CStyleCast =
- Nodes.getNodeAs<CStyleCastExpr>(CStyleCastId)) {
+ else if (const auto *CStyleCast =
+ Nodes.getNodeAs<CStyleCastExpr>(CStyleCastId))
processExplicitCastExpr(Result, CStyleCast);
- } else if (const auto *ExplicitCast =
- Nodes.getNodeAs<ExplicitCastExpr>(ExplicitCastId)) {
+ else if (const auto *ExplicitCast =
+ Nodes.getNodeAs<ExplicitCastExpr>(ExplicitCastId))
processExplicitCastExpr(Result, ExplicitCast);
- } else if (const auto *Lambda = Nodes.getNodeAs<LambdaExpr>(LambdaId)) {
+ else if (const auto *Lambda = Nodes.getNodeAs<LambdaExpr>(LambdaId))
processLambdaExpr(Result, Lambda);
- }
}
void RedundantVoidArgCheck::processFunctionDecl(
const MatchFinder::MatchResult &Result, const FunctionDecl *Function) {
+ const auto *Method = dyn_cast<CXXMethodDecl>(Function);
+ SourceLocation Start = Method && Method->getParent()->isLambda()
+ ? Method->getBeginLoc()
+ : Function->getLocation();
+ SourceLocation End = Function->getEndLoc();
if (Function->isThisDeclarationADefinition()) {
- SourceLocation Start = Function->getBeginLoc();
- SourceLocation End = Function->getEndLoc();
if (const Stmt *Body = Function->getBody()) {
End = Body->getBeginLoc();
if (End.isMacroID() &&
@@ -109,10 +109,20 @@ void RedundantVoidArgCheck::processFunctionDecl(
}
removeVoidArgumentTokens(Result, SourceRange(Start, End),
"function definition");
- } else {
- removeVoidArgumentTokens(Result, Function->getSourceRange(),
+ } else
+ removeVoidArgumentTokens(Result, SourceRange(Start, End),
"function declaration");
- }
+}
+
+bool isMacroIdentifier(const IdentifierTable &Idents, const Token &ProtoToken) {
+ if (!ProtoToken.is(tok::TokenKind::raw_identifier))
+ return false;
+
+ IdentifierTable::iterator It = Idents.find(ProtoToken.getRawIdentifier());
+ if (It == Idents.end())
+ return false;
+
+ return It->second->hadMacroDefinition();
}
void RedundantVoidArgCheck::removeVoidArgumentTokens(
@@ -127,49 +137,86 @@ void RedundantVoidArgCheck::removeVoidArgumentTokens(
.str();
Lexer PrototypeLexer(CharRange.getBegin(), getLangOpts(), DeclText.data(),
DeclText.data(), DeclText.data() + DeclText.size());
- enum TokenState {
- NothingYet,
- SawLeftParen,
- SawVoid,
+ enum class TokenState {
+ Start,
+ MacroId,
+ MacroLeftParen,
+ MacroArguments,
+ LeftParen,
+ Void,
};
- TokenState State = NothingYet;
+ TokenState State = TokenState::Start;
Token VoidToken;
Token ProtoToken;
+ const IdentifierTable &Idents = Result.Context->Idents;
+ int MacroLevel = 0;
std::string Diagnostic =
("redundant void argument list in " + GrammarLocation).str();
while (!PrototypeLexer.LexFromRawLexer(ProtoToken)) {
switch (State) {
- case NothingYet:
- if (ProtoToken.is(tok::TokenKind::l_paren)) {
- State = SawLeftParen;
- }
+ case TokenState::Start:
+ if (ProtoToken.is(tok::TokenKind::l_paren))
+ State = TokenState::LeftParen;
+ else if (isMacroIdentifier(Idents, ProtoToken))
+ State = TokenState::MacroId;
+ break;
+ case TokenState::MacroId:
+ if (ProtoToken.is(tok::TokenKind::l_paren))
+ State = TokenState::MacroLeftParen;
+ else
+ State = TokenState::Start;
+ break;
+ case TokenState::MacroLeftParen:
+ ++MacroLevel;
+ if (ProtoToken.is(tok::TokenKind::raw_identifier)) {
+ if (isMacroIdentifier(Idents, ProtoToken))
+ State = TokenState::MacroId;
+ else
+ State = TokenState::MacroArguments;
+ } else if (ProtoToken.is(tok::TokenKind::r_paren)) {
+ --MacroLevel;
+ if (MacroLevel == 0)
+ State = TokenState::Start;
+ else
+ State = TokenState::MacroId;
+ } else
+ State = TokenState::MacroArguments;
break;
- case SawLeftParen:
- if (ProtoToken.is(tok::TokenKind::raw_identifier) &&
- ProtoToken.getRawIdentifier() == "void") {
- State = SawVoid;
- VoidToken = ProtoToken;
- } else if (ProtoToken.is(tok::TokenKind::l_paren)) {
- State = SawLeftParen;
- } else {
- State = NothingYet;
+ case TokenState::MacroArguments:
+ if (isMacroIdentifier(Idents, ProtoToken))
+ State = TokenState::MacroLeftParen;
+ else if (ProtoToken.is(tok::TokenKind::r_paren)) {
+ --MacroLevel;
+ if (MacroLevel == 0)
+ State = TokenState::Start;
}
break;
- case SawVoid:
- State = NothingYet;
- if (ProtoToken.is(tok::TokenKind::r_paren)) {
+ case TokenState::LeftParen:
+ if (ProtoToken.is(tok::TokenKind::raw_identifier)) {
+ if (isMacroIdentifier(Idents, ProtoToken))
+ State = TokenState::MacroId;
+ else if (ProtoToken.getRawIdentifier() == "void") {
+ State = TokenState::Void;
+ VoidToken = ProtoToken;
+ }
+ } else if (ProtoToken.is(tok::TokenKind::l_paren))
+ State = TokenState::LeftParen;
+ else
+ State = TokenState::Start;
+ break;
+ case TokenState::Void:
+ State = TokenState::Start;
+ if (ProtoToken.is(tok::TokenKind::r_paren))
removeVoidToken(VoidToken, Diagnostic);
- } else if (ProtoToken.is(tok::TokenKind::l_paren)) {
- State = SawLeftParen;
- }
+ else if (ProtoToken.is(tok::TokenKind::l_paren))
+ State = TokenState::LeftParen;
break;
}
}
- if (State == SawVoid && ProtoToken.is(tok::TokenKind::r_paren)) {
+ if (State == TokenState::Void && ProtoToken.is(tok::TokenKind::r_paren))
removeVoidToken(VoidToken, Diagnostic);
- }
}
void RedundantVoidArgCheck::removeVoidToken(Token VoidToken,
@@ -181,19 +228,17 @@ void RedundantVoidArgCheck::removeVoidToken(Token VoidToken,
void RedundantVoidArgCheck::processTypedefNameDecl(
const MatchFinder::MatchResult &Result,
const TypedefNameDecl *TypedefName) {
- if (protoTypeHasNoParms(TypedefName->getUnderlyingType())) {
+ if (protoTypeHasNoParms(TypedefName->getUnderlyingType()))
removeVoidArgumentTokens(Result, TypedefName->getSourceRange(),
isa<TypedefDecl>(TypedefName) ? "typedef"
: "type alias");
- }
}
void RedundantVoidArgCheck::processFieldDecl(
const MatchFinder::MatchResult &Result, const FieldDecl *Member) {
- if (protoTypeHasNoParms(Member->getType())) {
+ if (protoTypeHasNoParms(Member->getType()))
removeVoidArgumentTokens(Result, Member->getSourceRange(),
"field declaration");
- }
}
void RedundantVoidArgCheck::processVarDecl(
@@ -206,30 +251,27 @@ void RedundantVoidArgCheck::processVarDecl(
.getLocWithOffset(-1);
removeVoidArgumentTokens(Result, SourceRange(Begin, InitStart),
"variable declaration with initializer");
- } else {
+ } else
removeVoidArgumentTokens(Result, Var->getSourceRange(),
"variable declaration");
- }
}
}
void RedundantVoidArgCheck::processNamedCastExpr(
const MatchFinder::MatchResult &Result, const CXXNamedCastExpr *NamedCast) {
- if (protoTypeHasNoParms(NamedCast->getTypeAsWritten())) {
+ if (protoTypeHasNoParms(NamedCast->getTypeAsWritten()))
removeVoidArgumentTokens(
Result,
NamedCast->getTypeInfoAsWritten()->getTypeLoc().getSourceRange(),
"named cast");
- }
}
void RedundantVoidArgCheck::processExplicitCastExpr(
const MatchFinder::MatchResult &Result,
const ExplicitCastExpr *ExplicitCast) {
- if (protoTypeHasNoParms(ExplicitCast->getTypeAsWritten())) {
+ if (protoTypeHasNoParms(ExplicitCast->getTypeAsWritten()))
removeVoidArgumentTokens(Result, ExplicitCast->getSourceRange(),
"cast expression");
- }
}
void RedundantVoidArgCheck::processLambdaExpr(
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
index 15348f4b05521..89bf7f04f5576 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
@@ -199,6 +199,7 @@ typedef void (function_ptr2)
// CHECK-FIXES-NEXT: {{^ \);$}}
// intentionally not LLVM style to check preservation of whitespace
+// clang-format off
typedef
void
(
@@ -240,7 +241,7 @@ void
// CHECK-FIXES-NOT: {{[^ ]}}
// CHECK-FIXES: {{^\)$}}
// CHECK-FIXES-NEXT: {{^;$}}
-
+// clang-format on
void (gronk::*p1)(void);
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: {{.*}} in variable declaration
@@ -556,3 +557,38 @@ void f_testTemplate() {
S_3<int>();
g_3<int>();
}
+
+#define return_t(T) T
+extern return_t(void) func(void);
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: redundant void argument list in function declaration
+// CHECK-FIXES: extern return_t(void) func();
+
+return_t(void) func(void) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: redundant void argument list in function definition
+ // CHECK-FIXES: return_t(void) func() {
+ int a;
+ (void)a;
+}
+
+extern return_t(void) func(return_t(void) (*fp)(void));
+// CHECK-MESSAGES: :[[@LINE-1]]:49: warning: redundant void argument list in variable declaration
+// CHECK-FIXES: extern return_t(void) func(return_t(void) (*fp)());
+
+return_t(void) func(return_t(void) (*fp)(void)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: redundant void argument list in variable declaration
+ // CHECK-FIXES: return_t(void) func(return_t(void) (*fp)()) {
+ int a;
+ (void)a;
+}
+
+extern return_t(return_t(void)) func2(return_t(return_t(void)) (*fp)(void));
+// CHECK-MESSAGES: :[[@LINE-1]]:70: warning: redundant void argument list in variable declaration
+// CHECK-FIXES: extern return_t(return_t(void)) func2(return_t(return_t(void)) (*fp)());
+
+return_t(return_t(void)) func2(return_t(return_t(void)) (*fp)(void)) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:63: warning: redundant void argument list in variable declaration
+ // CHECK-FIXES: return_t(return_t(void)) func2(return_t(return_t(void)) (*fp)()) {
+ int a;
+ (void)a;
+}
+#undef return_t
More information about the cfe-commits
mailing list