[clang-tools-extra] r303230 - [clang-tidy] Optimize misc-unused-parameters. NFCI

[Alexander Kornienko via cfe-commits]

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);