[clangd] Fix header-guard check for include insertion, and don't index header guards.

Summary:
Both of these attempt to check whether a header guard exists while parsing the
file. However the file is only marked as guarded once clang finishes processing
it. We defer the checks and work until SymbolCollector::finish().

This is ugly and ad-hoc, deferring *all* work might be cleaner.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, mgrang, arphaman, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D61442

llvm-svn: 359880

1 files changed, 40 insertions, 24 deletions

diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index b9f321727d3..af1938dafa8 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -352,9 +352,8 @@ bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
   const auto &SM = PP->getSourceManager();
   auto DefLoc = MI->getDefinitionLoc();
 
-  // Header guards are not interesting in index. Builtin macros don't have
-  // useful locations and are not needed for code completions.
-  if (MI->isUsedForHeaderGuard() || MI->isBuiltinMacro())
+  // Builtin macros don't have useful locations and aren't needed in completion.
+  if (MI->isBuiltinMacro())
     return true;
 
   // Skip main-file symbols if we are not collecting them.
@@ -408,22 +407,25 @@ bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
   std::string Signature;
   std::string SnippetSuffix;
   getSignature(*CCS, &Signature, &SnippetSuffix);
-
-  std::string Include;
-  if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
-    if (auto Header = getIncludeHeader(
-            Name->getName(), SM.getDecomposedExpansionLoc(DefLoc).first))
-      Include = std::move(*Header);
-  }
   S.Signature = Signature;
   S.CompletionSnippetSuffix = SnippetSuffix;
-  if (!Include.empty())
-    S.IncludeHeaders.emplace_back(Include, 1);
 
+  IndexedMacros.insert(Name);
+  setIncludeLocation(S, DefLoc);
   Symbols.insert(S);
   return true;
 }
 
+void SymbolCollector::setIncludeLocation(const Symbol &S,
+                                         SourceLocation Loc) {
+  if (Opts.CollectIncludePath)
+    if (shouldCollectIncludePath(S.SymInfo.Kind))
+      // Use the expansion location to get the #include header since this is
+      // where the symbol is exposed.
+      IncludeFiles[S.ID] =
+          PP->getSourceManager().getDecomposedExpansionLoc(Loc).first;
+}
+
 void SymbolCollector::finish() {
   // At the end of the TU, add 1 to the refcount of all referenced symbols.
   auto IncRef = [this](const SymbolID &ID) {
@@ -440,6 +442,14 @@ void SymbolCollector::finish() {
   }
   if (Opts.CollectMacro) {
     assert(PP);
+    // First, drop header guards. We can't identify these until EOF.
+    for (const IdentifierInfo *II : IndexedMacros) {
+      if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
+        if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
+          if (MI->isUsedForHeaderGuard())
+            Symbols.erase(*ID);
+    }
+    // Now increment refcounts.
     for (const IdentifierInfo *II : ReferencedMacros) {
       if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo())
         if (auto ID = getSymbolID(*II, MI, PP->getSourceManager()))
@@ -447,6 +457,21 @@ void SymbolCollector::finish() {
     }
   }
 
+  // Fill in IncludeHeaders.
+  // We delay this until end of TU so header guards are all resolved.
+  // Symbols in slabs aren' mutable, so insert() has to walk all the strings :-(
+  llvm::SmallString<256> QName;
+  for (const auto &Entry : IncludeFiles)
+    if (const Symbol *S = Symbols.find(Entry.first)) {
+      QName = S->Scope;
+      QName.append(S->Name);
+      if (auto Header = getIncludeHeader(QName, Entry.second)) {
+        Symbol NewSym = *S;
+        NewSym.IncludeHeaders.push_back({*Header, 1});
+        Symbols.insert(NewSym);
+      }
+    }
+
   const auto &SM = ASTCtx->getSourceManager();
   llvm::DenseMap<FileID, std::string> URICache;
   auto GetURI = [&](FileID FID) -> llvm::Optional<std::string> {
@@ -464,7 +489,7 @@ void SymbolCollector::finish() {
     }
     return Found->second;
   };
-
+  // Populate Refs slab from DeclRefs.
   if (auto MainFileURI = GetURI(SM.getMainFileID())) {
     for (const auto &It : DeclRefs) {
       if (auto ID = getSymbolID(It.first)) {
@@ -492,6 +517,7 @@ void SymbolCollector::finish() {
   DeclRefs.clear();
   FilesToIndexCache.clear();
   HeaderIsSelfContainedCache.clear();
+  IncludeFiles.clear();
 }
 
 const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID,
@@ -556,17 +582,6 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID,
   std::string ReturnType = getReturnType(*CCS);
   S.ReturnType = ReturnType;
 
-  std::string Include;
-  if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
-    // Use the expansion location to get the #include header since this is
-    // where the symbol is exposed.
-    if (auto Header = getIncludeHeader(
-            QName, SM.getDecomposedExpansionLoc(ND.getLocation()).first))
-      Include = std::move(*Header);
-  }
-  if (!Include.empty())
-    S.IncludeHeaders.emplace_back(Include, 1);
-
   llvm::Optional<OpaqueType> TypeStorage;
   if (S.Flags & Symbol::IndexedForCodeCompletion) {
     TypeStorage = OpaqueType::fromCompletionResult(*ASTCtx, SymbolCompletion);
@@ -575,6 +590,7 @@ const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID,
   }
 
   Symbols.insert(S);
+  setIncludeLocation(S, ND.getLocation());
   return Symbols.find(S.ID);
 }