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

[hokein]

Summary

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

[lebedev.ri]

Please add a test with #pragma once, which should still be counted as a header guard.

[sammccall]

It's not obvious whether/why this is the right thing to do.

One set of reasoning that occurs to me:

• headers without guards produce code in some context-sensitive way, they only exist in the context of their including file
• for the symbols they produce, we have ways to reconcile across contexts - by symbol ID
• but for refs, there's no such context of identity
• therefore it's not clear whether a ref in an unguarded header should be tracked once, once per include, or not at all. Not at all is defensible.

This falls down a bit though when we see that in practice we'll often have refs to the same symbol, from the same location, with the same role - this is a reasonable way to reconcile.

If this is just a way to trim down index size, we should consider codebases other than LLVM, as it seems pretty likely the use of generated files there is atypical.

clangd/index/SymbolCollector.cpp

479

High-level comment - what are you doing here? Why?

479

Use of "canonical" here and elsewhere isn't obvious to me - does this name come from somewhere?

"Modular" is how I would describe these headers - it's imprecise, but you could define it somewhere.

clangd/index/SymbolCollector.cpp

void SymbolCollector::finish() {
   if (Opts.CollectMacro) {
     assert(PP);
     for (const IdentifierInfo *II : ReferencedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
           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);
     if (Found == URICache.end()) {
       if (auto *FileEntry = SM.getFileEntryForID(FID)) {
         auto FileURI = toURI(SM, FileEntry->getName(), Opts);
         if (!FileURI) {
           log("Failed to create URI for file: {0}\n", FileEntry);
auto GetURI = [&](FileID FID) -> Optional<std::string> {
     return Found->second;
   };
 
   if (auto MainFileURI = GetURI(SM.getMainFileID())) {
     for (const auto &It : DeclRefs) {
       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());
             Ref R;
             R.Location.Start = Range.first;
             R.Location.End = Range.second;
             R.Location.FileURI = *FileURI;
             R.Kind = toRefKind(LocAndRole.second);

unittests/clangd/SymbolCollectorTests.cpp

TEST_F(SymbolCollectorTest, Refs) {
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _))));
   EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _))));
 }
 
 TEST_F(SymbolCollectorTest, RefsInHeaders) {
   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;
     class X {};
     class Y;
     class Z {}; // not used anywhere
     Y* y = nullptr;  // used in header doesn't count
     #define GLOBAL_Z(name) Z name;