Author: sbenza Date: Wed May 25 11:19:23 2016 New Revision: 270714 URL: http://llvm.org/viewvc/llvm-project?rev=270714&view=rev Log: Speed up check by using a recursive visitor.
Summary: Use a recursive visitor instead of forEachDescendant() matcher. The latter requires several layers of virtual function calls for each node and it is more expensive than the visitor. Benchmark results show improvement of ~6% walltime in clang-tidy. Reviewers: alexfh Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D20597 Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp?rev=270714&r1=270713&r2=270714&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.cpp Wed May 25 11:19:23 2016 @@ -8,6 +8,7 @@ //===----------------------------------------------------------------------===// #include "FunctionSizeCheck.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" using namespace clang::ast_matchers; @@ -16,6 +17,57 @@ namespace clang { namespace tidy { namespace readability { +class FunctionASTVisitor : public RecursiveASTVisitor<FunctionASTVisitor> { + using Base = RecursiveASTVisitor<FunctionASTVisitor>; + +public: + bool TraverseStmt(Stmt *Node) { + if (!Node) + return Base::TraverseStmt(Node); + + if (TrackedParent.back() && !isa<CompoundStmt>(Node)) + ++Info.Statements; + + switch (Node->getStmtClass()) { + case Stmt::IfStmtClass: + case Stmt::WhileStmtClass: + case Stmt::DoStmtClass: + case Stmt::CXXForRangeStmtClass: + case Stmt::ForStmtClass: + case Stmt::SwitchStmtClass: + ++Info.Branches; + // fallthrough + case Stmt::CompoundStmtClass: + TrackedParent.push_back(true); + break; + default: + TrackedParent.push_back(false); + break; + } + + Base::TraverseStmt(Node); + + TrackedParent.pop_back(); + return true; + } + + bool TraverseDecl(Decl *Node) { + TrackedParent.push_back(false); + Base::TraverseDecl(Node); + TrackedParent.pop_back(); + return true; + } + + struct FunctionInfo { + FunctionInfo() : Lines(0), Statements(0), Branches(0) {} + unsigned Lines; + unsigned Statements; + unsigned Branches; + }; + FunctionInfo Info; + std::vector<bool> TrackedParent; +}; + FunctionSizeCheck::FunctionSizeCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), LineThreshold(Options.get("LineThreshold", -1U)), @@ -29,76 +81,52 @@ void FunctionSizeCheck::storeOptions(Cla } void FunctionSizeCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - functionDecl( - unless(isInstantiated()), - forEachDescendant( - stmt(unless(compoundStmt()), - hasParent(stmt(anyOf(compoundStmt(), ifStmt(), - anyOf(whileStmt(), doStmt(), - cxxForRangeStmt(), forStmt()))))) - .bind("stmt"))).bind("func"), - this); + Finder->addMatcher(functionDecl(unless(isInstantiated())).bind("func"), this); } void FunctionSizeCheck::check(const MatchFinder::MatchResult &Result) { const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func"); - FunctionInfo &FI = FunctionInfos[Func]; + FunctionASTVisitor Visitor; + Visitor.TraverseDecl(const_cast<FunctionDecl *>(Func)); + auto &FI = Visitor.Info; + + if (FI.Statements == 0) + return; // Count the lines including whitespace and comments. Really simple. - if (!FI.Lines) { - if (const Stmt *Body = Func->getBody()) { - SourceManager *SM = Result.SourceManager; - if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) { - FI.Lines = SM->getSpellingLineNumber(Body->getLocEnd()) - - SM->getSpellingLineNumber(Body->getLocStart()); - } + if (const Stmt *Body = Func->getBody()) { + SourceManager *SM = Result.SourceManager; + if (SM->isWrittenInSameFile(Body->getLocStart(), Body->getLocEnd())) { + FI.Lines = SM->getSpellingLineNumber(Body->getLocEnd()) - + SM->getSpellingLineNumber(Body->getLocStart()); } } - const auto *Statement = Result.Nodes.getNodeAs<Stmt>("stmt"); - ++FI.Statements; - - // TODO: switch cases, gotos - if (isa<IfStmt>(Statement) || isa<WhileStmt>(Statement) || - isa<ForStmt>(Statement) || isa<SwitchStmt>(Statement) || - isa<DoStmt>(Statement) || isa<CXXForRangeStmt>(Statement)) - ++FI.Branches; -} - -void FunctionSizeCheck::onEndOfTranslationUnit() { - // If we're above the limit emit a warning. - for (const auto &P : FunctionInfos) { - const FunctionInfo &FI = P.second; - if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold || - FI.Branches > BranchThreshold) { - diag(P.first->getLocation(), - "function %0 exceeds recommended size/complexity thresholds") - << P.first; - } - - if (FI.Lines > LineThreshold) { - diag(P.first->getLocation(), - "%0 lines including whitespace and comments (threshold %1)", - DiagnosticIDs::Note) - << FI.Lines << LineThreshold; - } + if (FI.Lines > LineThreshold || FI.Statements > StatementThreshold || + FI.Branches > BranchThreshold) { + diag(Func->getLocation(), + "function %0 exceeds recommended size/complexity thresholds") + << Func; + } - if (FI.Statements > StatementThreshold) { - diag(P.first->getLocation(), "%0 statements (threshold %1)", - DiagnosticIDs::Note) - << FI.Statements << StatementThreshold; - } + if (FI.Lines > LineThreshold) { + diag(Func->getLocation(), + "%0 lines including whitespace and comments (threshold %1)", + DiagnosticIDs::Note) + << FI.Lines << LineThreshold; + } - if (FI.Branches > BranchThreshold) { - diag(P.first->getLocation(), "%0 branches (threshold %1)", - DiagnosticIDs::Note) - << FI.Branches << BranchThreshold; - } + if (FI.Statements > StatementThreshold) { + diag(Func->getLocation(), "%0 statements (threshold %1)", + DiagnosticIDs::Note) + << FI.Statements << StatementThreshold; } - FunctionInfos.clear(); + if (FI.Branches > BranchThreshold) { + diag(Func->getLocation(), "%0 branches (threshold %1)", DiagnosticIDs::Note) + << FI.Branches << BranchThreshold; + } } } // namespace readability Modified: clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h?rev=270714&r1=270713&r2=270714&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/readability/FunctionSizeCheck.h Wed May 25 11:19:23 2016 @@ -34,21 +34,11 @@ public: void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - void onEndOfTranslationUnit() override; private: - struct FunctionInfo { - FunctionInfo() : Lines(0), Statements(0), Branches(0) {} - unsigned Lines; - unsigned Statements; - unsigned Branches; - }; - const unsigned LineThreshold; const unsigned StatementThreshold; const unsigned BranchThreshold; - - llvm::DenseMap<const FunctionDecl *, FunctionInfo> FunctionInfos; }; } // namespace readability _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
