llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) <details> <summary>Changes</summary> This is probably easiest to review commit-by-commit. Besides simplifying the code, this refactor should also make it more efficient: instead of using the `hasAnySubstatement` matcher to find blocks we're interested in, which requires looking through every substatement, this PR introduces a custom `hasFinalStmt` matcher which only checks the last substatement. --- Full diff: https://github.com/llvm/llvm-project/pull/171639.diff 2 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp (+28-46) - (modified) clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.h (-13) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp index b77108c49f53c..46cbaa1f70301 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp @@ -8,76 +8,58 @@ #include "RedundantControlFlowCheck.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h" #include "clang/Lex/Lexer.h" using namespace clang::ast_matchers; namespace clang::tidy::readability { -static const char *const RedundantReturnDiag = +namespace { + +AST_MATCHER_P(CompoundStmt, hasFinalStmt, StatementMatcher, InnerMatcher) { + return !Node.body_empty() && + InnerMatcher.matches(*Node.body_back(), Finder, Builder); +} + +} // namespace + +static constexpr StringRef RedundantReturnDiag = "redundant return statement at the end " "of a function with a void return type"; -static const char *const RedundantContinueDiag = +static constexpr StringRef RedundantContinueDiag = "redundant continue statement at the " "end of loop statement"; -static bool isLocationInMacroExpansion(const SourceManager &SM, - SourceLocation Loc) { - return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc); -} - void RedundantControlFlowCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - functionDecl(isDefinition(), returns(voidType()), - hasBody(compoundStmt(hasAnySubstatement( - returnStmt(unless(has(expr()))))) - .bind("return"))), - this); - Finder->addMatcher( - mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt) - .with(hasBody(compoundStmt(hasAnySubstatement(continueStmt())) - .bind("continue"))), + functionDecl(returns(voidType()), + hasBody(compoundStmt(hasFinalStmt( + returnStmt(unless(has(expr()))).bind("stmt"))))), this); + Finder->addMatcher(mapAnyOf(forStmt, cxxForRangeStmt, whileStmt, doStmt) + .with(hasBody(compoundStmt( + hasFinalStmt(continueStmt().bind("stmt"))))), + this); } void RedundantControlFlowCheck::check(const MatchFinder::MatchResult &Result) { - if (const auto *Return = Result.Nodes.getNodeAs<CompoundStmt>("return")) - checkRedundantReturn(Result, Return); - else if (const auto *Continue = - Result.Nodes.getNodeAs<CompoundStmt>("continue")) - checkRedundantContinue(Result, Continue); -} - -void RedundantControlFlowCheck::checkRedundantReturn( - const MatchFinder::MatchResult &Result, const CompoundStmt *Block) { - const CompoundStmt::const_reverse_body_iterator Last = Block->body_rbegin(); - if (const auto *Return = dyn_cast<ReturnStmt>(*Last)) - issueDiagnostic(Result, Block, Return->getSourceRange(), - RedundantReturnDiag); -} - -void RedundantControlFlowCheck::checkRedundantContinue( - const MatchFinder::MatchResult &Result, const CompoundStmt *Block) { - const CompoundStmt::const_reverse_body_iterator Last = Block->body_rbegin(); - if (const auto *Continue = dyn_cast<ContinueStmt>(*Last)) - issueDiagnostic(Result, Block, Continue->getSourceRange(), - RedundantContinueDiag); -} + const auto &RedundantStmt = *Result.Nodes.getNodeAs<Stmt>("stmt"); + const SourceRange StmtRange = RedundantStmt.getSourceRange(); -void RedundantControlFlowCheck::issueDiagnostic( - const MatchFinder::MatchResult &Result, const CompoundStmt *const Block, - const SourceRange &StmtRange, const char *const Diag) { - const SourceManager &SM = *Result.SourceManager; - if (isLocationInMacroExpansion(SM, StmtRange.getBegin())) + if (StmtRange.getBegin().isMacroID()) return; const auto RemovedRange = CharSourceRange::getCharRange( StmtRange.getBegin(), - Lexer::findLocationAfterToken(StmtRange.getEnd(), tok::semi, SM, - getLangOpts(), + Lexer::findLocationAfterToken(StmtRange.getEnd(), tok::semi, + *Result.SourceManager, getLangOpts(), /*SkipTrailingWhitespaceAndNewLine=*/true)); - diag(StmtRange.getBegin(), Diag) << FixItHint::CreateRemoval(RemovedRange); + diag(StmtRange.getBegin(), isa<ReturnStmt>(RedundantStmt) + ? RedundantReturnDiag + : RedundantContinueDiag) + << FixItHint::CreateRemoval(RemovedRange); } } // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.h index fde305039d4c9..028f9948f74ab 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.h +++ b/clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.h @@ -30,19 +30,6 @@ class RedundantControlFlowCheck : public ClangTidyCheck { std::optional<TraversalKind> getCheckTraversalKind() const override { return TK_IgnoreUnlessSpelledInSource; } - -private: - void - checkRedundantReturn(const ast_matchers::MatchFinder::MatchResult &Result, - const CompoundStmt *Block); - - void - checkRedundantContinue(const ast_matchers::MatchFinder::MatchResult &Result, - const CompoundStmt *Block); - - void issueDiagnostic(const ast_matchers::MatchFinder::MatchResult &Result, - const CompoundStmt *Block, const SourceRange &StmtRange, - const char *Diag); }; } // namespace clang::tidy::readability `````````` </details> https://github.com/llvm/llvm-project/pull/171639 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
