ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
javed.absar.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54760
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
unittests/clangd/TUSchedulerTests.cpp
Index: unittests/clangd/TUSchedulerTests.cpp
===================================================================
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -10,6 +10,7 @@
#include "Context.h"
#include "TUScheduler.h"
#include "TestFS.h"
+#include "llvm/ADT/ScopeExit.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include <algorithm>
@@ -28,11 +29,49 @@
using ::testing::Pointee;
using ::testing::UnorderedElementsAre;
-void ignoreUpdate(Optional<std::vector<Diag>>) {}
void ignoreError(Error Err) {
handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {});
}
+/// Updates a file and executes a callback when the update is finished or
+/// cancelled.
+void updateWithCallback(TUScheduler &S, PathRef File, ParseInputs Inputs,
+ WantDiagnostics WD, llvm::unique_function<void()> CB) {
+ WithContextValue Ctx(llvm::make_scope_exit(std::move(CB)));
+ S.update(File, std::move(Inputs), WD);
+}
+
+static Key<llvm::unique_function<void(PathRef File, std::vector<Diag>)>>
+ DiagsCallbackKey;
+
+/// A diagnostics callback that should be passed to TUScheduler when it's used
+/// in updateWithDiags.
+void captureDiags(PathRef File, std::vector<Diag> Diags) {
+ auto D = Context::current().get(DiagsCallbackKey);
+ if (!D)
+ return;
+ const_cast<llvm::unique_function<void(PathRef, std::vector<Diag>)> &> (*D)(
+ File, Diags);
+}
+
+/// Schedule an update and call \p CB with the diagnostics it produces, if any.
+/// The TUScheduler should be created with captureDiags as a DiagsCallback for
+/// this to work.
+void updateWithDiags(TUScheduler &S, PathRef File, ParseInputs Inputs,
+ WantDiagnostics WD,
+ llvm::unique_function<void(std::vector<Diag>)> CB) {
+ Path OrigFile = File.str();
+ WithContextValue Ctx(
+ DiagsCallbackKey,
+ Bind(
+ [OrigFile](decltype(CB) CB, PathRef File, std::vector<Diag> Diags) {
+ assert(File == OrigFile);
+ CB(std::move(Diags));
+ },
+ std::move(CB)));
+ S.update(File, std::move(Inputs), WD);
+}
+
class TUSchedulerTests : public ::testing::Test {
protected:
ParseInputs getInputs(PathRef File, std::string Contents) {
@@ -57,7 +96,7 @@
auto Missing = testPath("missing.cpp");
Files[Missing] = "";
- S.update(Added, getInputs(Added, ""), WantDiagnostics::No, ignoreUpdate);
+ S.update(Added, getInputs(Added, ""), WantDiagnostics::No);
// Assert each operation for missing file is an error (even if it's available
// in VFS).
@@ -106,23 +145,25 @@
getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ ASTRetentionPolicy(), captureDiags);
auto Path = testPath("foo.cpp");
- S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
- [&](std::vector<Diag>) { Ready.wait(); });
-
- S.update(Path, getInputs(Path, "request diags"), WantDiagnostics::Yes,
- [&](std::vector<Diag> Diags) { ++CallbackCount; });
- S.update(Path, getInputs(Path, "auto (clobbered)"), WantDiagnostics::Auto,
- [&](std::vector<Diag> Diags) {
- ADD_FAILURE() << "auto should have been cancelled by auto";
- });
- S.update(Path, getInputs(Path, "request no diags"), WantDiagnostics::No,
- [&](std::vector<Diag> Diags) {
- ADD_FAILURE() << "no diags should not be called back";
- });
- S.update(Path, getInputs(Path, "auto (produces)"), WantDiagnostics::Auto,
- [&](std::vector<Diag> Diags) { ++CallbackCount; });
+ updateWithDiags(S, Path, getInputs(Path, ""), WantDiagnostics::Yes,
+ [&](std::vector<Diag>) { Ready.wait(); });
+ updateWithDiags(S, Path, getInputs(Path, "request diags"),
+ WantDiagnostics::Yes,
+ [&](std::vector<Diag>) { ++CallbackCount; });
+ updateWithDiags(S, Path, getInputs(Path, "auto (clobbered)"),
+ WantDiagnostics::Auto, [&](std::vector<Diag>) {
+ ADD_FAILURE()
+ << "auto should have been cancelled by auto";
+ });
+ updateWithDiags(S, Path, getInputs(Path, "request no diags"),
+ WantDiagnostics::No, [&](std::vector<Diag>) {
+ ADD_FAILURE() << "no diags should not be called back";
+ });
+ updateWithDiags(S, Path, getInputs(Path, "auto (produces)"),
+ WantDiagnostics::Auto,
+ [&](std::vector<Diag>) { ++CallbackCount; });
Ready.notify();
}
EXPECT_EQ(2, CallbackCount);
@@ -134,19 +175,22 @@
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::seconds(1),
- ASTRetentionPolicy());
+ ASTRetentionPolicy(), captureDiags);
// FIXME: we could probably use timeouts lower than 1 second here.
auto Path = testPath("foo.cpp");
- S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
- [&](std::vector<Diag> Diags) {
- ADD_FAILURE() << "auto should have been debounced and canceled";
- });
+ updateWithDiags(S, Path, getInputs(Path, "auto (debounced)"),
+ WantDiagnostics::Auto, [&](std::vector<Diag>) {
+ ADD_FAILURE()
+ << "auto should have been debounced and canceled";
+ });
std::this_thread::sleep_for(std::chrono::milliseconds(200));
- S.update(Path, getInputs(Path, "auto (timed out)"), WantDiagnostics::Auto,
- [&](std::vector<Diag> Diags) { ++CallbackCount; });
+ updateWithDiags(S, Path, getInputs(Path, "auto (timed out)"),
+ WantDiagnostics::Auto,
+ [&](std::vector<Diag>) { ++CallbackCount; });
std::this_thread::sleep_for(std::chrono::seconds(2));
- S.update(Path, getInputs(Path, "auto (shut down)"), WantDiagnostics::Auto,
- [&](std::vector<Diag> Diags) { ++CallbackCount; });
+ updateWithDiags(S, Path, getInputs(Path, "auto (shut down)"),
+ WantDiagnostics::Auto,
+ [&](std::vector<Diag>) { ++CallbackCount; });
}
EXPECT_EQ(2, CallbackCount);
}
@@ -172,22 +216,21 @@
// Schedule two updates (A, B) and two preamble reads (stale, consistent).
// The stale read should see A, and the consistent read should see B.
// (We recognize the preambles by their included files).
- S.update(Path, getInputs(Path, "#include <A>"), WantDiagnostics::Yes,
- [&](std::vector<Diag> Diags) {
- // This callback runs in between the two preamble updates.
-
- // This blocks update B, preventing it from winning the race
- // against the stale read.
- // If the first read was instead consistent, this would deadlock.
- InconsistentReadDone.wait();
- // This delays update B, preventing it from winning a race
- // against the consistent read. The consistent read sees B
- // only because it waits for it.
- // If the second read was stale, it would usually see A.
- std::this_thread::sleep_for(std::chrono::milliseconds(100));
- });
- S.update(Path, getInputs(Path, "#include <B>"), WantDiagnostics::Yes,
- [&](std::vector<Diag> Diags) {});
+ updateWithCallback(
+ S, Path, getInputs(Path, "#include <A>"), WantDiagnostics::Yes, [&]() {
+ // This callback runs in between the two preamble updates.
+
+ // This blocks update B, preventing it from winning the race
+ // against the stale read.
+ // If the first read was instead consistent, this would deadlock.
+ InconsistentReadDone.wait();
+ // This delays update B, preventing it from winning a race
+ // against the consistent read. The consistent read sees B
+ // only because it waits for it.
+ // If the second read was stale, it would usually see A.
+ std::this_thread::sleep_for(std::chrono::milliseconds(100));
+ });
+ S.update(Path, getInputs(Path, "#include <B>"), WantDiagnostics::Yes);
S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
[&](Expected<InputsAndPreamble> Pre) {
@@ -223,7 +266,7 @@
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::milliseconds(50),
- ASTRetentionPolicy());
+ ASTRetentionPolicy(), captureDiags);
std::vector<std::string> Files;
for (int I = 0; I < FilesCount; ++I) {
@@ -253,17 +296,15 @@
{
WithContextValue WithNonce(NonceKey, ++Nonce);
- S.update(File, Inputs, WantDiagnostics::Auto,
- [File, Nonce, &Mut,
- &TotalUpdates](Optional<std::vector<Diag>> Diags) {
- EXPECT_THAT(Context::current().get(NonceKey),
- Pointee(Nonce));
-
- std::lock_guard<std::mutex> Lock(Mut);
- ++TotalUpdates;
- EXPECT_EQ(File,
- *TUScheduler::getFileBeingProcessedInContext());
- });
+ updateWithDiags(
+ S, File, Inputs, WantDiagnostics::Auto,
+ [File, Nonce, &Mut, &TotalUpdates](std::vector<Diag>) {
+ EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
+
+ std::lock_guard<std::mutex> Lock(Mut);
+ ++TotalUpdates;
+ EXPECT_EQ(File, *TUScheduler::getFileBeingProcessedInContext());
+ });
}
{
@@ -336,26 +377,30 @@
// Build one file in advance. We will not access it later, so it will be the
// one that the cache will evict.
- S.update(Foo, getInputs(Foo, SourceContents), WantDiagnostics::Yes,
- [&BuiltASTCounter](std::vector<Diag> Diags) { ++BuiltASTCounter; });
+ updateWithCallback(S, Foo, getInputs(Foo, SourceContents),
+ WantDiagnostics::Yes,
+ [&BuiltASTCounter]() { ++BuiltASTCounter; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
ASSERT_EQ(BuiltASTCounter.load(), 1);
// Build two more files. Since we can retain only 2 ASTs, these should be the
// ones we see in the cache later.
- S.update(Bar, getInputs(Bar, SourceContents), WantDiagnostics::Yes,
- [&BuiltASTCounter](std::vector<Diag> Diags) { ++BuiltASTCounter; });
- S.update(Baz, getInputs(Baz, SourceContents), WantDiagnostics::Yes,
- [&BuiltASTCounter](std::vector<Diag> Diags) { ++BuiltASTCounter; });
+ updateWithCallback(S, Bar, getInputs(Bar, SourceContents),
+ WantDiagnostics::Yes,
+ [&BuiltASTCounter]() { ++BuiltASTCounter; });
+ updateWithCallback(S, Baz, getInputs(Baz, SourceContents),
+ WantDiagnostics::Yes,
+ [&BuiltASTCounter]() { ++BuiltASTCounter; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
ASSERT_EQ(BuiltASTCounter.load(), 3);
// Check only the last two ASTs are retained.
ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz));
// Access the old file again.
- S.update(Foo, getInputs(Foo, OtherSourceContents), WantDiagnostics::Yes,
- [&BuiltASTCounter](std::vector<Diag> Diags) { ++BuiltASTCounter; });
+ updateWithCallback(S, Foo, getInputs(Foo, OtherSourceContents),
+ WantDiagnostics::Yes,
+ [&BuiltASTCounter]() { ++BuiltASTCounter; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
ASSERT_EQ(BuiltASTCounter.load(), 4);
@@ -382,8 +427,7 @@
int main() {}
)cpp";
auto WithEmptyPreamble = R"cpp(int main() {})cpp";
- S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto,
- [](std::vector<Diag>) {});
+ S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto);
S.runWithPreamble("getNonEmptyPreamble", Foo, TUScheduler::Stale,
[&](Expected<InputsAndPreamble> Preamble) {
// We expect to get a non-empty preamble.
@@ -396,8 +440,7 @@
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
// Update the file which results in an empty preamble.
- S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto,
- [](std::vector<Diag>) {});
+ S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto);
// Wait for the preamble is being built.
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
S.runWithPreamble("getEmptyPreamble", Foo, TUScheduler::Stale,
@@ -428,8 +471,7 @@
constexpr int ReadsToSchedule = 10;
std::mutex PreamblesMut;
std::vector<const void *> Preambles(ReadsToSchedule, nullptr);
- S.update(Foo, getInputs(Foo, NonEmptyPreamble), WantDiagnostics::Auto,
- [](std::vector<Diag>) {});
+ S.update(Foo, getInputs(Foo, NonEmptyPreamble), WantDiagnostics::Auto);
for (int I = 0; I < ReadsToSchedule; ++I) {
S.runWithPreamble(
"test", Foo, TUScheduler::Stale,
@@ -450,7 +492,7 @@
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
/*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ ASTRetentionPolicy(), captureDiags);
auto Source = testPath("foo.cpp");
auto Header = testPath("foo.h");
@@ -467,8 +509,8 @@
auto DoUpdate = [&](ParseInputs Inputs) -> bool {
std::atomic<bool> Updated(false);
Updated = false;
- S.update(Source, std::move(Inputs), WantDiagnostics::Yes,
- [&Updated](std::vector<Diag>) { Updated = true; });
+ updateWithDiags(S, Source, std::move(Inputs), WantDiagnostics::Yes,
+ [&Updated](std::vector<Diag>) { Updated = true; });
bool UpdateFinished = S.blockUntilIdle(timeoutSeconds(10));
if (!UpdateFinished)
ADD_FAILURE() << "Updated has not finished in one second. Threading bug?";
@@ -503,13 +545,14 @@
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
/*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
- ASTRetentionPolicy());
+ ASTRetentionPolicy(), captureDiags);
auto FooCpp = testPath("foo.cpp");
auto Contents = "int a; int b;";
- S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::No,
- [](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
+ updateWithDiags(
+ S, FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::No,
+ [](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
S.runWithAST("touchAST", FooCpp, [](Expected<InputsAndAST> IA) {
// Make sure the AST was actually built.
cantFail(std::move(IA));
@@ -519,15 +562,15 @@
// Even though the inputs didn't change and AST can be reused, we need to
// report the diagnostics, as they were not reported previously.
std::atomic<bool> SeenDiags(false);
- S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto,
- [&](std::vector<Diag>) { SeenDiags = true; });
+ updateWithDiags(S, FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto,
+ [&](std::vector<Diag>) { SeenDiags = true; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
ASSERT_TRUE(SeenDiags);
// Subsequent request does not get any diagnostics callback because the same
// diags have previously been reported and the inputs didn't change.
- S.update(
- FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto,
+ updateWithDiags(
+ S, FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto,
[&](std::vector<Diag>) { ADD_FAILURE() << "Should not be called."; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
}
Index: clangd/TUScheduler.h
===================================================================
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -83,10 +83,13 @@
/// FIXME(sammccall): pull out a scheduler options struct.
class TUScheduler {
public:
+ using DiagsCallback =
+ llvm::unique_function<void(PathRef File, std::vector<Diag>)>;
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
std::unique_ptr<ParsingCallbacks> ASTCallbacks,
std::chrono::steady_clock::duration UpdateDebounce,
- ASTRetentionPolicy RetentionPolicy);
+ ASTRetentionPolicy RetentionPolicy,
+ DiagsCallback OnDiags = nullptr);
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -100,9 +103,7 @@
/// Schedule an update for \p File. Adds \p File to a list of tracked files if
/// \p File was not part of it before.
- /// FIXME(ibiryukov): remove the callback from this function.
- void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD,
- llvm::unique_function<void(std::vector<Diag>)> OnUpdated);
+ void update(PathRef File, ParseInputs Inputs, WantDiagnostics WD);
/// Remove \p File from the list of tracked files and schedule removal of its
/// resources.
@@ -167,6 +168,7 @@
private:
const bool StorePreamblesInMemory;
const std::shared_ptr<PCHContainerOperations> PCHOps;
+ DiagsCallback OnDiags;
std::unique_ptr<ParsingCallbacks> Callbacks; // not nullptr
Semaphore Barrier;
llvm::StringMap<std::unique_ptr<FileData>> Files;
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -153,30 +153,32 @@
/// signals the worker to exit its run loop and gives up shared ownership of the
/// worker.
class ASTWorker {
+public:
+ using DiagsCallbackRef = llvm::function_ref<void(PathRef, std::vector<Diag>)>;
+
+private:
friend class ASTWorkerHandle;
ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache,
Semaphore &Barrier, bool RunSync,
steady_clock::duration UpdateDebounce,
std::shared_ptr<PCHContainerOperations> PCHs,
- bool StorePreamblesInMemory, ParsingCallbacks &Callbacks);
+ bool StorePreamblesInMemory, ParsingCallbacks &Callbacks,
+ DiagsCallbackRef OnDiags);
public:
/// Create a new ASTWorker and return a handle to it.
/// The processing thread is spawned using \p Tasks. However, when \p Tasks
/// is null, all requests will be processed on the calling thread
/// synchronously instead. \p Barrier is acquired when processing each
/// request, it is used to limit the number of actively running threads.
- static ASTWorkerHandle create(PathRef FileName,
- TUScheduler::ASTCache &IdleASTs,
- AsyncTaskRunner *Tasks, Semaphore &Barrier,
- steady_clock::duration UpdateDebounce,
- std::shared_ptr<PCHContainerOperations> PCHs,
- bool StorePreamblesInMemory,
- ParsingCallbacks &Callbacks);
+ static ASTWorkerHandle create(
+ PathRef FileName, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
+ Semaphore &Barrier, steady_clock::duration UpdateDebounce,
+ std::shared_ptr<PCHContainerOperations> PCHs, bool StorePreamblesInMemory,
+ ParsingCallbacks &Callbacks, DiagsCallbackRef OnDiags);
~ASTWorker();
- void update(ParseInputs Inputs, WantDiagnostics,
- llvm::unique_function<void(std::vector<Diag>)> OnUpdated);
+ void update(ParseInputs Inputs, WantDiagnostics);
void runWithAST(StringRef Name,
unique_function<void(Expected<InputsAndAST>)> Action);
bool blockUntilIdle(Deadline Timeout) const;
@@ -230,8 +232,10 @@
const Path FileName;
/// Whether to keep the built preambles in memory or on disk.
const bool StorePreambleInMemory;
- /// Callback, invoked when preamble or main file AST is built.
+ /// Callback invoked when preamble or main file AST is built.
ParsingCallbacks &Callbacks;
+ /// Callback invoked when diagnostics are ready.
+ DiagsCallbackRef OnDiags;
/// Helper class required to build the ASTs.
const std::shared_ptr<PCHContainerOperations> PCHs;
@@ -295,16 +299,14 @@
std::shared_ptr<ASTWorker> Worker;
};
-ASTWorkerHandle ASTWorker::create(PathRef FileName,
- TUScheduler::ASTCache &IdleASTs,
- AsyncTaskRunner *Tasks, Semaphore &Barrier,
- steady_clock::duration UpdateDebounce,
- std::shared_ptr<PCHContainerOperations> PCHs,
- bool StorePreamblesInMemory,
- ParsingCallbacks &Callbacks) {
+ASTWorkerHandle ASTWorker::create(
+ PathRef FileName, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks,
+ Semaphore &Barrier, steady_clock::duration UpdateDebounce,
+ std::shared_ptr<PCHContainerOperations> PCHs, bool StorePreamblesInMemory,
+ ParsingCallbacks &Callbacks, DiagsCallbackRef OnDiags) {
std::shared_ptr<ASTWorker> Worker(new ASTWorker(
FileName, IdleASTs, Barrier, /*RunSync=*/!Tasks, UpdateDebounce,
- std::move(PCHs), StorePreamblesInMemory, Callbacks));
+ std::move(PCHs), StorePreamblesInMemory, Callbacks, OnDiags));
if (Tasks)
Tasks->runAsync("worker:" + sys::path::filename(FileName),
[Worker]() { Worker->run(); });
@@ -316,11 +318,12 @@
Semaphore &Barrier, bool RunSync,
steady_clock::duration UpdateDebounce,
std::shared_ptr<PCHContainerOperations> PCHs,
- bool StorePreamblesInMemory, ParsingCallbacks &Callbacks)
+ bool StorePreamblesInMemory, ParsingCallbacks &Callbacks,
+ DiagsCallbackRef OnDiags)
: IdleASTs(LRUCache), RunSync(RunSync), UpdateDebounce(UpdateDebounce),
FileName(FileName), StorePreambleInMemory(StorePreamblesInMemory),
- Callbacks(Callbacks), PCHs(std::move(PCHs)), Barrier(Barrier),
- Done(false) {}
+ Callbacks(Callbacks), OnDiags(OnDiags), PCHs(std::move(PCHs)),
+ Barrier(Barrier), Done(false) {}
ASTWorker::~ASTWorker() {
// Make sure we remove the cached AST, if any.
@@ -332,9 +335,8 @@
#endif
}
-void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags,
- unique_function<void(std::vector<Diag>)> OnUpdated) {
- auto Task = [=](decltype(OnUpdated) OnUpdated) mutable {
+void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags) {
+ auto Task = [=]() mutable {
// Will be used to check if we can avoid rebuilding the AST.
bool InputsAreTheSame =
std::tie(FileInputs.CompileCommand, FileInputs.Contents) ==
@@ -417,16 +419,16 @@
// spam us with updates.
// Note *AST can still be null if buildAST fails.
if (*AST) {
- OnUpdated((*AST)->getDiagnostics());
+ OnDiags(FileName, (*AST)->getDiagnostics());
trace::Span Span("Running main AST callback");
Callbacks.onMainAST(FileName, **AST);
DiagsWereReported = true;
}
// Stash the AST in the cache for further use.
IdleASTs.put(this, std::move(*AST));
};
- startTask("Update", Bind(Task, std::move(OnUpdated)), WantDiags);
+ startTask("Update", std::move(Task), WantDiags);
}
void ASTWorker::runWithAST(
@@ -660,9 +662,11 @@
bool StorePreamblesInMemory,
std::unique_ptr<ParsingCallbacks> Callbacks,
std::chrono::steady_clock::duration UpdateDebounce,
- ASTRetentionPolicy RetentionPolicy)
+ ASTRetentionPolicy RetentionPolicy,
+ DiagsCallback OnDiags)
: StorePreamblesInMemory(StorePreamblesInMemory),
PCHOps(std::make_shared<PCHContainerOperations>()),
+ OnDiags(OnDiags ? std::move(OnDiags) : [](PathRef, std::vector<Diag>) {}),
Callbacks(Callbacks ? move(Callbacks)
: llvm::make_unique<ParsingCallbacks>()),
Barrier(AsyncThreadsCount),
@@ -696,21 +700,21 @@
}
void TUScheduler::update(PathRef File, ParseInputs Inputs,
- WantDiagnostics WantDiags,
- unique_function<void(std::vector<Diag>)> OnUpdated) {
+ WantDiagnostics WantDiags) {
std::unique_ptr<FileData> &FD = Files[File];
if (!FD) {
// Create a new worker to process the AST-related tasks.
ASTWorkerHandle Worker = ASTWorker::create(
File, *IdleASTs, WorkerThreads ? WorkerThreads.getPointer() : nullptr,
- Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, *Callbacks);
+ Barrier, UpdateDebounce, PCHOps, StorePreamblesInMemory, *Callbacks,
+ OnDiags);
FD = std::unique_ptr<FileData>(new FileData{
Inputs.Contents, Inputs.CompileCommand, std::move(Worker)});
} else {
FD->Contents = Inputs.Contents;
FD->Command = Inputs.CompileCommand;
}
- FD->Worker->update(std::move(Inputs), WantDiags, std::move(OnUpdated));
+ FD->Worker->update(std::move(Inputs), WantDiags);
}
void TUScheduler::remove(PathRef File) {
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -227,6 +227,9 @@
ArrayRef<tooling::Range> Ranges);
typedef uint64_t DocVersion;
+ /// A unique version of the document stashed in the context on calls to
+ /// addDocument. Used to avoid races when sending diagnostics to the clients.
+ static Key<ClangdServer::DocVersion> DocVersionKey;
void consumeDiagnostics(PathRef File, DocVersion Version,
std::vector<Diag> Diags);
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -116,10 +116,14 @@
// is parsed.
// FIXME(ioeric): this can be slow and we may be able to index on less
// critical paths.
- WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
- DynamicIdx ? makeUpdateCallbacks(DynamicIdx.get())
- : nullptr,
- Opts.UpdateDebounce, Opts.RetentionPolicy) {
+ WorkScheduler(
+ Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+ DynamicIdx ? makeUpdateCallbacks(DynamicIdx.get()) : nullptr,
+ Opts.UpdateDebounce, Opts.RetentionPolicy,
+ [this](PathRef File, std::vector<Diag> Diags) {
+ DocVersion DocVer = Context::current().getExisting(DocVersionKey);
+ this->consumeDiagnostics(File, DocVer, std::move(Diags));
+ }) {
if (DynamicIdx && Opts.StaticIndex) {
MergedIdx =
llvm::make_unique<MergedIndex>(DynamicIdx.get(), Opts.StaticIndex);
@@ -135,14 +139,11 @@
void ClangdServer::addDocument(PathRef File, StringRef Contents,
WantDiagnostics WantDiags) {
DocVersion Version = ++InternalVersion[File];
+ WithContextValue Ctx(DocVersionKey, Version);
ParseInputs Inputs = {getCompileCommand(File), FSProvider.getFileSystem(),
Contents.str()};
-
Path FileStr = File.str();
- WorkScheduler.update(File, std::move(Inputs), WantDiags,
- [this, FileStr, Version](std::vector<Diag> Diags) {
- consumeDiagnostics(FileStr, Version, std::move(Diags));
- });
+ WorkScheduler.update(File, std::move(Inputs), WantDiags);
}
void ClangdServer::removeDocument(PathRef File) {
@@ -445,6 +446,8 @@
WorkScheduler.runWithAST("Hover", File, Bind(Action, std::move(CB)));
}
+Key<ClangdServer::DocVersion> ClangdServer::DocVersionKey;
+
void ClangdServer::consumeDiagnostics(PathRef File, DocVersion Version,
std::vector<Diag> Diags) {
// We need to serialize access to resulting diagnostics to avoid calling
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
