[clang-tidy] Fix an unused-using-decl false positive about template arguments in function call expression.

[hokein]

Summary

The check doesn't mark the template argument as used when the template
argument is a template.

[klimek]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

84–87

I thought the idea was to make an overload for hasTemplateArgument to work on FunctionDecl. Any specific reason not to make the matchers better?

[hokein]

Use new ast matchers to simplify the code.

Updating D22803: [clang-tidy] Fix an unused-using-decl false positive about template arguments in

function call expression.

[hokein]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

84–87

Have added the new matchers to clang ASTMatchers (D22963, D22957), and switched to use these matchers.

[alexfh]

LG

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

 // checked. Only variable, function, function template, class template, class,
 // enum declaration and enum constant declaration are considered.
 static bool ShouldCheckDecl(const Decl *TargetDecl) {
   return isa<RecordDecl>(TargetDecl) || isa<ClassTemplateDecl>(TargetDecl) ||
          isa<FunctionDecl>(TargetDecl) || isa<VarDecl>(TargetDecl) ||
          isa<FunctionTemplateDecl>(TargetDecl) || isa<EnumDecl>(TargetDecl) ||
          isa<EnumConstantDecl>(TargetDecl);
 }
-
 void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
   auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
   Finder->addMatcher(loc(enumType(DeclMatcher)), this);
   Finder->addMatcher(loc(recordType(DeclMatcher)), this);
   Finder->addMatcher(loc(templateSpecializationType(DeclMatcher)), this);
   Finder->addMatcher(declRefExpr().bind("used"), this);
   Finder->addMatcher(callExpr(callee(unresolvedLookupExpr().bind("used"))),
                      this);
+  Finder->addMatcher(
+      callExpr(hasDeclaration(functionDecl(hasAnyTemplateArgument(
+          anyOf(refersToTemplate(templateName().bind("used")),
+                refersToDeclaration(functionDecl().bind("used"))))))),
+      this);
 }
 
 void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *Using = Result.Nodes.getNodeAs<UsingDecl>("using")) {
     // Ignores using-declarations defined in macros.
     if (Using->getLocation().isMacroID())
       return;
 
for (const auto *UsingShadow : Using->shadows()) {
       const auto *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();
       if (ShouldCheckDecl(TargetDecl))
         Context.UsingTargetDecls.insert(TargetDecl);
     }
     if (!Context.UsingTargetDecls.empty())
       Contexts.push_back(Context);
     return;
   }
-
   // Mark using declarations as used by setting FoundDecls' value to zero. As
   // the AST is walked in order, usages are only marked after a the
   // corresponding using declaration has been found.
   // FIXME: This currently doesn't look at whether the type reference is
   // actually found with the help of the using declaration.
   if (const auto *Used = Result.Nodes.getNodeAs<NamedDecl>("used")) {
-    if (const auto *Specialization =
-            dyn_cast<ClassTemplateSpecializationDecl>(Used))
+    if (const auto *FD = dyn_cast<FunctionDecl>(Used)) {
+      removeFromFoundDecls(FD->getPrimaryTemplate());
+    } else if (const auto *Specialization =
+            dyn_cast<ClassTemplateSpecializationDecl>(Used)) {
       Used = Specialization->getSpecializedTemplate();
+    }
     removeFromFoundDecls(Used);
     return;
   }
 
+  if (const auto *Used = Result.Nodes.getNodeAs<TemplateName>("used")) {
+    removeFromFoundDecls(Used->getAsTemplateDecl());
+    return;
+  }
+
   if (const auto *DRE = Result.Nodes.getNodeAs<DeclRefExpr>("used")) {
     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);
     } else if (const auto *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {
       removeFromFoundDecls(ECD);
       if (const auto *ET = ECD->getType()->getAs<EnumType>())
         removeFromFoundDecls(ET->getDecl());
     }
   }
   // Check the uninstantiated template function usage.
   if (const auto *ULE = Result.Nodes.getNodeAs<UnresolvedLookupExpr>("used")) {
     for (const NamedDecl* ND : ULE->decls()) {
       if (const auto *USD = dyn_cast<UsingShadowDecl>(ND))
         removeFromFoundDecls(USD->getTargetDecl()->getCanonicalDecl());
     }
   }
 }
 
 void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
+  if (!D)
+    return;
   // FIXME: Currently, we don't handle the using-decls being used in different
   // scopes (such as different namespaces, different functions). Instead of
   // giving an incorrect message, we mark all of them as used.
   //
   // FIXME: Use a more efficient way to find a matching context.
   for (auto &Context : Contexts) {
     if (Context.UsingTargetDecls.count(D->getCanonicalDecl()) > 0)
       Context.IsUsed = true;

test/clang-tidy/Inputs/unused-using-decls.h

+class MyClass {
+public:
+  template <template <typename> class S, typename T>
+  S<T> *func1(T *a) {
+    return new S<T>();
+  }
+  template <typename T, T (*S)()>
+  void func2(T a) {
+    S();
+  }
+};

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

-// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -isystem %S/Inputs/
+
 
 // ----- Definitions -----
 template <typename T> class vector {};
 namespace n {
 class A;
 class B;
 class C;
 class D;

 enum Color2 { Red };
 
 enum Color3 { Yellow };
 
 enum Color4 { Blue };
 
 }  // namespace n
 
+#include "unused-using-decls.h"
+namespace ns {
+template <typename T>
+class AA {
+  T t;
+};
+template <typename T>
+T ff() { T t; return t; }
+} // namespace ns
+
 // ----- Using declarations -----
 // eol-comments aren't removed (yet)
 using n::A; // A
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
 // CHECK-FIXES: {{^}}// A
 using n::B;
 using n::C;
 using n::D;

 using n::Color1;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'Color1' is unused
 using n::Green;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'Green' is unused
 using n::Color2;
 using n::Color3;
 using n::Blue;
 
+using ns::AA;
+using ns::ff;
+
 // ----- Usages -----
 void f(B b);
 void g() {
   vector<C> data;
   D::i = 1;
   F<int> f;
   void (*func)() = &G::func;
   int *i = &H::i;
   UsedInstance.i;
   UsedFunc();
   UsedTemplateFunc<int>();
   cout << endl;
   Color2 color2;
   int t1 = Color3::Yellow;
   int t2 = Blue;
+
+  MyClass a;
+  int t3 = 0;
+  a.func1<AA>(&t3);
+  a.func2<int, ff>(t3);
 }