Author: alexfh Date: Tue May 16 21:25:11 2017 New Revision: 303230 URL: http://llvm.org/viewvc/llvm-project?rev=303230&view=rev Log: [clang-tidy] Optimize misc-unused-parameters. NFCI
Don't traverse AST each time we need to find references to a certain function. Traverse the AST once using RAV and cache the index of function references. The speed up on a particular large file was about 1000x. Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=303230&r1=303229&r2=303230&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Tue May 16 21:25:11 2017 @@ -9,8 +9,10 @@ #include "UnusedParametersCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" +#include <unordered_set> using namespace clang::ast_matchers; @@ -27,7 +29,10 @@ bool isOverrideMethod(const FunctionDecl } // namespace void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(functionDecl().bind("function"), this); + Finder->addMatcher( + functionDecl(isDefinition(), hasBody(stmt()), hasAnyParameter(decl())) + .bind("function"), + this); } template <typename T> @@ -65,21 +70,74 @@ static FixItHint removeArgument(const Ma Index + 1 < Call->getNumArgs() ? Call->getArg(Index + 1) : nullptr)); } +class UnusedParametersCheck::IndexerVisitor + : public RecursiveASTVisitor<IndexerVisitor> { +public: + IndexerVisitor(TranslationUnitDecl *Top) { TraverseDecl(Top); } + + const std::unordered_set<const CallExpr *> & + getFnCalls(const FunctionDecl *Fn) { + return Index[Fn->getCanonicalDecl()].Calls; + } + + const std::unordered_set<const DeclRefExpr *> & + getOtherRefs(const FunctionDecl *Fn) { + return Index[Fn->getCanonicalDecl()].OtherRefs; + } + + bool shouldTraversePostOrder() const { return true; } + + bool WalkUpFromDeclRefExpr(DeclRefExpr *DeclRef) { + if (const auto *Fn = dyn_cast<FunctionDecl>(DeclRef->getDecl())) { + Fn = Fn->getCanonicalDecl(); + Index[Fn].OtherRefs.insert(DeclRef); + } + return true; + } + + bool WalkUpFromCallExpr(CallExpr *Call) { + if (const auto *Fn = + dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl())) { + Fn = Fn->getCanonicalDecl(); + if (const auto *Ref = + dyn_cast<DeclRefExpr>(Call->getCallee()->IgnoreImplicit())) { + Index[Fn].OtherRefs.erase(Ref); + } + Index[Fn].Calls.insert(Call); + } + return true; + } + +private: + struct IndexEntry { + std::unordered_set<const CallExpr *> Calls; + std::unordered_set<const DeclRefExpr *> OtherRefs; + }; + + std::unordered_map<const FunctionDecl *, IndexEntry> Index; +}; + +UnusedParametersCheck::~UnusedParametersCheck() = default; + +UnusedParametersCheck::UnusedParametersCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void UnusedParametersCheck::warnOnUnusedParameter( const MatchFinder::MatchResult &Result, const FunctionDecl *Function, unsigned ParamIndex) { const auto *Param = Function->getParamDecl(ParamIndex); auto MyDiag = diag(Param->getLocation(), "parameter %0 is unused") << Param; - auto DeclRefExpr = - declRefExpr(to(equalsNode(Function)), - unless(hasAncestor(callExpr(callee(equalsNode(Function)))))); + if (!Indexer) { + Indexer = llvm::make_unique<IndexerVisitor>( + Result.Context->getTranslationUnitDecl()); + } // Comment out parameter name for non-local functions. if (Function->isExternallyVisible() || !Result.SourceManager->isInMainFile(Function->getLocation()) || - !ast_matchers::match(DeclRefExpr, *Result.Context).empty() || - isOverrideMethod(Function)) { + !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) { SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd()); // Note: We always add a space before the '/*' to not accidentally create a // '*/*' for pointer types, which doesn't start a comment. clang-format will @@ -95,19 +153,13 @@ void UnusedParametersCheck::warnOnUnused MyDiag << removeParameter(Result, FD, ParamIndex); // Fix all call sites. - auto CallMatches = ast_matchers::match( - decl(forEachDescendant( - callExpr(callee(functionDecl(equalsNode(Function)))).bind("x"))), - *Result.Context->getTranslationUnitDecl(), *Result.Context); - for (const auto &Match : CallMatches) - MyDiag << removeArgument(Result, Match.getNodeAs<CallExpr>("x"), - ParamIndex); + for (const auto *Call : Indexer->getFnCalls(Function)) + MyDiag << removeArgument(Result, Call, ParamIndex); } void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) { const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function"); - if (!Function->doesThisDeclarationHaveABody() || - !Function->hasWrittenPrototype() || Function->isTemplateInstantiation()) + if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation()) return; if (const auto *Method = dyn_cast<CXXMethodDecl>(Function)) if (Method->isLambdaStaticInvoker()) Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h?rev=303230&r1=303229&r2=303230&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.h Tue May 16 21:25:11 2017 @@ -20,12 +20,15 @@ namespace misc { /// turned on. class UnusedParametersCheck : public ClangTidyCheck { public: - UnusedParametersCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UnusedParametersCheck(StringRef Name, ClangTidyContext *Context); + ~UnusedParametersCheck(); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; private: + class IndexerVisitor; + std::unique_ptr<IndexerVisitor> Indexer; + void warnOnUnusedParameter(const ast_matchers::MatchFinder::MatchResult &Result, const FunctionDecl *Function, unsigned ParamIndex); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits