From: Alexander Kornienko Date: Wed, 17 May 2017 02:25:11 +0000 (+0000) Subject: [clang-tidy] Optimize misc-unused-parameters. NFCI X-Git-Tag: llvmorg-5.0.0-rc1~4912 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4d5dd3b7ef59d2abc88d3cee9c479c9be68fe0da;p=platform%2Fupstream%2Fllvm.git [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. llvm-svn: 303230 --- diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp index 6c60ede..07e82f0 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp @@ -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 using namespace clang::ast_matchers; @@ -27,7 +29,10 @@ bool isOverrideMethod(const FunctionDecl *Function) { } // namespace void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher(functionDecl().bind("function"), this); + Finder->addMatcher( + functionDecl(isDefinition(), hasBody(stmt()), hasAnyParameter(decl())) + .bind("function"), + this); } template @@ -65,21 +70,74 @@ static FixItHint removeArgument(const MatchFinder::MatchResult &Result, Index + 1 < Call->getNumArgs() ? Call->getArg(Index + 1) : nullptr)); } +class UnusedParametersCheck::IndexerVisitor + : public RecursiveASTVisitor { +public: + IndexerVisitor(TranslationUnitDecl *Top) { TraverseDecl(Top); } + + const std::unordered_set & + getFnCalls(const FunctionDecl *Fn) { + return Index[Fn->getCanonicalDecl()].Calls; + } + + const std::unordered_set & + getOtherRefs(const FunctionDecl *Fn) { + return Index[Fn->getCanonicalDecl()].OtherRefs; + } + + bool shouldTraversePostOrder() const { return true; } + + bool WalkUpFromDeclRefExpr(DeclRefExpr *DeclRef) { + if (const auto *Fn = dyn_cast(DeclRef->getDecl())) { + Fn = Fn->getCanonicalDecl(); + Index[Fn].OtherRefs.insert(DeclRef); + } + return true; + } + + bool WalkUpFromCallExpr(CallExpr *Call) { + if (const auto *Fn = + dyn_cast_or_null(Call->getCalleeDecl())) { + Fn = Fn->getCanonicalDecl(); + if (const auto *Ref = + dyn_cast(Call->getCallee()->IgnoreImplicit())) { + Index[Fn].OtherRefs.erase(Ref); + } + Index[Fn].Calls.insert(Call); + } + return true; + } + +private: + struct IndexEntry { + std::unordered_set Calls; + std::unordered_set OtherRefs; + }; + + std::unordered_map 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( + 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::warnOnUnusedParameter( 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("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("function"); - if (!Function->doesThisDeclarationHaveABody() || - !Function->hasWrittenPrototype() || Function->isTemplateInstantiation()) + if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation()) return; if (const auto *Method = dyn_cast(Function)) if (Method->isLambdaStaticInvoker()) diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h index 334f897..414ef11 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h +++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.h @@ -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 Indexer; + void warnOnUnusedParameter(const ast_matchers::MatchFinder::MatchResult &Result, const FunctionDecl *Function, unsigned ParamIndex);