[clang-tools-extra] r365120 - [clangd] Also cache failures while indexing

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 4 02:51:43 PDT 2019


Author: kadircet
Date: Thu Jul  4 02:51:43 2019
New Revision: 365120

URL: http://llvm.org/viewvc/llvm-project?rev=365120&view=rev
Log:
[clangd] Also cache failures while indexing

Summary:
Clangd currently doesn't cache any indexing failures, which results in
retrying those failed files even if their contents haven't changed.

Reviewers: sammccall

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

Tags: #clang

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

Modified:
    clang-tools-extra/trunk/clangd/index/Background.cpp
    clang-tools-extra/trunk/clangd/index/Background.h
    clang-tools-extra/trunk/clangd/index/IndexAction.cpp
    clang-tools-extra/trunk/clangd/index/Serialization.h
    clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=365120&r1=365119&r2=365120&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Jul  4 02:51:43 2019
@@ -9,7 +9,9 @@
 #include "index/Background.h"
 #include "ClangdUnit.h"
 #include "Compiler.h"
+#include "Headers.h"
 #include "Logger.h"
+#include "Path.h"
 #include "SourceCode.h"
 #include "Symbol.h"
 #include "Threading.h"
@@ -17,6 +19,8 @@
 #include "URI.h"
 #include "index/IndexAction.h"
 #include "index/MemIndex.h"
+#include "index/Ref.h"
+#include "index/Relation.h"
 #include "index/Serialization.h"
 #include "index/SymbolCollector.h"
 #include "clang/Basic/SourceLocation.h"
@@ -25,6 +29,8 @@
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/Threading.h"
 
@@ -271,7 +277,8 @@ void BackgroundIndex::enqueueTask(Task T
 /// information on IndexStorage.
 void BackgroundIndex::update(llvm::StringRef MainFile, IndexFileIn Index,
                              const llvm::StringMap<FileDigest> &DigestsSnapshot,
-                             BackgroundIndexStorage *IndexStorage) {
+                             BackgroundIndexStorage *IndexStorage,
+                             bool HadErrors) {
   // Partition symbols/references into files.
   struct File {
     llvm::DenseSet<const Symbol *> Symbols;
@@ -283,6 +290,14 @@ void BackgroundIndex::update(llvm::Strin
   URIToFileCache URICache(MainFile);
   for (const auto &IndexIt : *Index.Sources) {
     const auto &IGN = IndexIt.getValue();
+    // In case of failures, we only store main file of TU. That way we can still
+    // get symbols from headers if some other TU can compile them. Note that
+    // sources do not contain any information regarding missing headers, since
+    // we don't even know what absolute path they should fall in.
+    // FIXME: Also store contents from other files whenever the current contents
+    // for those files are missing or if they had errors before.
+    if (HadErrors && !IGN.IsTU)
+      continue;
     const auto AbsPath = URICache.resolve(IGN.URI);
     const auto DigestIt = DigestsSnapshot.find(AbsPath);
     // File has different contents.
@@ -347,8 +362,10 @@ void BackgroundIndex::update(llvm::Strin
     auto RelS = llvm::make_unique<RelationSlab>(std::move(Relations).build());
     auto IG = llvm::make_unique<IncludeGraph>(
         getSubGraph(URI::create(Path), Index.Sources.getValue()));
+
     // We need to store shards before updating the index, since the latter
     // consumes slabs.
+    // FIXME: Also skip serializing the shard if it is already up-to-date.
     if (IndexStorage) {
       IndexFileOut Shard;
       Shard.Symbols = SS.get();
@@ -360,6 +377,7 @@ void BackgroundIndex::update(llvm::Strin
         elog("Failed to write background-index shard for file {0}: {1}", Path,
              std::move(Error));
     }
+
     {
       std::lock_guard<std::mutex> Lock(DigestsMu);
       auto Hash = FileIt.second.Digest;
@@ -460,12 +478,6 @@ llvm::Error BackgroundIndex::index(tooli
     return Err;
 
   Action->EndSourceFile();
-  if (Clang->hasDiagnostics() &&
-      Clang->getDiagnostics().hasUncompilableErrorOccurred()) {
-    return llvm::createStringError(
-        llvm::inconvertibleErrorCode(),
-        "IndexingAction failed: has uncompilable errors");
-  }
 
   assert(Index.Symbols && Index.Refs && Index.Sources &&
          "Symbols, Refs and Sources must be set.");
@@ -477,7 +489,12 @@ llvm::Error BackgroundIndex::index(tooli
   SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs()));
   SPAN_ATTACH(Tracer, "sources", int(Index.Sources->size()));
 
-  update(AbsolutePath, std::move(Index), DigestsSnapshot, IndexStorage);
+  bool HadErrors = Clang->hasDiagnostics() &&
+                   Clang->getDiagnostics().hasUncompilableErrorOccurred();
+  update(AbsolutePath, std::move(Index), DigestsSnapshot, IndexStorage,
+         HadErrors);
+  if (HadErrors)
+    log("Failed to compile {0}, index may be incomplete", AbsolutePath);
 
   if (BuildIndexPeriodMs > 0)
     SymbolsUpdatedSinceLastIndex = true;
@@ -568,6 +585,8 @@ BackgroundIndex::loadShard(const tooling
         continue;
       }
       // If digests match then dependency doesn't need re-indexing.
+      // FIXME: Also check for dependencies(sources) of this shard and compile
+      // commands for cache invalidation.
       CurDependency.NeedsReIndexing =
           digest(Buf->get()->getBuffer()) != I.getValue().Digest;
     }

Modified: clang-tools-extra/trunk/clangd/index/Background.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.h?rev=365120&r1=365119&r2=365120&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Background.h (original)
+++ clang-tools-extra/trunk/clangd/index/Background.h Thu Jul  4 02:51:43 2019
@@ -98,7 +98,7 @@ private:
   /// information on IndexStorage.
   void update(llvm::StringRef MainFile, IndexFileIn Index,
               const llvm::StringMap<FileDigest> &DigestsSnapshot,
-              BackgroundIndexStorage *IndexStorage);
+              BackgroundIndexStorage *IndexStorage, bool HadErrors);
 
   // configuration
   const FileSystemProvider &FSProvider;

Modified: clang-tools-extra/trunk/clangd/index/IndexAction.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/IndexAction.cpp?rev=365120&r1=365119&r2=365120&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/IndexAction.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/IndexAction.cpp Thu Jul  4 02:51:43 2019
@@ -7,10 +7,13 @@
 //===----------------------------------------------------------------------===//
 
 #include "IndexAction.h"
+#include "Logger.h"
+#include "index/Relation.h"
 #include "index/SymbolOrigin.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/Tooling.h"
+#include <utility>
 
 namespace clang {
 namespace clangd {
@@ -149,12 +152,6 @@ public:
   void EndSourceFileAction() override {
     WrapperFrontendAction::EndSourceFileAction();
 
-    const auto &CI = getCompilerInstance();
-    if (CI.hasDiagnostics() &&
-        CI.getDiagnostics().hasUncompilableErrorOccurred()) {
-      llvm::errs() << "Skipping TU due to uncompilable errors\n";
-      return;
-    }
     SymbolsCallback(Collector->takeSymbols());
     if (RefsCallback != nullptr)
       RefsCallback(Collector->takeRefs());

Modified: clang-tools-extra/trunk/clangd/index/Serialization.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Serialization.h?rev=365120&r1=365119&r2=365120&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Serialization.h (original)
+++ clang-tools-extra/trunk/clangd/index/Serialization.h Thu Jul  4 02:51:43 2019
@@ -62,7 +62,8 @@ struct IndexFileOut {
   IndexFileOut(const IndexFileIn &I)
       : Symbols(I.Symbols ? I.Symbols.getPointer() : nullptr),
         Refs(I.Refs ? I.Refs.getPointer() : nullptr),
-        Relations(I.Relations ? I.Relations.getPointer() : nullptr) {}
+        Relations(I.Relations ? I.Relations.getPointer() : nullptr),
+        Sources(I.Sources ? I.Sources.getPointer() : nullptr) {}
 };
 // Serializes an index file.
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const IndexFileOut &O);

Modified: clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp?rev=365120&r1=365119&r2=365120&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/BackgroundIndexTests.cpp Thu Jul  4 02:51:43 2019
@@ -491,5 +491,40 @@ TEST_F(BackgroundIndexTest, NoDotsInAbsP
   }
 }
 
+TEST_F(BackgroundIndexTest, UncompilableFiles) {
+  MockFSProvider FS;
+  llvm::StringMap<std::string> Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(Context::empty(), FS, CDB,
+                      [&](llvm::StringRef) { return &MSS; });
+
+  tooling::CompileCommand Cmd;
+  FS.Files[testPath("A.h")] = "void foo();";
+  FS.Files[testPath("B.h")] = "#include \"C.h\"\nasdf;";
+  FS.Files[testPath("C.h")] = "";
+  FS.Files[testPath("A.cc")] = R"cpp(
+  #include "A.h"
+  #include "B.h"
+  #include "not_found_header.h"
+
+  void foo() {}
+  )cpp";
+  Cmd.Filename = "../A.cc";
+  Cmd.Directory = testPath("build");
+  Cmd.CommandLine = {"clang++", "../A.cc"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  // Make sure we only store the shard for main file.
+  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc")));
+  auto Shard = MSS.loadShard(testPath("A.cc"));
+  EXPECT_THAT(*Shard->Symbols, UnorderedElementsAre(Named("foo")));
+  EXPECT_THAT(Shard->Sources->keys(),
+              UnorderedElementsAre("unittest:///A.cc", "unittest:///A.h",
+                                   "unittest:///B.h"));
+}
+
 } // namespace clangd
 } // namespace clang




More information about the cfe-commits mailing list