[clang-tidy] Fix a false positive for explicit template instantiations in misc-unused-using-decls.

[hokein]

Summary

The AST for explicit template instantiations is slightly different
from the source code (see note below): there is no AST node for explicit template
instantiations, so regular ast matchers like typeLoc can't find the
type which is referenced in template arguments of an explicit template
instantiations.

Our workaround is to record all declarations referenced from template arguments
of explicit template instantiations and the instantiation source location of
instantiations, and assumes that the using-decl is used when the using-decl's
source location is before the instantiation source location.

While the workaround avoids false positives, it may introduce true negatives.
but this case rarely happens and users won't get bothered by true positives (No
warning message is print by the check).

Note:

// NamespaceDecl 0x7f875981e798 </tmp/t.cc:1:1, line:3:1> line:1:11 ns
// | `-CXXRecordDecl 0x7f875a02f400 <line:2:1, col:10> col:7 referenced class A definition
// |   `-CXXRecordDecl 0x7f875a02f520 <col:1, col:7> col:7 implicit class A
// |-FunctionTemplateDecl 0x7f875a02f768 <line:4:1, line:6:1> line:5:6 test
// | |-TemplateTypeParmDecl 0x7f875a02f5b8 <line:4:10, col:16> col:16 class T
// | |-FunctionDecl 0x7f875a02f6c0 <line:5:1, line:6:1> line:5:6 test 'void (void)'
// | | `-CompoundStmt 0x7f875a02f7f8 <col:13, line:6:1>
// | `-FunctionDecl 0x7f875a02f9d0 <line:5:1, line:6:1> line:5:6 test 'void (void)'
// |   |-TemplateArgument type 'class ns::A'
// |   `-CompoundStmt 0x7f875a02f7f8 <col:13, line:6:1>
// |-UsingDecl 0x7f875a02f830 <line:7:1, col:11> col:11 ns::A
// `-UsingShadowDecl 0x7f875a02f880 <col:11> col:11 implicit CXXRecord 0x7f875a02f400 'A'
//   `-RecordType 0x7f875a02f4a0 'class ns::A'
//       `-CXXRecord 0x7f875a02f400 'A'
namespace ns {
class A {};
}
template<class T>
void test() {
}
using ns::A;
template void test<A>();

[alexfh]

it may introduce true positives

True positives is all we need from clang-tidy checks ;) I guess, you meant "false negatives" (https://en.wikipedia.org/wiki/False_positives_and_false_negatives).

The main question here is whether we should extend the AST instead of hacking around its limitations. It's probably best to discuss with Richard Smith.

[alexfh]

Richard, it seems like the AST could be improved here by adding nodes for the explicit instantiation declarations and definitions or using existing nodes, if there are suitable ones. What do you think?

[alexfh]

I think, we decided to extend the AST instead of working around its incompleteness.

clang-tidy/misc/UnusedUsingDeclsCheck.h

 //===--- UnusedUsingDeclsCheck.h - clang-tidy--------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H
 
 #include "../ClangTidy.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include <vector>
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 /// Finds unused using declarations.
struct UsingDeclContext {
     const UsingDecl *FoundUsingDecl;
     // The source range of the UsingDecl.
     CharSourceRange UsingDeclRange;
     // Whether the UsingDecl is used.
     bool IsUsed;
   };
 
   std::vector<UsingDeclContext> Contexts;
+  // Mapping from the declaration (CXXRecordDecl, EnumDecl), which is referenced
+  // from a template argument of an explicit template instantiation, to the
+  // source location of the instantiation.
+  //
+  // It is used to determine whether a declaration of using-decl is used in
+  // explicit template instantiations.
+  // As there is no AST node for explicit template instantiations, regular
+  // matchers like "loc" can't find types referenced in template argument
+  // of an explicit template instantiation. We handle this case specifically.
+  llvm::DenseMap<const Decl *, SourceLocation>
+      TemplateArgsInExplicitInstantiation;
 };
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
+namespace {
+AST_POLYMORPHIC_MATCHER(
+    isExplicitInstantiation,
+    AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
+                                    FunctionDecl)) {
+  auto Kind = Node.getTemplateSpecializationKind();
+  return Kind == TSK_ExplicitInstantiationDeclaration ||
+         Kind == TSK_ExplicitInstantiationDefinition;
+}
+} // namespace
+
 // A function that helps to tell whether a TargetDecl in a UsingDecl will be
 // 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(
       callExpr(hasDeclaration(functionDecl(hasAnyTemplateArgument(
           anyOf(refersToTemplate(templateName().bind("used")),
                 refersToDeclaration(functionDecl().bind("used"))))))),
       this);
   Finder->addMatcher(loc(templateSpecializationType(hasAnyTemplateArgument(
                          templateArgument().bind("used")))),
                      this);
+
+  auto TypeTemplateArgument = templateArgument(
+      anyOf(refersToType(enumType()),
+            refersToType(qualType(hasDeclaration(cxxRecordDecl())))));
+  // Find usage in explicitly instantiated function declarations.
+  Finder->addMatcher(functionDecl(hasAnyTemplateArgument(
+                                      TypeTemplateArgument.bind("maybe_used")),
+                                  isExplicitInstantiation())
+                         .bind("fun_decl"),
+                     this);
+  // Find usages in explicitly instantiated class declarations.
+  Finder->addMatcher(
+      classTemplateSpecializationDecl(
+          hasAnyTemplateArgument(TypeTemplateArgument.bind("maybe_used")),
+          isExplicitInstantiation())
+          .bind("class_decl"),
+      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;
 
if (const auto *Used = Result.Nodes.getNodeAs<NamedDecl>("used")) {
     } else if (const auto *Specialization =
                    dyn_cast<ClassTemplateSpecializationDecl>(Used)) {
       Used = Specialization->getSpecializedTemplate();
     }
     removeFromFoundDecls(Used);
     return;
   }
 
+  if (const auto *Used =
+          Result.Nodes.getNodeAs<TemplateArgument>("maybe_used")) {
+    SourceLocation InstantiationLoc;
+    if (const auto *DC = Result.Nodes.getNodeAs<FunctionDecl>("fun_decl")) {
+      InstantiationLoc = DC->getPointOfInstantiation();
+    } else if (const auto *DC =
+                   Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>(
+                       "class_decl")) {
+      InstantiationLoc= DC->getPointOfInstantiation();
+    }
+
+    assert(Used->getKind() == TemplateArgument::Type);
+    QualType Type = Used->getAsType();
+    if (auto *RD = Type->getAsCXXRecordDecl()) {
+      TemplateArgsInExplicitInstantiation[RD] = InstantiationLoc;
+    } else if (auto *ET = Type->getAs<EnumType>()) {
+      TemplateArgsInExplicitInstantiation[ET->getDecl()] = InstantiationLoc;
+    }
+  }
+
   if (const auto *Used = Result.Nodes.getNodeAs<TemplateArgument>("used")) {
     // FIXME: Support non-type template parameters.
     if (Used->getKind() == TemplateArgument::Template) {
       if (const auto *TD = Used->getAsTemplate().getAsTemplateDecl())
         removeFromFoundDecls(TD);
     } else if (Used->getKind() == TemplateArgument::Type) {
       if (auto *RD = Used->getAsType()->getAsCXXRecordDecl())
         removeFromFoundDecls(RD);
for (auto &Context : Contexts) {
     if (Context.UsingTargetDecls.count(D->getCanonicalDecl()) > 0)
       Context.IsUsed = true;
   }
 }
 
 void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
   for (const auto &Context : Contexts) {
     if (!Context.IsUsed) {
+      // Filter out all usages in explicit template instantiations.
+      bool IsUsed = false;
+      for (auto Iter : TemplateArgsInExplicitInstantiation) {
+        if (Context.UsingTargetDecls.count(Iter.first) > 0) {
+          const auto &SM = Iter.first->getASTContext().getSourceManager();
+          // Check whether the source location of instantiation is before the
+          // source location of using-decl.
+          if (SM.isBeforeInTranslationUnit(
+                  Context.FoundUsingDecl->getLocation(), Iter.second)) {
+            IsUsed = true;
+            break;
+          }
+        }
+      }
+      if (!IsUsed) {
         diag(Context.FoundUsingDecl->getLocation(), "using decl %0 is unused")
             << Context.FoundUsingDecl
             << FixItHint::CreateRemoval(Context.UsingDeclRange);
       }
     }
+  }
   Contexts.clear();
 }
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang

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

 template <typename T> class K {};
 template <template <typename> class S>
 class L {};
 
 template <typename T> class M {};
 class N {};
 
 template <int T> class P {};
+
+class Q {};
+
+template <int T> class R {};
+
+class S {};
+
+class U {};
+class V {};
 const int Constant = 0;
 
 class Base {
  public:
   void f();
 };
 
 D UsedInstance;

 enum Color1 { Green };
 
 enum Color2 { Red };
 
 enum Color3 { Yellow };
 
 enum Color4 { Blue };
 
+enum Color5 {};
+enum Color6 {};
+enum Color7 {};
+enum Color8 {};
 }  // namespace n
 
 #include "unused-using-decls.h"
 namespace ns {
 template <typename T>
 class AA {
   T t;
 };

 
 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 n::Color5;
+using n::Color7;
 
 using ns::AA;
 using ns::ff;
 
 using n::K;
 
 using n::N;
 
 // FIXME: Currently non-type template arguments are not supported.
 using n::Constant;
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'Constant' is unused
 
+using n::Q;
+
+using n::U;
+
 // ----- Usages -----
 void f(B b);
 void g() {
   vector<C> data;
   D::i = 1;
   F<int> f;
   void (*func)() = &G::func;
   int *i = &H::i;

 void h(n::M<T>* t) {}
 // n::N is used the explicit template instantiation.
 template void h(n::M<N>* t);
 
 // Test on Non-type template arguments.
 template <int T>
 void i(n::P<T>* t) {}
 template void i(n::P<Constant>* t);
+
+// Test template arguments in explicit function template instantiations.
+template<class T>
+void j() {};
+template void j<Q>();
+template void j<Color5>();
+
+template void j<n::S>();
+using n::S;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'S' is unused
+template void j<n::Color6>();
+using n::Color6;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'Color6' is unused
+
+// Test template arguments in explicit class template instantiations.
+template<class T>
+class k {};
+template class k<U>;
+template class k<Color7>;
+
+template class k<n::V>;
+using n::V;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'V' is unused
+
+template class k<n::Color8>;
+using n::Color8;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'Color8' is unused