diff --git a/clang-tools-extra/clangd/index/Background.h b/clang-tools-extra/clangd/index/Background.h --- a/clang-tools-extra/clangd/index/Background.h +++ b/clang-tools-extra/clangd/index/Background.h @@ -98,7 +98,7 @@ /// information on IndexStorage. void update(llvm::StringRef MainFile, IndexFileIn Index, const llvm::StringMap &DigestsSnapshot, - BackgroundIndexStorage *IndexStorage); + BackgroundIndexStorage *IndexStorage, bool HadErrors); // configuration const FileSystemProvider &FSProvider; diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp --- a/clang-tools-extra/clangd/index/Background.cpp +++ b/clang-tools-extra/clangd/index/Background.cpp @@ -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 @@ /// information on IndexStorage. void BackgroundIndex::update(llvm::StringRef MainFile, IndexFileIn Index, const llvm::StringMap &DigestsSnapshot, - BackgroundIndexStorage *IndexStorage) { + BackgroundIndexStorage *IndexStorage, + bool HadErrors) { // Partition symbols/references into files. struct File { llvm::DenseSet Symbols; @@ -283,6 +290,14 @@ 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. + 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 @@ auto RelS = llvm::make_unique(std::move(Relations).build()); auto IG = llvm::make_unique( 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 @@ elog("Failed to write background-index shard for file {0}: {1}", Path, std::move(Error)); } + { std::lock_guard Lock(DigestsMu); auto Hash = FileIt.second.Digest; @@ -460,12 +478,6 @@ 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 @@ 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 @@ 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; } diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -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 namespace clang { namespace clangd { @@ -149,12 +152,6 @@ 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()); diff --git a/clang-tools-extra/clangd/index/Serialization.h b/clang-tools-extra/clangd/index/Serialization.h --- a/clang-tools-extra/clangd/index/Serialization.h +++ b/clang-tools-extra/clangd/index/Serialization.h @@ -62,7 +62,8 @@ 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); diff --git a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp --- a/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp @@ -491,5 +491,40 @@ } } +TEST_F(BackgroundIndexTest, UncompilableFiles) { + MockFSProvider FS; + llvm::StringMap 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