llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Victor Chernyakin (localspook)
<details>
<summary>Changes</summary>
This refactor is not entirely NFC:
- It changes the diagnostic message. What used to be `redundant void argument
list in {function definition, lambda expression, etc.}` is now just `redundant
void argument list`. I don't think we're losing any important information
though.
- It causes the check to fire in more places than it did before. Specifically,
it now removes the 2 `void`s in this test case:
https://github.com/llvm/llvm-project/blob/01effcd82dfbd3ce880a20c335334045191f217b/clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg.cpp#L434-L442
Seeing as this check does want to remove `void` in macros, I believe this is
an improvement. And I don't believe I'm violating the intent of the test case,
because [the intent was just to ensure the check doesn't crash when
encountering it](https://reviews.llvm.org/D14204).
---
Patch is 44.67 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/173340.diff
4 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
(+33-264)
- (modified) clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.h
(-34)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg-delayed.cpp
(+3-3)
- (modified)
clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg.cpp
(+100-95)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
index 831c8565eb74d..01f6ff60b9d6e 100644
--- a/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.cpp
@@ -7,281 +7,50 @@
//===----------------------------------------------------------------------===//
#include "RedundantVoidArgCheck.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Lex/Lexer.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
namespace clang::tidy::modernize {
-// Determine if the given QualType is a nullary function or pointer to same.
-static bool protoTypeHasNoParms(QualType QT) {
- if (const auto *PT = QT->getAs<PointerType>())
- QT = PT->getPointeeType();
- if (auto *MPT = QT->getAs<MemberPointerType>())
- QT = MPT->getPointeeType();
- if (const auto *FP = QT->getAs<FunctionProtoType>())
- return FP->getNumParams() == 0;
- return false;
-}
-
-static const char FunctionId[] = "function";
-static const char TypedefId[] = "typedef";
-static const char FieldId[] = "field";
-static const char VarId[] = "var";
-static const char NamedCastId[] = "named-cast";
-static const char CStyleCastId[] = "c-style-cast";
-static const char ExplicitCastId[] = "explicit-cast";
-static const char LambdaId[] = "lambda";
-
void RedundantVoidArgCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(functionDecl(parameterCountIs(0), unless(isImplicit()),
- unless(isInstantiated()),
unless(isExternC()))
- .bind(FunctionId),
- this);
- Finder->addMatcher(typedefNameDecl(unless(isImplicit())).bind(TypedefId),
+ const VariadicDynCastAllOfMatcher<TypeLoc, FunctionProtoTypeLoc>
+ functionProtoTypeLoc; // NOLINT(readability-identifier-naming)
+ Finder->addMatcher(traverse(TK_IgnoreUnlessSpelledInSource,
+ functionProtoTypeLoc(
+ unless(hasParent(functionDecl(isExternC()))))
+ .bind("fn")),
this);
- auto ParenFunctionType = parenType(innerType(functionType()));
- auto PointerToFunctionType = pointee(ParenFunctionType);
- auto FunctionOrMemberPointer =
- anyOf(hasType(pointerType(PointerToFunctionType)),
- hasType(memberPointerType(PointerToFunctionType)));
- Finder->addMatcher(fieldDecl(FunctionOrMemberPointer).bind(FieldId), this);
- Finder->addMatcher(varDecl(FunctionOrMemberPointer).bind(VarId), this);
- auto CastDestinationIsFunction =
- hasDestinationType(pointsTo(ParenFunctionType));
Finder->addMatcher(
- cStyleCastExpr(CastDestinationIsFunction).bind(CStyleCastId), this);
- Finder->addMatcher(
- cxxStaticCastExpr(CastDestinationIsFunction).bind(NamedCastId), this);
- Finder->addMatcher(
- cxxReinterpretCastExpr(CastDestinationIsFunction).bind(NamedCastId),
- this);
- Finder->addMatcher(
- cxxConstCastExpr(CastDestinationIsFunction).bind(NamedCastId), this);
- Finder->addMatcher(lambdaExpr().bind(LambdaId), this);
+ traverse(TK_IgnoreUnlessSpelledInSource, lambdaExpr().bind("fn")), this);
}
void RedundantVoidArgCheck::check(const MatchFinder::MatchResult &Result) {
- const BoundNodes &Nodes = Result.Nodes;
- if (const auto *Function = Nodes.getNodeAs<FunctionDecl>(FunctionId))
- processFunctionDecl(Result, Function);
- else if (const auto *TypedefName =
- Nodes.getNodeAs<TypedefNameDecl>(TypedefId))
- processTypedefNameDecl(Result, TypedefName);
- else if (const auto *Member = Nodes.getNodeAs<FieldDecl>(FieldId))
- processFieldDecl(Result, Member);
- else if (const auto *Var = Nodes.getNodeAs<VarDecl>(VarId))
- processVarDecl(Result, Var);
- else if (const auto *NamedCast =
- Nodes.getNodeAs<CXXNamedCastExpr>(NamedCastId))
- processNamedCastExpr(Result, NamedCast);
- else if (const auto *CStyleCast =
- Nodes.getNodeAs<CStyleCastExpr>(CStyleCastId))
- processExplicitCastExpr(Result, CStyleCast);
- else if (const auto *ExplicitCast =
- Nodes.getNodeAs<ExplicitCastExpr>(ExplicitCastId))
- processExplicitCastExpr(Result, ExplicitCast);
- 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);
- const SourceLocation Start = Method && Method->getParent()->isLambda()
- ? Method->getBeginLoc()
- : Function->getLocation();
- SourceLocation End = Function->getEndLoc();
- if (Function->isThisDeclarationADefinition()) {
- if (const Stmt *Body = Function->getBody()) {
- End = Body->getBeginLoc();
- if (End.isMacroID() &&
- Result.SourceManager->isAtStartOfImmediateMacroExpansion(End))
- End = Result.SourceManager->getExpansionLoc(End);
- End = End.getLocWithOffset(-1);
- }
- removeVoidArgumentTokens(Result, SourceRange(Start, End),
- "function definition");
- } else
- removeVoidArgumentTokens(Result, SourceRange(Start, End),
- "function declaration");
-}
-
-static bool isMacroIdentifier(const IdentifierTable &Idents,
- const Token &ProtoToken) {
- if (!ProtoToken.is(tok::TokenKind::raw_identifier))
- return false;
-
- const IdentifierTable::iterator It =
- Idents.find(ProtoToken.getRawIdentifier());
- if (It == Idents.end())
- return false;
-
- return It->second->hadMacroDefinition();
-}
-
-void RedundantVoidArgCheck::removeVoidArgumentTokens(
- const ast_matchers::MatchFinder::MatchResult &Result, SourceRange Range,
- StringRef GrammarLocation) {
- const CharSourceRange CharRange =
- Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Range),
- *Result.SourceManager, getLangOpts());
-
- std::string DeclText =
- Lexer::getSourceText(CharRange, *Result.SourceManager, getLangOpts())
- .str();
- Lexer PrototypeLexer(CharRange.getBegin(), getLangOpts(), DeclText.data(),
- DeclText.data(), DeclText.data() + DeclText.size());
- enum class TokenState {
- Start,
- MacroId,
- MacroLeftParen,
- MacroArguments,
- LeftParen,
- Void,
- };
- TokenState State = TokenState::Start;
- Token VoidToken;
- Token ProtoToken;
- const IdentifierTable &Idents = Result.Context->Idents;
- int MacroLevel = 0;
- const std::string Diagnostic =
- ("redundant void argument list in " + GrammarLocation).str();
-
- while (!PrototypeLexer.LexFromRawLexer(ProtoToken)) {
- switch (State) {
- 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 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 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 = TokenState::LeftParen;
- break;
- }
- }
-
- if (State == TokenState::Void && ProtoToken.is(tok::TokenKind::r_paren))
- removeVoidToken(VoidToken, Diagnostic);
-}
-
-void RedundantVoidArgCheck::removeVoidToken(Token VoidToken,
- StringRef Diagnostic) {
- const SourceLocation VoidLoc = VoidToken.getLocation();
- diag(VoidLoc, Diagnostic) << FixItHint::CreateRemoval(VoidLoc);
-}
-
-void RedundantVoidArgCheck::processTypedefNameDecl(
- const MatchFinder::MatchResult &Result,
- const TypedefNameDecl *TypedefName) {
- 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()))
- removeVoidArgumentTokens(Result, Member->getSourceRange(),
- "field declaration");
-}
-
-void RedundantVoidArgCheck::processVarDecl(
- const MatchFinder::MatchResult &Result, const VarDecl *Var) {
- if (protoTypeHasNoParms(Var->getType())) {
- const SourceLocation Begin = Var->getBeginLoc();
- if (Var->hasInit()) {
- const SourceLocation InitStart =
- Result.SourceManager->getExpansionLoc(Var->getInit()->getBeginLoc())
- .getLocWithOffset(-1);
- removeVoidArgumentTokens(Result, SourceRange(Begin, InitStart),
- "variable declaration with initializer");
- } else
- removeVoidArgumentTokens(Result, Var->getSourceRange(),
- "variable declaration");
- }
-}
-
-void RedundantVoidArgCheck::processNamedCastExpr(
- const MatchFinder::MatchResult &Result, const CXXNamedCastExpr *NamedCast)
{
- 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()))
- removeVoidArgumentTokens(Result, ExplicitCast->getSourceRange(),
- "cast expression");
-}
-
-void RedundantVoidArgCheck::processLambdaExpr(
- const MatchFinder::MatchResult &Result, const LambdaExpr *Lambda) {
- if (Lambda->getLambdaClass()->getLambdaCallOperator()->getNumParams() == 0 &&
- Lambda->hasExplicitParameters()) {
- const SourceManager *SM = Result.SourceManager;
- const TypeLoc TL =
- Lambda->getLambdaClass()->getLambdaTypeInfo()->getTypeLoc();
- removeVoidArgumentTokens(Result,
- {SM->getSpellingLoc(TL.getBeginLoc()),
- SM->getSpellingLoc(TL.getEndLoc())},
- "lambda expression");
- }
+ const FunctionProtoTypeLoc TL = [&] {
+ if (const auto *TL = Result.Nodes.getNodeAs<FunctionProtoTypeLoc>("fn"))
+ return *TL;
+ return Result.Nodes.getNodeAs<LambdaExpr>("fn")
+ ->getCallOperator()
+ ->getTypeSourceInfo()
+ ->getTypeLoc()
+ .getAs<FunctionProtoTypeLoc>();
+ }();
+
+ if (TL.getNumParams() != 0)
+ return;
+
+ const std::optional<Token> Tok = utils::lexer::findNextTokenSkippingComments(
+ Result.SourceManager->getSpellingLoc(TL.getLParenLoc()),
+ *Result.SourceManager, getLangOpts());
+
+ if (!Tok || Tok->isNot(tok::raw_identifier) ||
+ Tok->getRawIdentifier() != "void")
+ return;
+
+ diag(Tok->getLocation(), "redundant void argument list")
+ << FixItHint::CreateRemoval(Tok->getLocation());
}
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.h
b/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.h
index d6edd9950ddae..77ebdc84be7e9 100644
--- a/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/RedundantVoidArgCheck.h
@@ -10,9 +10,6 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_REDUNDANTVOIDARGCHECK_H
#include "../ClangTidyCheck.h"
-#include "clang/Lex/Token.h"
-
-#include <string>
namespace clang::tidy::modernize {
@@ -38,37 +35,6 @@ class RedundantVoidArgCheck : public ClangTidyCheck {
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-
-private:
- void processFunctionDecl(const ast_matchers::MatchFinder::MatchResult
&Result,
- const FunctionDecl *Function);
-
- void
- processTypedefNameDecl(const ast_matchers::MatchFinder::MatchResult &Result,
- const TypedefNameDecl *Typedef);
-
- void processFieldDecl(const ast_matchers::MatchFinder::MatchResult &Result,
- const FieldDecl *Member);
-
- void processVarDecl(const ast_matchers::MatchFinder::MatchResult &Result,
- const VarDecl *Var);
-
- void
- processNamedCastExpr(const ast_matchers::MatchFinder::MatchResult &Result,
- const CXXNamedCastExpr *NamedCast);
-
- void
- processExplicitCastExpr(const ast_matchers::MatchFinder::MatchResult &Result,
- const ExplicitCastExpr *ExplicitCast);
-
- void processLambdaExpr(const ast_matchers::MatchFinder::MatchResult &Result,
- const LambdaExpr *Lambda);
-
- void
- removeVoidArgumentTokens(const ast_matchers::MatchFinder::MatchResult
&Result,
- SourceRange Range, StringRef GrammarLocation);
-
- void removeVoidToken(Token VoidToken, StringRef Diagnostic);
};
} // namespace clang::tidy::modernize
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg-delayed.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg-delayed.cpp
index 4e05ada1d2992..b5916146ac8d8 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg-delayed.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/modernize/redundant-void-arg-delayed.cpp
@@ -1,7 +1,7 @@
// RUN: %check_clang_tidy %s modernize-redundant-void-arg %t -- --
-fdelayed-template-parsing
int foo(void) {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant void argument list in
function definition [modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: int foo() {
return 0;
}
@@ -9,7 +9,7 @@ int foo(void) {
template <class T>
struct MyFoo {
int foo(void) {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list in
function definition [modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: int foo() {
return 0;
}
@@ -21,7 +21,7 @@ template <class T>
struct MyBar {
// This declaration isn't instantiated and won't be parsed
'delayed-template-parsing'.
int foo(void) {
-// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list in
function definition [modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: int foo() {
return 0;
}
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 631149761e5e8..09dc2bb5c1775 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
@@ -18,7 +18,7 @@ extern int i;
int j = 1;
int foo(void) {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant void argument list in
function definition [modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: int foo() {
return 0;
}
@@ -30,24 +30,24 @@ 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-MESSAGES: :[[@LINE-1]]:27: warning: redundant void argument list
[modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: redundant void argument list
[modernize-redundant-void-arg]
// 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-MESSAGES: :[[@LINE-1]]:37: warning: redundant void argument list
[modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-2]]:44: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: typedef int (*returns_fn_void_int_t())();
// Should work for type aliases as well as typedef.
using returns_fn_void_int_t2 = int (*(void))(void);
-// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: {{.*}} in type alias
-// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: {{.*}} in type alias
+// CHECK-MESSAGES: :[[@LINE-1]]:39: warning: redundant void argument list
[modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-2]]:46: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: using returns_fn_void_int_t2 = int (*())();
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-MESSAGES: :[[@LINE-1]]:27: warning: redundant void argument list
[modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: redundant void argument list
[modernize-redundant-void-arg]
// CHECK-FIXES: int (*returns_fn_void_int())() {
return nullptr;
}
@@ -55,27 +55,27 @@ int (*returns_fn_void_int(void))(void) {
// 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-MESSAGES: :[[@LINE-1]]:42: warning: redundant void argument list
[modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: redundant void argument list
[modernize-redundant-void-arg]
+// CHECK-MESSAGES: :[[@LINE-3]]:56: warning: redundant void argument list
[modernize-redundant-void-arg]
// 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: {{...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/173340
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
