hokein updated this revision to Diff 56579.
hokein marked an inline comment as done.
hokein added a comment.

Address comments.


http://reviews.llvm.org/D19816

Files:
  clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tidy/misc/UnusedUsingDeclsCheck.h
  include-fixer/find-all-symbols/CMakeLists.txt
  include-fixer/find-all-symbols/FindAllSymbols.cpp
  include-fixer/find-all-symbols/FindAllSymbols.h
  include-fixer/find-all-symbols/HeaderMapCollector.h
  include-fixer/find-all-symbols/PragmaCommentHandler.cpp
  include-fixer/find-all-symbols/PragmaCommentHandler.h
  include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
  test/clang-tidy/misc-unused-using-decls.cpp
  unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Index: unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
===================================================================
--- unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
+++ unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
@@ -8,11 +8,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "FindAllSymbols.h"
+#include "HeaderMapCollector.h"
+#include "PragmaCommentHandler.h"
 #include "SymbolInfo.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
@@ -62,6 +65,41 @@
   std::vector<SymbolInfo> Symbols;
 };
 
+class TestFindAllSymbolsAction : public clang::ASTFrontendAction {
+public:
+  TestFindAllSymbolsAction(FindAllSymbols::ResultReporter *Reporter)
+      : MatchFinder(), Collector(), Handler(&Collector),
+        Matcher(Reporter, &Collector) {
+    Matcher.registerMatchers(&MatchFinder);
+  }
+
+  std::unique_ptr<clang::ASTConsumer>
+  CreateASTConsumer(clang::CompilerInstance &Compiler,
+                    StringRef InFile) override {
+    Compiler.getPreprocessor().addCommentHandler(&Handler);
+    return MatchFinder.newASTConsumer();
+  }
+
+private:
+  ast_matchers::MatchFinder MatchFinder;
+  HeaderMapCollector Collector;
+  PragmaCommentHandler Handler;
+  FindAllSymbols Matcher;
+};
+
+class TestFindAllSymbolsActionFactory
+    : public clang::tooling::FrontendActionFactory {
+public:
+  TestFindAllSymbolsActionFactory(MockReporter *Reporter)
+      : Reporter(Reporter) {}
+  clang::FrontendAction *create() override {
+    return new TestFindAllSymbolsAction(Reporter);
+  }
+
+private:
+  MockReporter *const Reporter;
+};
+
 class FindAllSymbolsTest : public ::testing::Test {
 public:
   bool hasSymbol(const SymbolInfo &Symbol) {
@@ -73,18 +111,16 @@
   }
 
   bool runFindAllSymbols(StringRef Code) {
-    FindAllSymbols matcher(&Reporter);
-    clang::ast_matchers::MatchFinder MatchFinder;
-    matcher.registerMatchers(&MatchFinder);
-
     llvm::IntrusiveRefCntPtr<vfs::InMemoryFileSystem> InMemoryFileSystem(
         new vfs::InMemoryFileSystem);
     llvm::IntrusiveRefCntPtr<FileManager> Files(
         new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
     std::string FileName = "symbol.cc";
-    std::unique_ptr<clang::tooling::FrontendActionFactory> Factory =
-        clang::tooling::newFrontendActionFactory(&MatchFinder);
+
+    std::unique_ptr<clang::tooling::FrontendActionFactory> Factory(
+        new TestFindAllSymbolsActionFactory(&Reporter));
+
     tooling::ToolInvocation Invocation(
         {std::string("find_all_symbols"), std::string("-fsyntax-only"),
          FileName},
@@ -371,5 +407,20 @@
   }
 }
 
+TEST_F(FindAllSymbolsTest, IWYUPrivatePragmaTest) {
+  static const char Code[] = R"(
+    // IWYU pragma: private, include "bar.h"
+    struct Bar {
+    };
+  )";
+  runFindAllSymbols(Code);
+
+  {
+    SymbolInfo Symbol =
+        CreateSymbolInfo("Bar", SymbolInfo::Class, "bar.h", 3, {});
+    EXPECT_TRUE(hasSymbol(Symbol));
+  }
+}
+
 } // namespace find_all_symbols
 } // namespace clang
Index: test/clang-tidy/misc-unused-using-decls.cpp
===================================================================
--- test/clang-tidy/misc-unused-using-decls.cpp
+++ test/clang-tidy/misc-unused-using-decls.cpp
@@ -10,6 +10,21 @@
 class D { public: static int i; };
 template <typename T> class E {};
 template <typename T> class 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 -----
@@ -24,12 +39,25 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'E' is unused
 // CHECK-FIXES: {{^}}// E
 using n::F;
+using n::UsedInstance;
+using n::UsedFunc;
+using n::UsedTemplateFunc;
+using n::UnusedInstance;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedInstance' is unused
+using n::UnusedFunc;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'UnusedFunc' is unused
+using n::cout;
+using n::endl;
 
 // ----- Usages -----
 void f(B b);
 void g() {
   vector<C> data;
   D::i = 1;
   F<int> f;
+  UsedInstance.i;
+  UsedFunc();
+  UsedTemplateFunc<int>();
+  cout << endl;
 }
 
Index: include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
===================================================================
--- include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
+++ include-fixer/find-all-symbols/tool/FindAllSymbolsMain.cpp
@@ -8,8 +8,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "FindAllSymbols.h"
+#include "HeaderMapCollector.h"
+#include "PragmaCommentHandler.h"
 #include "SymbolInfo.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -82,6 +88,31 @@
   std::map<std::string, std::set<SymbolInfo>> Symbols;
 };
 
+class FindAllSymbolsAction : public clang::ASTFrontendAction {
+public:
+  FindAllSymbolsAction()
+      : Reporter(), MatchFinder(), Collector(), Handler(&Collector),
+        Matcher(&Reporter, &Collector) {
+    Matcher.registerMatchers(&MatchFinder);
+  }
+
+  std::unique_ptr<clang::ASTConsumer>
+  CreateASTConsumer(clang::CompilerInstance &Compiler,
+                    StringRef InFile) override {
+    Compiler.getPreprocessor().addCommentHandler(&Handler);
+    return MatchFinder.newASTConsumer();
+  }
+
+  void EndSourceFileAction() override { Reporter.Write(OutputDir); }
+
+private:
+  YamlReporter Reporter;
+  clang::ast_matchers::MatchFinder MatchFinder;
+  HeaderMapCollector Collector;
+  PragmaCommentHandler Handler;
+  FindAllSymbols Matcher;
+};
+
 bool Merge(llvm::StringRef MergeDir, llvm::StringRef OutputFile) {
   std::error_code EC;
   std::set<SymbolInfo> UniqueSymbols;
@@ -142,11 +173,8 @@
     return 0;
   }
 
-  clang::find_all_symbols::YamlReporter Reporter;
-  clang::find_all_symbols::FindAllSymbols Matcher(&Reporter);
-  clang::ast_matchers::MatchFinder MatchFinder;
-  Matcher.registerMatchers(&MatchFinder);
-  Tool.run(newFrontendActionFactory(&MatchFinder).get());
-  Reporter.Write(OutputDir);
+  Tool.run(
+      newFrontendActionFactory<clang::find_all_symbols::FindAllSymbolsAction>()
+          .get());
   return 0;
 }
Index: include-fixer/find-all-symbols/PragmaCommentHandler.h
===================================================================
--- /dev/null
+++ include-fixer/find-all-symbols/PragmaCommentHandler.h
@@ -0,0 +1,41 @@
+//===-- PragmaCommentHandler.h - find all symbols----------------*- 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_FIND_ALL_SYMBOLS_PRAGMA_COMMENT_HANDLER_H
+#define LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_PRAGMA_COMMENT_HANDLER_H
+
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/Preprocessor.h"
+#include <map>
+
+namespace clang {
+namespace find_all_symbols {
+
+class HeaderMapCollector;
+
+/// \brief PragmaCommentHandler parses pragma comment on include files to
+/// determine when we should include a different header from the header that
+/// directly defines a symbol.
+///
+/// Currently it only supports IWYU private pragma:
+/// https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-private
+class PragmaCommentHandler : public clang::CommentHandler {
+public:
+  PragmaCommentHandler(HeaderMapCollector *Collector) : Collector(Collector) {}
+
+  bool HandleComment(Preprocessor &PP, SourceRange Range) override;
+
+private:
+  HeaderMapCollector *const Collector;
+};
+
+} // namespace find_all_symbols
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_PRAGMA_COMMENT_HANDLER_H
Index: include-fixer/find-all-symbols/PragmaCommentHandler.cpp
===================================================================
--- /dev/null
+++ include-fixer/find-all-symbols/PragmaCommentHandler.cpp
@@ -0,0 +1,37 @@
+//===-- PragmaCommentHandler.cpp - find all symbols -----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "PragmaCommentHandler.h"
+#include "FindAllSymbols.h"
+#include "HeaderMapCollector.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace find_all_symbols {
+namespace {
+const char IWYUPragma[] = "// IWYU pragma: private, include ";
+} // namespace
+
+bool PragmaCommentHandler::HandleComment(Preprocessor &PP, SourceRange Range) {
+  StringRef Text =
+      Lexer::getSourceText(CharSourceRange::getCharRange(Range),
+                           PP.getSourceManager(), PP.getLangOpts());
+  size_t Pos = Text.find(IWYUPragma);
+  if (Pos == StringRef::npos)
+    return false;
+  StringRef RemappingFilePath = Text.substr(Pos + std::strlen(IWYUPragma));
+  Collector->addHeaderMapping(
+      PP.getSourceManager().getFilename(Range.getBegin()),
+      RemappingFilePath.trim("\"<>"));
+  return false;
+}
+
+} // namespace find_all_symbols
+} // namespace clang
Index: include-fixer/find-all-symbols/HeaderMapCollector.h
===================================================================
--- /dev/null
+++ include-fixer/find-all-symbols/HeaderMapCollector.h
@@ -0,0 +1,38 @@
+//===-- HeaderMapCoolector.h - find all symbols------------------*- 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_FIND_ALL_SYMBOLS_HEADER_MAP_COLLECTOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_HEADER_MAP_COLLECTOR_H
+
+#include "llvm/ADT/StringMap.h"
+#include <string>
+
+namespace clang {
+namespace find_all_symbols {
+
+/// \brief HeaderMappCollector collects all remapping header files.
+class HeaderMapCollector {
+public:
+  typedef llvm::StringMap<std::string> HeaderMap;
+
+  void addHeaderMapping(llvm::StringRef OrignalHeaderPath,
+                        llvm::StringRef MappingHeaderPath) {
+    HeaderMappingTable[OrignalHeaderPath] = MappingHeaderPath;
+  };
+  const HeaderMap &getHeaderMappingTable() { return HeaderMappingTable; };
+
+private:
+  /// A string-to-string map saving the mapping relationship.
+  HeaderMap HeaderMappingTable;
+};
+
+} // namespace find_all_symbols
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_FIND_ALL_SYMBOLS_HEADER_MAP_COLLECTOR_H
Index: include-fixer/find-all-symbols/FindAllSymbols.h
===================================================================
--- include-fixer/find-all-symbols/FindAllSymbols.h
+++ include-fixer/find-all-symbols/FindAllSymbols.h
@@ -12,11 +12,15 @@
 
 #include "SymbolInfo.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include <map>
 #include <string>
 
 namespace clang {
 namespace find_all_symbols {
 
+class HeaderMapCollector;
+
 /// \brief FindAllSymbols collects all classes, free standing functions and
 /// global variables with some extra information such as the path of the header
 /// file, the namespaces they are contained in, the type of variables and the
@@ -39,15 +43,18 @@
                               const SymbolInfo &Symbol) = 0;
   };
 
-  explicit FindAllSymbols(ResultReporter *Reporter) : Reporter(Reporter) {}
+  explicit FindAllSymbols(ResultReporter *Reporter,
+                          HeaderMapCollector *Collector)
+      : Reporter(Reporter), Collector(Collector) {}
 
   void registerMatchers(clang::ast_matchers::MatchFinder *MatchFinder);
 
   void
   run(const clang::ast_matchers::MatchFinder::MatchResult &result) override;
 
 private:
   ResultReporter *const Reporter;
+  HeaderMapCollector *const Collector;
 };
 
 } // namespace find_all_symbols
Index: include-fixer/find-all-symbols/FindAllSymbols.cpp
===================================================================
--- include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -8,6 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "FindAllSymbols.h"
+#include "HeaderMapCollector.h"
 #include "SymbolInfo.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
@@ -43,8 +44,9 @@
   }
 }
 
-bool SetCommonInfo(const MatchFinder::MatchResult &Result,
-                   const NamedDecl *ND, SymbolInfo *Symbol) {
+bool SetCommonInfo(const MatchFinder::MatchResult &Result, const NamedDecl *ND,
+                   SymbolInfo *Symbol,
+                   const HeaderMapCollector::HeaderMap &HeaderMappingTable) {
   SetContext(ND, Symbol);
 
   Symbol->Name = ND->getNameAsString();
@@ -64,6 +66,13 @@
   if (FilePath.empty())
     return false;
 
+  // Check pragma remapping header.
+  auto Iter = HeaderMappingTable.find(FilePath);
+  if (Iter != HeaderMappingTable.end()) {
+    Symbol->FilePath = Iter->second;
+    return true;
+  }
+
   llvm::SmallString<128> AbsolutePath;
   if (llvm::sys::path::is_absolute(FilePath)) {
     AbsolutePath = FilePath;
@@ -174,7 +183,7 @@
   const SourceManager *SM = Result.SourceManager;
 
   SymbolInfo Symbol;
-  if (!SetCommonInfo(Result, ND, &Symbol))
+  if (!SetCommonInfo(Result, ND, &Symbol, Collector->getHeaderMappingTable()))
     return;
 
   if (const auto *VD = llvm::dyn_cast<VarDecl>(ND)) {
Index: include-fixer/find-all-symbols/CMakeLists.txt
===================================================================
--- include-fixer/find-all-symbols/CMakeLists.txt
+++ include-fixer/find-all-symbols/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_clang_library(findAllSymbols
   FindAllSymbols.cpp
+  PragmaCommentHandler.cpp
   SymbolInfo.cpp
 
   LINK_LIBS
Index: clang-tidy/misc/UnusedUsingDeclsCheck.h
===================================================================
--- clang-tidy/misc/UnusedUsingDeclsCheck.h
+++ clang-tidy/misc/UnusedUsingDeclsCheck.h
@@ -30,6 +30,7 @@
   void onEndOfTranslationUnit() override;
 
 private:
+  void removeFromFoundDecls(const Decl *D);
   llvm::DenseMap<const Decl*, const UsingDecl*> FoundDecls;
   llvm::DenseMap<const Decl*, CharSourceRange> FoundRanges;
 };
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===================================================================
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -23,6 +23,7 @@
   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) {
@@ -34,8 +35,9 @@
     const auto *TargetDecl =
         Using->shadow_begin()->getTargetDecl()->getCanonicalDecl();
 
-    // FIXME: Handle other target types.
-    if (!isa<RecordDecl>(TargetDecl) && !isa<ClassTemplateDecl>(TargetDecl))
+    if (!isa<RecordDecl>(TargetDecl) && !isa<ClassTemplateDecl>(TargetDecl) &&
+        !isa<FunctionDecl>(TargetDecl) && !isa<VarDecl>(TargetDecl) &&
+        !isa<FunctionTemplateDecl>(TargetDecl))
       return;
 
     FoundDecls[TargetDecl] = Using;
@@ -57,12 +59,26 @@
     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->getCanonicalDecl());
+  } else 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);
+  if (I != FoundDecls.end())
+    I->second = nullptr;
+}
+
 void UnusedUsingDeclsCheck::onEndOfTranslationUnit() {
   for (const auto &FoundDecl : FoundDecls) {
     if (FoundDecl.second == nullptr)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to