[PATCH] D53808: [clangd] Don't collect refs from non-canonical headers.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.

With this patch, we only collect refs for canonical headers (headers
with header guards). As non-canonical headers are usually generated, and
not interesting to users.

For LLVM, we have ~2.8 million refs (out of 6 million) for generated inc
headers.

|           | Before | After |
| file size | 78 MB  | 55MB  |
| memory    | 330MB  | 260MB |


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53808

Files:
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -486,13 +486,29 @@
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   Annotations Header(R"(
+  #ifndef HEAD_H_
+  #define HEAD_H_
   class [[Foo]] {};
+  #endif
   )");
   runSymbolCollector(Header.code(), "");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
                                   HaveRanges(Header.ranges()))));
 }
 
+TEST_F(SymbolCollectorTest, NoRefsInNonCanonicalHeaders) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+  class [[Foo]] {};
+  inline void f() {
+    [[Foo]] foo;
+  }
+  )");
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "Foo").ID, _))));
+}
+
 TEST_F(SymbolCollectorTest, References) {
   const std::string Header = R"(
     class W;
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -475,8 +475,15 @@
           IncRef(*ID);
     }
   }
-
   const auto &SM = ASTCtx->getSourceManager();
+  llvm::DenseSet<FileID> CanonicalHeaders;
+  for (auto &IT : PP->macros()) {
+    auto MI = PP->getMacroInfo(IT.first);
+    if (MI && MI->isUsedForHeaderGuard()) {
+      auto FID = SM.getFileID(MI->getDefinitionLoc());
+      CanonicalHeaders.insert(FID);
+    }
+  }
   DenseMap<FileID, std::string> URICache;
   auto GetURI = [&](FileID FID) -> Optional<std::string> {
     auto Found = URICache.find(FID);
@@ -503,6 +510,9 @@
       if (auto ID = getSymbolID(It.first)) {
         for (const auto &LocAndRole : It.second) {
           auto FileID = SM.getFileID(LocAndRole.first);
+          // Only collect refs from the main file and canonical headers.
+          if (FileID != SM.getMainFileID() && !CanonicalHeaders.count(FileID))
+            continue;
           if (auto FileURI = GetURI(FileID)) {
             auto Range =
                 getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());



Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -486,13 +486,29 @@
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = true;
   Annotations Header(R"(
+  #ifndef HEAD_H_
+  #define HEAD_H_
   class [[Foo]] {};
+  #endif
   )");
   runSymbolCollector(Header.code(), "");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID,
                                   HaveRanges(Header.ranges()))));
 }
 
+TEST_F(SymbolCollectorTest, NoRefsInNonCanonicalHeaders) {
+  CollectorOpts.RefFilter = RefKind::All;
+  CollectorOpts.RefsInHeaders = true;
+  Annotations Header(R"(
+  class [[Foo]] {};
+  inline void f() {
+    [[Foo]] foo;
+  }
+  )");
+  runSymbolCollector(Header.code(), "");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "Foo").ID, _))));
+}
+
 TEST_F(SymbolCollectorTest, References) {
   const std::string Header = R"(
     class W;
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -475,8 +475,15 @@
           IncRef(*ID);
     }
   }
-
   const auto &SM = ASTCtx->getSourceManager();
+  llvm::DenseSet<FileID> CanonicalHeaders;
+  for (auto &IT : PP->macros()) {
+    auto MI = PP->getMacroInfo(IT.first);
+    if (MI && MI->isUsedForHeaderGuard()) {
+      auto FID = SM.getFileID(MI->getDefinitionLoc());
+      CanonicalHeaders.insert(FID);
+    }
+  }
   DenseMap<FileID, std::string> URICache;
   auto GetURI = [&](FileID FID) -> Optional<std::string> {
     auto Found = URICache.find(FID);
@@ -503,6 +510,9 @@
       if (auto ID = getSymbolID(It.first)) {
         for (const auto &LocAndRole : It.second) {
           auto FileID = SM.getFileID(LocAndRole.first);
+          // Only collect refs from the main file and canonical headers.
+          if (FileID != SM.getMainFileID() && !CanonicalHeaders.count(FileID))
+            continue;
           if (auto FileURI = GetURI(FileID)) {
             auto Range =
                 getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts());

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits