kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Clangd currently doesn't cache any indexing failures, which results in
retrying those failed files even if their contents haven't changed.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D63986
Files:
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/index/IndexAction.cpp
clang-tools-extra/clangd/index/Serialization.h
clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ 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<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());
+
+ 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
Index: clang-tools-extra/clangd/index/Serialization.h
===================================================================
--- clang-tools-extra/clangd/index/Serialization.h
+++ 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);
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===================================================================
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ 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 <utility>
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.
Index: clang-tools-extra/clangd/index/Background.h
===================================================================
--- clang-tools-extra/clangd/index/Background.h
+++ 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 <atomic>
Index: clang-tools-extra/clangd/index/Background.cpp
===================================================================
--- clang-tools-extra/clangd/index/Background.cpp
+++ 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<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.
- 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<std::mutex> 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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
