[clangd] Rollforward include-cleaner library usage in symbol collector.

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

clang-tools-extra/clangd/index/SymbolCollector.cpp

@@ -821,25 +821,45 @@ void SymbolCollector::setIncludeLocation(const Symbol &S, SourceLocation DefLoc,
   // Use the expansion location to get the #include header since this is
   // where the symbol is exposed.
   IncludeFiles[S.ID] = SM.getDecomposedExpansionLoc(DefLoc).first;
+
+  // We update providers for a symbol with each occurence, as SymbolCollector
+  // might run while parsing, rather than at the end of a translation unit.
+  // Hence we see more and more redecls over time.
+  auto [It, Inserted] = SymbolProviders.try_emplace(S.ID);
+  auto Headers =
+      include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes);
+  if (Headers.empty())
+    return;
+
+  auto *HeadersIter = Headers.begin();
+  include_cleaner::Header H = *HeadersIter;
+  while (HeadersIter != Headers.end() &&
+         H.kind() == include_cleaner::Header::Physical &&
+         !tooling::isSelfContainedHeader(H.physical(), SM,
+                                         PP->getHeaderSearchInfo())) {
+    H = *HeadersIter;
+    HeadersIter++;
+  }
+  It->second = H;
 }
 
 llvm::StringRef getStdHeader(const Symbol *S, const LangOptions &LangOpts) {
   tooling::stdlib::Lang Lang = tooling::stdlib::Lang::CXX;
-    if (LangOpts.C11)
-      Lang = tooling::stdlib::Lang::C;
-    else if(!LangOpts.CPlusPlus)
-      return "";
-
-    if (S->Scope == "std::" && S->Name == "move") {
-      if (!S->Signature.contains(','))
-        return "<utility>";
-      return "<algorithm>";
-    }
-
-    if (auto StdSym = tooling::stdlib::Symbol::named(S->Scope, S->Name, Lang))
-     if (auto Header = StdSym->header())
-       return Header->name();
+  if (LangOpts.C11)
+    Lang = tooling::stdlib::Lang::C;
+  else if(!LangOpts.CPlusPlus)
     return "";
+
+  if (S->Scope == "std::" && S->Name == "move") {
+    if (!S->Signature.contains(','))
+      return "<utility>";
+    return "<algorithm>";
+  }
+
+  if (auto StdSym = tooling::stdlib::Symbol::named(S->Scope, S->Name, Lang))
+    if (auto Header = StdSym->header())
+      return Header->name();
+  return "";
 }
 
 void SymbolCollector::finish() {
@@ -865,13 +885,16 @@ void SymbolCollector::finish() {
     }
   }
   llvm::DenseMap<FileID, bool> FileToContainsImportsOrObjC;
+  llvm::DenseMap<include_cleaner::Header, std::string> HeaderSpelling;
   // Fill in IncludeHeaders.
   // We delay this until end of TU so header guards are all resolved.
-  for (const auto &[SID, FID] : IncludeFiles) {
+  for (const auto &[SID, OptionalProvider] : SymbolProviders) {
     const Symbol *S = Symbols.find(SID);
     if (!S)
       continue;
+    assert(IncludeFiles.find(SID) != IncludeFiles.end());
 
+    const auto FID = IncludeFiles.at(SID);
     // Determine if the FID is #include'd or #import'ed.
     Symbol::IncludeDirective Directives = Symbol::Invalid;
     auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind);
@@ -891,20 +914,61 @@ void SymbolCollector::finish() {
     if (Directives == Symbol::Invalid)
       continue;
 
-    // FIXME: Use the include-cleaner library instead.
-    llvm::StringRef IncludeHeader = getStdHeader(S, ASTCtx->getLangOpts());
-    if (IncludeHeader.empty())
-      IncludeHeader = HeaderFileURIs->getIncludeHeader(FID);
+    // Use the include location-based logic for Objective-C symbols.
+    if (Directives & Symbol::Import) {
+      llvm::StringRef IncludeHeader = getStdHeader(S, ASTCtx->getLangOpts());
+      if (IncludeHeader.empty())
+        IncludeHeader = HeaderFileURIs->getIncludeHeader(FID);
+
+      if (!IncludeHeader.empty()) {
+        auto NewSym = *S;
+        NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives});
+        Symbols.insert(NewSym);
+      }
+      // FIXME: use providers from include-cleaner library once it's polished
+      // for Objective-C.
+      continue;
+    }
+
+    assert(Directives == Symbol::Include);
+    // For #include's, use the providers computed by the include-cleaner
+    // library.
+    if (!OptionalProvider)
+      continue;
+    const auto &H = *OptionalProvider;
+    const auto [SpellingIt, Inserted] = HeaderSpelling.try_emplace(H);
+    if (Inserted) {
+      auto &SM = ASTCtx->getSourceManager();
+      if (H.kind() == include_cleaner::Header::Kind::Physical) {
+        // FIXME: Get rid of this once include-cleaner has support for system
+        // headers.
+        if (auto Canonical =
+                HeaderFileURIs->mapCanonical(H.physical()->getName());
+            !Canonical.empty())
+          SpellingIt->second = Canonical;
+        // For physical files, prefer URIs as spellings might change
+        // depending on the translation unit.
+        else if (tooling::isSelfContainedHeader(H.physical(), SM,
+                                                PP->getHeaderSearchInfo()))
+          SpellingIt->second =
+              HeaderFileURIs->toURI(H.physical()->getLastRef());
+      } else {
+        SpellingIt->second = include_cleaner::spellHeader(
+            {H, PP->getHeaderSearchInfo(),
+             SM.getFileEntryForID(SM.getMainFileID())});
+      }
+    }
 
-    if (!IncludeHeader.empty()) {
+    if (!SpellingIt->second.empty()) {
       auto NewSym = *S;
-      NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives});
+      NewSym.IncludeHeaders.push_back({SpellingIt->second, 1, Directives});
       Symbols.insert(NewSym);
     }
   }
 
   ReferencedSymbols.clear();
   IncludeFiles.clear();
+  SymbolProviders.clear();
   FilesWithObjCConstructs.clear();
 }
 

clang-tools-extra/clangd/index/SymbolCollector.h

@@ -175,6 +175,10 @@ class SymbolCollector : public index::IndexDataConsumer {
   void setIncludeLocation(const Symbol &S, SourceLocation,
                           const include_cleaner::Symbol &Sym);
 
+  // Providers for Symbol.IncludeHeaders.
+  // The final spelling is calculated in finish().
+  llvm::DenseMap<SymbolID, std::optional<include_cleaner::Header>>
+      SymbolProviders;
   // Files which contain ObjC symbols.
   // This is finalized and used in finish().
   llvm::DenseSet<FileID> FilesWithObjCConstructs;

clang-tools-extra/clangd/unittests/FileIndexTests.cpp

@@ -239,7 +239,7 @@ TEST(FileIndexTest, IncludeCollected) {
               "<the/good/header.h>");
 }
 
-TEST(FileIndexTest, DISABLED_IWYUPragmaExport) {
+TEST(FileIndexTest, IWYUPragmaExport) {
   FileIndex M;
 
   TestTU File;

clang-tools-extra/clangd/unittests/IndexActionTests.cpp

@@ -8,11 +8,15 @@
 
 #include "Headers.h"
 #include "TestFS.h"
+#include "URI.h"
 #include "index/IndexAction.h"
 #include "index/Serialization.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Tooling.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <string>
 
 namespace clang {
 namespace clangd {
@@ -40,6 +44,11 @@ MATCHER(hasSameURI, "") {
   return toUri(Path) == URI;
 }
 
+MATCHER_P(includeHeader, P, "") {
+  return (arg.IncludeHeaders.size() == 1) &&
+         (arg.IncludeHeaders.begin()->IncludeHeader == P);
+}
+
 ::testing::Matcher<const IncludeGraphNode &>
 includesAre(const std::vector<std::string> &Includes) {
   return ::testing::Field(&IncludeGraphNode::DirectIncludes,
@@ -312,6 +321,26 @@ TEST_F(IndexActionTest, SkipNestedSymbols) {
   EXPECT_THAT(*IndexFile.Symbols, testing::Contains(hasName("Bar")));
   EXPECT_THAT(*IndexFile.Symbols, Not(testing::Contains(hasName("Baz"))));
 }
+
+TEST_F(IndexActionTest, SymbolFromCC) {
+  std::string MainFilePath = testPath("main.cpp");
+  addFile(MainFilePath, R"cpp(
+ #include "main.h"
+ void foo() {}
+ )cpp");
+  addFile(testPath("main.h"), R"cpp(
+ #pragma once
+ void foo();
+ )cpp");
+  Opts.FileFilter = [](const SourceManager &SM, FileID F) {
+    return !SM.getFileEntryRefForID(F)->getName().endswith("main.h");
+  };
+  IndexFileIn IndexFile = runIndexingAction(MainFilePath, {"-std=c++14"});
+  EXPECT_THAT(*IndexFile.Symbols,
+              UnorderedElementsAre(AllOf(
+                  hasName("foo"),
+                  includeHeader(URI::create(testPath("main.h")).toString()))));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

@@ -14,6 +14,7 @@
 #include "index/SymbolCollector.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexingOptions.h"
@@ -1554,6 +1555,8 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
         // Move overloads have special handling.
         template <typename _T> T&& move(_T&& __value);
         template <typename _I, typename _O> _O move(_I, _I, _O);
+        template <typename _T, typename _O, typename _I> _O move(
+          _T&&, _O, _O, _I);
       }
       )cpp",
       /*Main=*/"");
@@ -1565,7 +1568,8 @@ TEST_F(SymbolCollectorTest, CanonicalSTLHeader) {
                 includeHeader("<string>")),
           // Parameter names are demangled.
           AllOf(labeled("move(T &&value)"), includeHeader("<utility>")),
-          AllOf(labeled("move(I, I, O)"), includeHeader("<algorithm>"))));
+          AllOf(labeled("move(I, I, O)"), includeHeader("<algorithm>")),
+          AllOf(labeled("move(T &&, O, O, I)"), includeHeader("<algorithm>"))));
 }
 
 TEST_F(SymbolCollectorTest, IWYUPragma) {
@@ -1592,7 +1596,7 @@ TEST_F(SymbolCollectorTest, IWYUPragmaWithDoubleQuotes) {
                                  includeHeader("\"the/good/header.h\""))));
 }
 
-TEST_F(SymbolCollectorTest, DISABLED_IWYUPragmaExport) {
+TEST_F(SymbolCollectorTest, IWYUPragmaExport) {
   CollectorOpts.CollectIncludePath = true;
   const std::string Header = R"cpp(#pragma once
     #include "exporter.h"
@@ -1978,6 +1982,24 @@ TEST_F(SymbolCollectorTest, Concepts) {
                   qName("A"), hasKind(clang::index::SymbolKind::Concept))));
 }
 
+TEST_F(SymbolCollectorTest, IncludeHeaderForwardDecls) {
+  CollectorOpts.CollectIncludePath = true;
+  const std::string Header = R"cpp(#pragma once 
+struct Foo;
+#include "full.h"
+)cpp";
+  auto FullFile = testPath("full.h");
+  InMemoryFileSystem->addFile(FullFile, 0,
+                              llvm::MemoryBuffer::getMemBuffer(R"cpp(
+#pragma once 
+struct Foo {};)cpp"));
+  runSymbolCollector(Header, /*Main=*/"",
+                     /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(
+                           qName("Foo"),
+                           includeHeader(URI::create(FullFile).toString()))))
+      << *Symbols.begin();
+}
 } // namespace
 } // namespace clangd
 } // namespace clang