[clang-tidy] Fix a template function false positive in misc-unused-using-decls check.

[hokein]

Summary

Ignore warning uninstantiated template function usages.

[alexfh]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

I think, we should use a node matcher for UnresolvedLookupExpr and instead use callExpr(callee(unresolvedLookupExpr().bind("something"))).

Also, I would create the new matcher in the ASTMatchers.h right away, together with a test and docs. But if you want this patch to be in soon, feel free to move the matcher to the matchers library in a follow up.

[hokein]

Address review comments.

[hokein]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

Agree. I will add a node matcher for UnresolvedLookupExpr in a follow-up patch.

[alexfh]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

Sorry for being unclear. Please change this to the node matcher in this patch. You can move the matcher to the ASTMatchers.h as a follow-up. hasUnresolvedLookupExpr is not in line with the matchers design: the CallExpr node has no property UnresolvedLookupExpr. You'll also be able to bind the UnresolvedLookupExpr to an identifier and make the code in check() slightly shorter.

[hokein]

Make a node matcher for unresolvedLookupExpr type.

[hokein]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

Done. But currently we can't bind the UnresolvedLookupExpr directly since the AST_MATCHER doesn't provide a bind function here.

[alexfh]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

That's because we need a node matcher, not narrowing matcher. I guess, this should work:

const internal::VariadicAllOfMatcher<UnresolvedLookupExpr> unresolvedLookupExpr;

[hokein]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

Yeah. But as the name indicates, VariadicAllOfMatcher is used internally in ASTMatcher, is it reasonable to use it here? I can't find such usage in clang-tidy side.

[alexfh]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

There are far fewer nodes in the AST than their properties, so the core matchers library should contain all node matchers. Thus, the user code is not supposed to create any node matchers and the corresponding API is considered an implementation detail of the matchers library. It's fine to use internal API here for a short period until we move the matcher to the core matchers library.

[hokein]

Create a node matcher for UnresolvedLookupExpr.

[hokein]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

24

Sounds good. Done.

[alexfh]

LG

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

95

Range-based for loop, maybe?

[hokein]

Use for-range loop.

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

 //===--- UnusedUsingDeclsCheck.cpp - clang-tidy----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "UnusedUsingDeclsCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
+namespace {
+// FIXME: Add a node matcher for UnresolvedLookupExpr in ASTMatcher.
+AST_MATCHER(CallExpr, hasUnresolvedLookupExpr) {
+  return isa<UnresolvedLookupExpr>(Node.getCallee());
+}
+}
+
 void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
   auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
   Finder->addMatcher(loc(recordType(DeclMatcher)), this);
   Finder->addMatcher(loc(templateSpecializationType(DeclMatcher)), this);
   Finder->addMatcher(declRefExpr().bind("used"), this);
+  Finder->addMatcher(callExpr(hasUnresolvedLookupExpr()).bind("used"), this);
 }
 
 void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
     // FIXME: Implement the correct behavior for using declarations with more
     // than one shadow.
     if (Using->shadow_size() != 1)
       return;
if (const auto *FD = dyn_cast<FunctionDecl>(DRE->getDecl())) {
       if (const auto *FDT = FD->getPrimaryTemplate())
         removeFromFoundDecls(FDT);
       else
         removeFromFoundDecls(FD);
     } else if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
       removeFromFoundDecls(VD);
     }
   }
+  // Check the uninstantiated template function usage.
+  if (auto *CE = Result.Nodes.getNodeAs<CallExpr>("used")) {
+    auto *ULE = dyn_cast<UnresolvedLookupExpr>(CE->getCallee());
+    for (auto I = ULE->decls_begin(), E = ULE->decls_end(); I != E; ++I) {
+      if (auto *USD = dyn_cast<UsingShadowDecl>(I.getDecl()))
+        removeFromFoundDecls(USD->getTargetDecl()->getCanonicalDecl());
+    }
+  }
 }
 
 void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
   auto I = FoundDecls.find(D->getCanonicalDecl());
   if (I != FoundDecls.end())
     I->second = nullptr;
 }
 

test/clang-tidy/misc-unused-using-decls.cpp

 template <typename T> class E {};
 template <typename T> class F {};
 class G { public: static void func() {} };
 class H { public: static int i; };
 class I {
  public:
   static int ii;
 };
+template <typename T> class J {};
 
 class Base {
  public:
   void f();
 };
 
 D UsedInstance;
 D UnusedInstance;
 
 int UsedFunc() { return 1; }
 int UnusedFunc() { return 1; }
 template <typename T> int UsedTemplateFunc() { return 1; }
 template <typename T> int UnusedTemplateFunc() { return 1; }
+template <typename T> int UsedInTemplateFunc() { return 1; }
 
 class ostream {
 public:
   ostream &operator<<(ostream &(*PF)(ostream &));
 };
 extern ostream cout;
 ostream &endl(ostream &os);
 }

 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedInstance' is unused
 // CHECK-FIXES: {{^}}// UnusedInstance
 using n::UnusedFunc; // UnusedFunc
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedFunc' is unused
 // CHECK-FIXES: {{^}}// UnusedFunc
 using n::cout;
 using n::endl;
 
+using n::UsedInTemplateFunc;
+using n::J;
+template <typename T> void Callee() {
+  J<T> j;
+  UsedInTemplateFunc<T>();
+}
+
 #define DEFINE_INT(name)        \
   namespace INT {               \
   static const int _##name = 1; \
   }                             \
   using INT::_##name
 DEFINE_INT(test);
 #undef DEFIND_INT