[PATCH] D83802: [clangd] Config: also propagate in sync (testing) mode
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf7160c0b0c1: [clangd] Config: also propagate in sync
(testing) mode (authored by sammccall).
Herald added a subscriber: jfb.
Changed prior to commit:
https://reviews.llvm.org/D83802?vs=277920=278130#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83802/new/
https://reviews.llvm.org/D83802
Files:
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
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
@@ -864,25 +864,28 @@
}
TEST_F(TUSchedulerTests, Run) {
- auto Opts = optsForTest();
- Opts.ContextProvider = bindPath;
- TUScheduler S(CDB, Opts);
- std::atomic Counter(0);
- S.run("add 1", /*Path=*/"", [&] { ++Counter; });
- S.run("add 2", /*Path=*/"", [&] { Counter += 2; });
- ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
- EXPECT_EQ(Counter.load(), 3);
-
- Notification TaskRun;
- Key TestKey;
- WithContextValue CtxWithKey(TestKey, 10);
- const char *Path = "somepath";
- S.run("props context", Path, [&] {
-EXPECT_EQ(Context::current().getExisting(TestKey), 10);
-EXPECT_EQ(Path, boundPath());
-TaskRun.notify();
- });
- TaskRun.wait();
+ for (bool Sync : {false, true}) {
+auto Opts = optsForTest();
+if (Sync)
+ Opts.AsyncThreadsCount = 0;
+TUScheduler S(CDB, Opts);
+std::atomic Counter(0);
+S.run("add 1", /*Path=*/"", [&] { ++Counter; });
+S.run("add 2", /*Path=*/"", [&] { Counter += 2; });
+ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+EXPECT_EQ(Counter.load(), 3);
+
+Notification TaskRun;
+Key TestKey;
+WithContextValue CtxWithKey(TestKey, 10);
+const char *Path = "somepath";
+S.run("props context", Path, [&] {
+ EXPECT_EQ(Context::current().getExisting(TestKey), 10);
+ EXPECT_EQ(Path, boundPath());
+ TaskRun.notify();
+});
+TaskRun.wait();
+ }
}
TEST_F(TUSchedulerTests, TUStatus) {
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -313,7 +313,7 @@
private:
const GlobalCompilationDatabase
- const Options Opts;
+ Options Opts;
std::unique_ptr Callbacks; // not nullptr
Semaphore Barrier;
llvm::StringMap> Files;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -965,6 +965,7 @@
if (RunSync) {
assert(!Done && "running a task after stop()");
trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName));
+WithContext WithProvidedContext(ContextProvider(FileName));
Task();
return;
}
@@ -1062,9 +1063,7 @@
Status.ASTActivity.K = ASTAction::RunningAction;
Status.ASTActivity.Name = CurrentRequest->Name;
});
- llvm::Optional WithProvidedContext;
- if (ContextProvider)
-WithProvidedContext.emplace(ContextProvider(FileName));
+ WithContext WithProvidedContext(ContextProvider(FileName));
CurrentRequest->Action();
}
@@ -1238,6 +1237,12 @@
Barrier(Opts.AsyncThreadsCount),
IdleASTs(
std::make_unique(Opts.RetentionPolicy.MaxRetainedASTs)) {
+ // Avoid null checks everywhere.
+ if (!Opts.ContextProvider) {
+this->Opts.ContextProvider = [](llvm::StringRef) {
+ return Context::current().clone();
+};
+ }
if (0 < Opts.AsyncThreadsCount) {
PreambleTasks.emplace();
WorkerThreads.emplace();
@@ -1300,16 +1305,16 @@
void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action) {
- if (!PreambleTasks)
+ if (!PreambleTasks) {
+WithContext WithProvidedContext(Opts.ContextProvider(Path));
return Action();
+ }
PreambleTasks->runAsync(Name, [this, Ctx = Context::current().clone(),
Path(Path.str()),
Action = std::move(Action)]() mutable {
std::lock_guard BarrierLock(Barrier);
WithContext WC(std::move(Ctx));
-llvm::Optional WithProvidedContext;
-if (Opts.ContextProvider)
- WithProvidedContext.emplace(Opts.ContextProvider(Path));
+WithContext WithProvidedContext(Opts.ContextProvider(Path));
Action();
});
}
@@ -1344,6 +1349,7 @@
SPAN_ATTACH(Tracer, "file", File);
std::shared_ptr Preamble =
It->second->Worker->getPossiblyStalePreamble();
+WithContext WithProvidedContext(Opts.ContextProvider(File));
[PATCH] D83802: [clangd] Config: also propagate in sync (testing) mode
sammccall marked an inline comment as done. sammccall added a comment. Also added one direct test for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83802/new/ https://reviews.llvm.org/D83802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83802: [clangd] Config: also propagate in sync (testing) mode
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.
thanks, lgtm.
Comment at: clang-tools-extra/clangd/TUScheduler.cpp:1241
+ // Avoid null checks everywhere.
+ if (!Opts.ContextProvider)
+this->Opts.ContextProvider = [](llvm::StringRef) {
nit: braces
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83802/new/
https://reviews.llvm.org/D83802
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83802: [clangd] Config: also propagate in sync (testing) mode
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
javed.absar, ilya-biryukov.
Herald added a project: clang.
Tests are coming (really!).
I hit this while trying to add a config-over-LSP lit test, which I think
is an appropriate way to test this feature.
That needs a few more changes though...
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D83802
Files:
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -313,7 +313,7 @@
private:
const GlobalCompilationDatabase
- const Options Opts;
+ Options Opts;
std::unique_ptr Callbacks; // not nullptr
Semaphore Barrier;
llvm::StringMap> Files;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -965,6 +965,7 @@
if (RunSync) {
assert(!Done && "running a task after stop()");
trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName));
+WithContext WithProvidedContext(ContextProvider(FileName));
Task();
return;
}
@@ -1062,9 +1063,7 @@
Status.ASTActivity.K = ASTAction::RunningAction;
Status.ASTActivity.Name = CurrentRequest->Name;
});
- llvm::Optional WithProvidedContext;
- if (ContextProvider)
-WithProvidedContext.emplace(ContextProvider(FileName));
+ WithContext WithProvidedContext(ContextProvider(FileName));
CurrentRequest->Action();
}
@@ -1238,6 +1237,11 @@
Barrier(Opts.AsyncThreadsCount),
IdleASTs(
std::make_unique(Opts.RetentionPolicy.MaxRetainedASTs)) {
+ // Avoid null checks everywhere.
+ if (!Opts.ContextProvider)
+this->Opts.ContextProvider = [](llvm::StringRef) {
+ return Context::current().clone();
+};
if (0 < Opts.AsyncThreadsCount) {
PreambleTasks.emplace();
WorkerThreads.emplace();
@@ -1300,16 +1304,16 @@
void TUScheduler::run(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action) {
- if (!PreambleTasks)
+ if (!PreambleTasks) {
+WithContext WithProvidedContext(Opts.ContextProvider(Path));
return Action();
+ }
PreambleTasks->runAsync(Name, [this, Ctx = Context::current().clone(),
Path(Path.str()),
Action = std::move(Action)]() mutable {
std::lock_guard BarrierLock(Barrier);
WithContext WC(std::move(Ctx));
-llvm::Optional WithProvidedContext;
-if (Opts.ContextProvider)
- WithProvidedContext.emplace(Opts.ContextProvider(Path));
+WithContext WithProvidedContext(Opts.ContextProvider(Path));
Action();
});
}
@@ -1344,6 +1348,7 @@
SPAN_ATTACH(Tracer, "file", File);
std::shared_ptr Preamble =
It->second->Worker->getPossiblyStalePreamble();
+WithContext WithProvidedContext(Opts.ContextProvider(File));
Action(InputsAndPreamble{It->second->Contents,
It->second->Worker->getCurrentCompileCommand(),
Preamble.get()});
@@ -1370,9 +1375,7 @@
WithContext Guard(std::move(Ctx));
trace::Span Tracer(Name);
SPAN_ATTACH(Tracer, "file", File);
-llvm::Optional WithProvidedContext;
-if (Opts.ContextProvider)
- WithProvidedContext.emplace(Opts.ContextProvider(File));
+WithContext WithProvidedContext(Opts.ContextProvider(File));
Action(InputsAndPreamble{Contents, Command, Preamble.get()});
};
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -313,7 +313,7 @@
private:
const GlobalCompilationDatabase
- const Options Opts;
+ Options Opts;
std::unique_ptr Callbacks; // not nullptr
Semaphore Barrier;
llvm::StringMap> Files;
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -965,6 +965,7 @@
if (RunSync) {
assert(!Done && "running a task after stop()");
trace::Span Tracer(Name + ":" + llvm::sys::path::filename(FileName));
+WithContext WithProvidedContext(ContextProvider(FileName));
Task();
return;
}
@@ -1062,9 +1063,7 @@
Status.ASTActivity.K = ASTAction::RunningAction;
Status.ASTActivity.Name = CurrentRequest->Name;
});
- llvm::Optional WithProvidedContext;
- if (ContextProvider)
-
