Support variables and functions types in misc-unused-using-decls.

[hokein]

Summary

Fix PR27429.

[hokein]

Fix code style.

[djasper]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

63 ↗

(On Diff #56412)

Use early return here instead of "else".

65 ↗

(On Diff #56412)

No braces for single statement ifs.

68 ↗

(On Diff #56412)

Wouldn't you also want to do this via the canonical decl? Add a test.

71 ↗

(On Diff #56412)

Would it be important to look at something like the canonical decl here? Can you add a test with a (static) class variable that is initialized out of line?

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

60 ↗

(On Diff #56412)

Can you add tests where variables and functions are solely referenced by pointers to members? Compare:
http://reviews.llvm.org/D20054

[hokein]

• Address review comments.
• Ignore using-declarations defined in class definition.

[hokein]

I ran the check on LLVM project. The check found 8 hits: http://reviews.llvm.org/D20060 (Check all of them, no false positives).

[djasper]

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

39 ↗

(On Diff #56540)

We should also ignore using declarations in function context (Koenig lookup..).

clang-tidy/misc/UnusedUsingDeclsCheck.h

33 ↗

(On Diff #56540)

nit: Add an empty line after this.

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

27 ↗

(On Diff #56540)

Move this into the "Using declarations" section.

[hokein]

Update.

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

39 ↗

(On Diff #56540)

If we ignore them in function context, we will also skip the following case:

namespace a {
class A {};
};

void func() {
  using a::A; // unused-using decls.
}

[djasper]

Looks good.

clang-tidy/misc/UnusedUsingDeclsCheck.cpp

39 ↗

(On Diff #56555)

Ack. I think this is actually ok for now.

66 ↗

(On Diff #56555)

Maybe the ->getCanonicalDecl() should be moved into removeFromFoundDecls?

[hokein]

Address remaining comments.

clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.h

 public:
   UnusedUsingDeclsCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void onEndOfTranslationUnit() override;
 
 private:
+  void removeFromFoundDecls(const Decl *D);
+
   llvm::DenseMap<const Decl*, const UsingDecl*> FoundDecls;
   llvm::DenseMap<const Decl*, CharSourceRange> FoundRanges;
 };
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_USING_DECLS_H

clang-tools-extra/trunk/clang-tidy/misc/UnusedUsingDeclsCheck.cpp

 namespace tidy {
 namespace misc {
 
 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);
 }
 
 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;
     const auto *TargetDecl =
         Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
 
-    // FIXME: Handle other target types.
-    if (!isa<RecordDecl>(TargetDecl) && !isa<ClassTemplateDecl>(TargetDecl))
+    // Ignores using-declarations defined in class definition.
+    if (isa<CXXRecordDecl>(TargetDecl->getDeclContext()))
+      return;
+
+    if (!isa<RecordDecl>(TargetDecl) && !isa<ClassTemplateDecl>(TargetDecl) &&
+        !isa<FunctionDecl>(TargetDecl) && !isa<VarDecl>(TargetDecl) &&
+        !isa<FunctionTemplateDecl>(TargetDecl))
       return;
 
     FoundDecls[TargetDecl] = Using;
     FoundRanges[TargetDecl] = CharSourceRange::getCharRange(
         Using->getLocStart(),
         Lexer::findLocationAfterToken(
             Using->getLocEnd(), tok::semi, *Result.SourceManager,
             Result.Context->getLangOpts(),
             /*SkipTrailingWhitespaceAndNewLine=*/true));
     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))
       Used = Specialization->getSpecializedTemplate();
-    auto I = FoundDecls.find(Used->getCanonicalDecl());
-    if (I != FoundDecls.end())
-      I->second = nullptr;
-  }
-}
+    removeFromFoundDecls(Used);
+    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);
+    }
+  }
+}
+
+void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
+  auto I = FoundDecls.find(D->getCanonicalDecl());
 
 void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
   for (const auto &FoundDecl : FoundDecls) {
     if (FoundDecl.second == nullptr)
       continue;
     diag(FoundDecl.second->getLocation(), "using decl %0 is unused")
         << FoundDecl.second
         << FixItHint::CreateRemoval(FoundRanges[FoundDecl.first]);
   }
   FoundDecls.clear();
 }
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/test/clang-tidy/misc-unused-using-decls.cpp

 // RUN: %check_clang_tidy %s misc-unused-using-decls %t
 
 // ----- Definitions -----
 template <typename T> class vector {};
 namespace n {
 class A;
 class B;
 class C;
 class D;
 class D { public: static int i; };
 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;
+};
+
+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; }
+
+class ostream {
+public:
+  ostream &operator<<(ostream &(*PF)(ostream &));
+};
+extern ostream cout;
+ostream &endl(ostream &os);
 }
 
 // ----- 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::E; // E
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'E' is unused
 // CHECK-FIXES: {{^}}// E
 using n::F;
+using n::G;
+using n::H;
+using n::I;
+int I::ii = 1;
+class Derived : public n::Base {
+ public:
+  using Base::f;
+};
+using n::UsedInstance;
+using n::UsedFunc;
+using n::UsedTemplateFunc;
+using n::UnusedInstance; // UnusedInstance
+// 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;
 
 // ----- 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;
 }