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 @@ -18,6 +18,7 @@ #include "index/Serialization.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Support/SHA1.h" #include "llvm/Support/Threading.h" #include 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" @@ -130,6 +136,27 @@ } return AbsolutePath; } + +void storeSlabs(BackgroundIndexStorage *IndexStorage, PathRef Identifier, + IndexFileOut Out) { + if (!IndexStorage) + return; + if (auto Error = IndexStorage->storeShard(Identifier, Out)) + elog("Failed to write background-index shard for file {0}: {1}", Identifier, + std::move(Error)); +} + +void storeSlabs(BackgroundIndexStorage *IndexStorage, PathRef Identifier, + SymbolSlab *Syms, RefSlab *Refs, RelationSlab *Rels, + IncludeGraph *Sources) { + IndexFileOut Out; + Out.Symbols = Syms; + Out.Refs = Refs; + Out.Relations = Rels; + Out.Sources = Sources; + storeSlabs(IndexStorage, Identifier, Out); +} + } // namespace BackgroundIndex::BackgroundIndex( @@ -347,19 +374,11 @@ 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. - if (IndexStorage) { - IndexFileOut Shard; - Shard.Symbols = SS.get(); - Shard.Refs = RS.get(); - Shard.Relations = RelS.get(); - Shard.Sources = IG.get(); - - if (auto Error = IndexStorage->storeShard(Path, Shard)) - elog("Failed to write background-index shard for file {0}: {1}", Path, - std::move(Error)); - } + storeSlabs(IndexStorage, Path, SS.get(), RS.get(), RelS.get(), IG.get()); + { std::lock_guard Lock(DigestsMu); auto Hash = FileIt.second.Digest; @@ -460,12 +479,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,6 +490,19 @@ SPAN_ATTACH(Tracer, "refs", int(Index.Refs->numRefs())); SPAN_ATTACH(Tracer, "sources", int(Index.Sources->size())); + if (Clang->hasDiagnostics() && + Clang->getDiagnostics().hasUncompilableErrorOccurred()) { + // In case of failures, we don't shard index into files but rather save + // everything for the 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. + storeSlabs(IndexStorage, AbsolutePath, std::move(Index)); + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "IndexingAction failed: has uncompilable errors"); + } + update(AbsolutePath, std::move(Index), DigestsSnapshot, IndexStorage); if (BuildIndexPeriodMs > 0) @@ -568,6 +594,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,12 @@ //===----------------------------------------------------------------------===// #include "IndexAction.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,17 +151,26 @@ void EndSourceFileAction() override { WrapperFrontendAction::EndSourceFileAction(); + SymbolSlab Syms; + RefSlab Refs; + RelationSlab Rels; + const auto &CI = getCompilerInstance(); if (CI.hasDiagnostics() && CI.getDiagnostics().hasUncompilableErrorOccurred()) { llvm::errs() << "Skipping TU due to uncompilable errors\n"; - return; + } else { + // Return empty information for non-compilable TUs. + Syms = Collector->takeSymbols(); + Refs = Collector->takeRefs(); + Rels = Collector->takeRelations(); } - SymbolsCallback(Collector->takeSymbols()); + + SymbolsCallback(std::move(Syms)); if (RefsCallback != nullptr) - RefsCallback(Collector->takeRefs()); + RefsCallback(std::move(Refs)); if (RelationsCallback != nullptr) - RelationsCallback(Collector->takeRelations()); + RelationsCallback(std::move(Rels)); if (IncludeGraphCallback != nullptr) { #ifndef NDEBUG // This checks if all nodes are initialized. 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 @@ -12,6 +12,7 @@ using ::testing::AllOf; using ::testing::Contains; using ::testing::ElementsAre; +using ::testing::IsEmpty; using ::testing::Not; using ::testing::UnorderedElementsAre; @@ -491,5 +492,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()); + + EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"))); + + auto Shard = MSS.loadShard(testPath("A.cc")); + EXPECT_THAT(*Shard->Symbols, IsEmpty()); + EXPECT_THAT(Shard->Sources->keys(), + UnorderedElementsAre("unittest:///A.cc", "unittest:///A.h", + "unittest:///B.h", "unittest:///C.h")); +} + } // namespace clangd } // namespace clang