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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
