kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman, javed.absar.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Don't invoke parsing callback for preamble if clangd is using a
previously built one.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112137
Files:
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1165,6 +1165,32 @@
Ready.notify();
}
+TEST_F(TUSchedulerTests, OnlyPublishWhenPreambleIsBuilt) {
+ struct PreamblePublishCounter : public ParsingCallbacks {
+ PreamblePublishCounter(int &PreamblePublishCount)
+ : PreamblePublishCount(PreamblePublishCount) {}
+ void onPreamblePublished(PathRef File) override { ++PreamblePublishCount; }
+ int &PreamblePublishCount;
+ };
+
+ int PreamblePublishCount = 0;
+ TUScheduler S(CDB, optsForTest(),
+
std::make_unique<PreamblePublishCounter>(PreamblePublishCount));
+
+ Path File = testPath("foo.cpp");
+ S.update(File, getInputs(File, ""), WantDiagnostics::Auto);
+ S.blockUntilIdle(timeoutSeconds(10));
+ EXPECT_EQ(PreamblePublishCount, 1);
+ // Same contents, no publish.
+ S.update(File, getInputs(File, ""), WantDiagnostics::Auto);
+ S.blockUntilIdle(timeoutSeconds(10));
+ EXPECT_EQ(PreamblePublishCount, 1);
+ // New contents, should publish.
+ S.update(File, getInputs(File, "#define FOO"), WantDiagnostics::Auto);
+ S.blockUntilIdle(timeoutSeconds(10));
+ EXPECT_EQ(PreamblePublishCount, 2);
+}
+
// If a header file is missing from the CDB (or inferred using heuristics), and
// it's included by another open file, then we parse it using that files flags.
TEST_F(TUSchedulerTests, IncluderCache) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -901,15 +901,17 @@
void PreambleThread::build(Request Req) {
assert(Req.CI && "Got preamble request with null compiler invocation");
const ParseInputs &Inputs = Req.Inputs;
+ bool RebuiltPreamble = true;
Status.update([&](TUStatus &Status) {
Status.PreambleActivity = PreambleAction::Building;
});
- auto _ = llvm::make_scope_exit([this, &Req] {
+ auto _ = llvm::make_scope_exit([this, &Req, &RebuiltPreamble] {
ASTPeer.updatePreamble(std::move(Req.CI), std::move(Req.Inputs),
LatestBuild, std::move(Req.CIDiags),
std::move(Req.WantDiags));
- Callbacks.onPreamblePublished(FileName);
+ if (RebuiltPreamble)
+ Callbacks.onPreamblePublished(FileName);
});
if (!LatestBuild || Inputs.ForceRebuild) {
@@ -918,6 +920,7 @@
} else if (isPreambleCompatible(*LatestBuild, Inputs, FileName, *Req.CI)) {
vlog("Reusing preamble version {0} for version {1} of {2}",
LatestBuild->Version, Inputs.Version, FileName);
+ RebuiltPreamble = false;
return;
} else {
vlog("Rebuilding invalidated preamble for {0} version {1} (previous was "
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1165,6 +1165,32 @@
Ready.notify();
}
+TEST_F(TUSchedulerTests, OnlyPublishWhenPreambleIsBuilt) {
+ struct PreamblePublishCounter : public ParsingCallbacks {
+ PreamblePublishCounter(int &PreamblePublishCount)
+ : PreamblePublishCount(PreamblePublishCount) {}
+ void onPreamblePublished(PathRef File) override { ++PreamblePublishCount; }
+ int &PreamblePublishCount;
+ };
+
+ int PreamblePublishCount = 0;
+ TUScheduler S(CDB, optsForTest(),
+ std::make_unique<PreamblePublishCounter>(PreamblePublishCount));
+
+ Path File = testPath("foo.cpp");
+ S.update(File, getInputs(File, ""), WantDiagnostics::Auto);
+ S.blockUntilIdle(timeoutSeconds(10));
+ EXPECT_EQ(PreamblePublishCount, 1);
+ // Same contents, no publish.
+ S.update(File, getInputs(File, ""), WantDiagnostics::Auto);
+ S.blockUntilIdle(timeoutSeconds(10));
+ EXPECT_EQ(PreamblePublishCount, 1);
+ // New contents, should publish.
+ S.update(File, getInputs(File, "#define FOO"), WantDiagnostics::Auto);
+ S.blockUntilIdle(timeoutSeconds(10));
+ EXPECT_EQ(PreamblePublishCount, 2);
+}
+
// If a header file is missing from the CDB (or inferred using heuristics), and
// it's included by another open file, then we parse it using that files flags.
TEST_F(TUSchedulerTests, IncluderCache) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===================================================================
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -901,15 +901,17 @@
void PreambleThread::build(Request Req) {
assert(Req.CI && "Got preamble request with null compiler invocation");
const ParseInputs &Inputs = Req.Inputs;
+ bool RebuiltPreamble = true;
Status.update([&](TUStatus &Status) {
Status.PreambleActivity = PreambleAction::Building;
});
- auto _ = llvm::make_scope_exit([this, &Req] {
+ auto _ = llvm::make_scope_exit([this, &Req, &RebuiltPreamble] {
ASTPeer.updatePreamble(std::move(Req.CI), std::move(Req.Inputs),
LatestBuild, std::move(Req.CIDiags),
std::move(Req.WantDiags));
- Callbacks.onPreamblePublished(FileName);
+ if (RebuiltPreamble)
+ Callbacks.onPreamblePublished(FileName);
});
if (!LatestBuild || Inputs.ForceRebuild) {
@@ -918,6 +920,7 @@
} else if (isPreambleCompatible(*LatestBuild, Inputs, FileName, *Req.CI)) {
vlog("Reusing preamble version {0} for version {1} of {2}",
LatestBuild->Version, Inputs.Version, FileName);
+ RebuiltPreamble = false;
return;
} else {
vlog("Rebuilding invalidated preamble for {0} version {1} (previous was "
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
