[clang-tidy] Ignore function context in misc-unused-using-decls.

[hokein]

Summary

Make the check's behavior more correct when handling using-decls in multiple scopes.

[alexfh]

LG

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

111

I suspect that linear search here may be the reason for the poor performance of the check (on a large file it's in top 3 by time spent). This should be addressed separately. Maybe leave a FIXME?

[hokein]

Address code comments.

clang-tidy/misc/UnusedUsingDeclsCheck.h

public:
   void onEndOfTranslationUnit() override;
 
 private:
   void removeFromFoundDecls(const Decl *D);
 
   struct UsingDeclContext {
     explicit UsingDeclContext(const UsingDecl *FoundUsingDecl)
         : FoundUsingDecl(FoundUsingDecl), IsUsed(false) {}
+    // A set saves all UsingShadowDecls introduced by a UsingDecl. A UsingDecl
+    // can introduce multiple UsingShadowDecls in some cases (such as
+    // overloaded functions).
     llvm::SmallPtrSet<const Decl *, 4> UsingTargetDecls;
+    // The original UsingDecl.
     const UsingDecl *FoundUsingDecl;
+    // The source range of the UsingDecl.
     CharSourceRange UsingDeclRange;
+    // Whether the UsingDecl is used.
     bool IsUsed;
   };
 
   std::vector<UsingDeclContext> Contexts;
 };
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(callExpr(callee(unresolvedLookupExpr().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 ;
+      return;
 
     // Ignores using-declarations defined in class definition.
     if (isa<CXXRecordDecl>(Using->getDeclContext()))
-      return ;
+      return;
 
+    // FIXME: We ignore using-decls defined in function definitions at the
+    // moment because of false positives caused by ADL and different function
+    // scopes.
+    if (isa<FunctionDecl>(Using->getDeclContext()))
+      return;
+
     UsingDeclContext Context(Using);
     Context.UsingDeclRange = CharSourceRange::getCharRange(
         Using->getLocStart(),
         Lexer::findLocationAfterToken(
             Using->getLocEnd(), tok::semi, *Result.SourceManager,
             Result.Context->getLangOpts(),
             /*SkipTrailingWhitespaceAndNewLine=*/true));
     for (const auto *UsingShadow : Using->shadows()) {
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) {
+  // 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) {
+    if (Context.UsingTargetDecls.count(D->getCanonicalDecl()) > 0)
       Context.IsUsed = true;
-      break;
-    }
   }
 }
 
 void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
   for (const auto &Context : Contexts) {
     if (!Context.IsUsed) {
       diag(Context.FoundUsingDecl->getLocation(), "using decl %0 is unused")
           << Context.FoundUsingDecl

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

 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 G;
+class H;
 
 class Base {
  public:
   void f();
 };
 
 D UsedInstance;
 D UnusedInstance;

 DEFINE_INT(test);
 #undef DEFIND_INT
 
 #define USING_FUNC \
   using n::FuncUsedByUsingDeclInMacro;
 USING_FUNC
 #undef USING_FUNC
 
+namespace N1 {
+// n::G is used in namespace N2.
+// Currently, the check doesn't support multiple scopes. All the relevant
+// using-decls will be marked as used once we see an usage even the usage is in
+// other scope.
+using n::G;
+}
+
+namespace N2 {
+using n::G;
+void f(G g);
+}
+
+void IgnoreFunctionScope() {
+// Using-decls defined in function scope will be ignored.
+using n::H;
+}
+
 // ----- 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;
 }
-