kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Background indexing triggers indexing only on the first TU that
includes a stale dependency.
This patch extends this behavior to also trigger indexing on TUs with errors.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D64718
Files:
clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
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
@@ -181,7 +181,6 @@
void f_b();
class A_CC {};
)cpp";
- std::string A_CC = "";
FS.Files[testPath("root/A.cc")] = R"cpp(
#include "A.h"
void g() { (void)common; }
@@ -678,5 +677,61 @@
EXPECT_EQ(LoRan, 0u);
}
+TEST(BackgroundIndexLoaderTest, IndexAllTUsWithErrors) {
+ MockFSProvider FS;
+ FS.Files[testPath("root/A.h")] = "broken_header";
+ FS.Files[testPath("root/A.cc")] = R"cpp(#include "A.h")cpp";
+ FS.Files[testPath("root/B.cc")] = R"cpp(#include "A.h")cpp";
+
+ llvm::StringMap<std::string> Storage;
+ size_t CacheHits = 0;
+ MemoryShardStorage MSS(Storage, CacheHits);
+
+ auto CmdForFile = [](PathRef RelativePath) {
+ tooling::CompileCommand Cmd;
+ Cmd.Filename = testPath(RelativePath);
+ Cmd.Directory = testPath("root");
+ Cmd.CommandLine = {"clang++", testPath(RelativePath)};
+ return Cmd;
+ };
+ std::vector<Path> TUs = {"root/A.cc", "root/B.cc"};
+
+ {
+ OverlayCDB CDB(/*Base=*/nullptr);
+ for (PathRef RelPath : TUs)
+ CDB.setCompileCommand(RelPath, CmdForFile(RelPath));
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
+ [&](llvm::StringRef) { return &MSS; });
+ Idx.enqueue(TUs);
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ }
+ EXPECT_EQ(CacheHits, 0U);
+ EXPECT_THAT(Storage.keys(),
+ UnorderedElementsAre(testPath("root/A.cc"),
testPath("root/B.cc"),
+ testPath("root/A.h")));
+ for (PathRef RelPath : TUs) {
+ EXPECT_THAT(MSS.loadShard(testPath(RelPath))
+ ->Sources->lookup(("unittest:///" + RelPath).str()),
+ HadErrors());
+ }
+
+ FS.Files[testPath("root/A.h")] = "int broken_header;";
+ {
+ OverlayCDB CDB(/*Base=*/nullptr);
+ for (PathRef RelPath : TUs)
+ CDB.setCompileCommand(RelPath, CmdForFile(RelPath));
+
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
+ [&](llvm::StringRef) { return &MSS; });
+ Idx.enqueue(TUs);
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ }
+ for (PathRef RelPath : TUs) {
+ EXPECT_THAT(MSS.loadShard(testPath(RelPath))
+ ->Sources->lookup(("unittest:///" + RelPath).str()),
+ Not(HadErrors()));
+ }
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
===================================================================
--- clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
+++ clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
@@ -100,8 +100,7 @@
auto LoadResult = loadShard(ShardIdentifier, Storage);
const CachedShard &CS = LoadResult.first;
// Report the file as stale if it is the first time we see it.
- // FIXME: We should still update staleness for main file, if it had errors.
- if (!LoadResult.second && CS.IsStale)
+ if ((!LoadResult.second || MainShard.SI.HadErrors) && CS.IsStale)
IsStale = true;
for (PathRef Edge : CS.Edges) {
if (InQueue.insert(Edge).second)
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -181,7 +181,6 @@
void f_b();
class A_CC {};
)cpp";
- std::string A_CC = "";
FS.Files[testPath("root/A.cc")] = R"cpp(
#include "A.h"
void g() { (void)common; }
@@ -678,5 +677,61 @@
EXPECT_EQ(LoRan, 0u);
}
+TEST(BackgroundIndexLoaderTest, IndexAllTUsWithErrors) {
+ MockFSProvider FS;
+ FS.Files[testPath("root/A.h")] = "broken_header";
+ FS.Files[testPath("root/A.cc")] = R"cpp(#include "A.h")cpp";
+ FS.Files[testPath("root/B.cc")] = R"cpp(#include "A.h")cpp";
+
+ llvm::StringMap<std::string> Storage;
+ size_t CacheHits = 0;
+ MemoryShardStorage MSS(Storage, CacheHits);
+
+ auto CmdForFile = [](PathRef RelativePath) {
+ tooling::CompileCommand Cmd;
+ Cmd.Filename = testPath(RelativePath);
+ Cmd.Directory = testPath("root");
+ Cmd.CommandLine = {"clang++", testPath(RelativePath)};
+ return Cmd;
+ };
+ std::vector<Path> TUs = {"root/A.cc", "root/B.cc"};
+
+ {
+ OverlayCDB CDB(/*Base=*/nullptr);
+ for (PathRef RelPath : TUs)
+ CDB.setCompileCommand(RelPath, CmdForFile(RelPath));
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
+ [&](llvm::StringRef) { return &MSS; });
+ Idx.enqueue(TUs);
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ }
+ EXPECT_EQ(CacheHits, 0U);
+ EXPECT_THAT(Storage.keys(),
+ UnorderedElementsAre(testPath("root/A.cc"), testPath("root/B.cc"),
+ testPath("root/A.h")));
+ for (PathRef RelPath : TUs) {
+ EXPECT_THAT(MSS.loadShard(testPath(RelPath))
+ ->Sources->lookup(("unittest:///" + RelPath).str()),
+ HadErrors());
+ }
+
+ FS.Files[testPath("root/A.h")] = "int broken_header;";
+ {
+ OverlayCDB CDB(/*Base=*/nullptr);
+ for (PathRef RelPath : TUs)
+ CDB.setCompileCommand(RelPath, CmdForFile(RelPath));
+
+ BackgroundIndex Idx(Context::empty(), FS, CDB,
+ [&](llvm::StringRef) { return &MSS; });
+ Idx.enqueue(TUs);
+ ASSERT_TRUE(Idx.blockUntilIdleForTest());
+ }
+ for (PathRef RelPath : TUs) {
+ EXPECT_THAT(MSS.loadShard(testPath(RelPath))
+ ->Sources->lookup(("unittest:///" + RelPath).str()),
+ Not(HadErrors()));
+ }
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
===================================================================
--- clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
+++ clang-tools-extra/clangd/index/BackgroundIndexLoader.cpp
@@ -100,8 +100,7 @@
auto LoadResult = loadShard(ShardIdentifier, Storage);
const CachedShard &CS = LoadResult.first;
// Report the file as stale if it is the first time we see it.
- // FIXME: We should still update staleness for main file, if it had errors.
- if (!LoadResult.second && CS.IsStale)
+ if ((!LoadResult.second || MainShard.SI.HadErrors) && CS.IsStale)
IsStale = true;
for (PathRef Edge : CS.Edges) {
if (InQueue.insert(Edge).second)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
