ilya-biryukov updated this revision to Diff 175028.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Remove accidentally added redundant 'private:' section
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,7 +29,6 @@
using ::testing::Pointee;
using ::testing::UnorderedElementsAre;
-void ignoreUpdate(Optional<std::vector<Diag>>) {}
void ignoreError(Error Err) {
handleAllErrors(std::move(Err), [](const ErrorInfoBase &) {});
}
@@ -40,11 +40,62 @@
buildTestFS(Files, Timestamps), std::move(Contents)};
}
+void updateWithCallback(TUScheduler &S, PathRef File, StringRef Contents,
+ WantDiagnostics WD, llvm::unique_function<void()> CB) {
+ WithContextValue Ctx(llvm::make_scope_exit(std::move(CB)));
+ S.update(File, getInputs(File, Contents), 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.
+static std::unique_ptr<ParsingCallbacks> captureDiags() {
+ class CaptureDiags : public ParsingCallbacks {
+ void onDiagnostics(PathRef File, std::vector<Diag> Diags) override {
+ auto D = Context::current().get(DiagsCallbackKey);
+ if (!D)
+ return;
+ const_cast<llvm::unique_function<void(PathRef, std::vector<Diag>)> &> (
+ *D)(File, Diags);
+ }
+ };
+ return llvm::make_unique<CaptureDiags>();
+}
+
+/// 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);
+}
+
+ void updateWithDiags(TUScheduler &S, PathRef File, llvm::StringRef Contents,
+ WantDiagnostics WD,
+ llvm::unique_function<void(std::vector<Diag>)> CB) {
+ return updateWithDiags(S, File, getInputs(File, Contents), WD, std::move(CB));
+ }
+
StringMap<std::string> Files;
StringMap<time_t> Timestamps;
MockCompilationDatabase CDB;
};
+Key<llvm::unique_function<void(PathRef File, std::vector<Diag>)>>
+ TUSchedulerTests::DiagsCallbackKey;
+
TEST_F(TUSchedulerTests, MissingFiles) {
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
@@ -57,7 +108,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).
@@ -104,25 +155,25 @@
Notification Ready;
TUScheduler S(
getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
+ /*StorePreamblesInMemory=*/true, captureDiags(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
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, "", WantDiagnostics::Yes,
+ [&](std::vector<Diag>) { Ready.wait(); });
+ updateWithDiags(S, Path, "request diags", WantDiagnostics::Yes,
+ [&](std::vector<Diag>) { ++CallbackCount; });
+ updateWithDiags(S, Path, "auto (clobbered)",
+ WantDiagnostics::Auto, [&](std::vector<Diag>) {
+ ADD_FAILURE()
+ << "auto should have been cancelled by auto";
+ });
+ updateWithDiags(S, Path, "request no diags", WantDiagnostics::No,
+ [&](std::vector<Diag>) {
+ ADD_FAILURE() << "no diags should not be called back";
+ });
+ updateWithDiags(S, Path, "auto (produces)", WantDiagnostics::Auto,
+ [&](std::vector<Diag>) { ++CallbackCount; });
Ready.notify();
}
EXPECT_EQ(2, CallbackCount);
@@ -132,21 +183,23 @@
std::atomic<int> CallbackCount(0);
{
TUScheduler S(getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
+ /*StorePreamblesInMemory=*/true, captureDiags(),
/*UpdateDebounce=*/std::chrono::seconds(1),
ASTRetentionPolicy());
// 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, "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, "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, "auto (shut down)", WantDiagnostics::Auto,
+ [&](std::vector<Diag>) { ++CallbackCount; });
}
EXPECT_EQ(2, CallbackCount);
}
@@ -172,22 +225,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, "#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) {
@@ -221,7 +273,7 @@
// Run TUScheduler and collect some stats.
{
TUScheduler S(getDefaultAsyncThreadsCount(),
- /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr,
+ /*StorePreamblesInMemory=*/true, captureDiags(),
/*UpdateDebounce=*/std::chrono::milliseconds(50),
ASTRetentionPolicy());
@@ -250,22 +302,18 @@
auto File = Files[FileI];
auto Inputs = getInputs(File, Contents.str());
-
{
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());
+ });
+ }
{
WithContextValue WithNonce(NonceKey, ++Nonce);
S.runWithAST("CheckAST", File,
@@ -336,26 +384,29 @@
// 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, 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, SourceContents,
+ WantDiagnostics::Yes,
+ [&BuiltASTCounter]() { ++BuiltASTCounter; });
+ updateWithCallback(S, 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, OtherSourceContents, WantDiagnostics::Yes,
+ [&BuiltASTCounter]() { ++BuiltASTCounter; });
ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
ASSERT_EQ(BuiltASTCounter.load(), 4);
@@ -382,8 +433,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 +446,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 +477,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,
@@ -448,7 +496,7 @@
TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
TUScheduler S(
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
+ /*StorePreambleInMemory=*/true, captureDiags(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
@@ -464,52 +512,53 @@
)cpp";
// Return value indicates if the updated callback was received.
- auto DoUpdate = [&](ParseInputs Inputs) -> bool {
+ auto DoUpdate = [&](std::string Contents) -> 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, Contents, 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?";
return Updated;
};
// Test that subsequent updates with the same inputs do not cause rebuilds.
- ASSERT_TRUE(DoUpdate(getInputs(Source, SourceContents)));
- ASSERT_FALSE(DoUpdate(getInputs(Source, SourceContents)));
+ ASSERT_TRUE(DoUpdate(SourceContents));
+ ASSERT_FALSE(DoUpdate(SourceContents));
// Update to a header should cause a rebuild, though.
Files[Header] = time_t(1);
- ASSERT_TRUE(DoUpdate(getInputs(Source, SourceContents)));
- ASSERT_FALSE(DoUpdate(getInputs(Source, SourceContents)));
+ ASSERT_TRUE(DoUpdate(SourceContents));
+ ASSERT_FALSE(DoUpdate(SourceContents));
// Update to the contents should cause a rebuild.
auto OtherSourceContents = R"cpp(
#include "foo.h"
int c = d;
)cpp";
- ASSERT_TRUE(DoUpdate(getInputs(Source, OtherSourceContents)));
- ASSERT_FALSE(DoUpdate(getInputs(Source, OtherSourceContents)));
+ ASSERT_TRUE(DoUpdate(OtherSourceContents));
+ ASSERT_FALSE(DoUpdate(OtherSourceContents));
// Update to the compile commands should also cause a rebuild.
CDB.ExtraClangFlags.push_back("-DSOMETHING");
- ASSERT_TRUE(DoUpdate(getInputs(Source, OtherSourceContents)));
- ASSERT_FALSE(DoUpdate(getInputs(Source, OtherSourceContents)));
+ ASSERT_TRUE(DoUpdate(OtherSourceContents));
+ ASSERT_FALSE(DoUpdate(OtherSourceContents));
}
TEST_F(TUSchedulerTests, NoChangeDiags) {
TUScheduler S(
/*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
- /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr,
+ /*StorePreambleInMemory=*/true, captureDiags(),
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
ASTRetentionPolicy());
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, 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 +568,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, 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, 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
@@ -72,6 +72,9 @@
/// in this callback (obtained via ParsedAST::getLocalTopLevelDecls) to obtain
/// optimal performance.
virtual void onMainAST(PathRef Path, ParsedAST &AST) {}
+
+ /// Called whenever the diagnostics for \p File are produced.
+ virtual void onDiagnostics(PathRef File, std::vector<Diag> Diags) {}
};
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
@@ -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.
Index: clangd/TUScheduler.cpp
===================================================================
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -166,17 +166,14 @@
/// 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);
~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,7 +227,7 @@
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;
/// Helper class required to build the ASTs.
const std::shared_ptr<PCHContainerOperations> PCHs;
@@ -332,9 +329,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 +413,16 @@
// spam us with updates.
// Note *AST can still be null if buildAST fails.
if (*AST) {
- OnUpdated((*AST)->getDiagnostics());
+ Callbacks.onDiagnostics(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(
@@ -696,8 +692,7 @@
}
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.
@@ -710,7 +705,7 @@
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
@@ -72,22 +72,34 @@
} // namespace
// Returns callbacks that can be used to update the FileIndex with new ASTs.
-static std::unique_ptr<ParsingCallbacks>
-makeUpdateCallbacks(FileIndex *FIndex) {
+static std::unique_ptr<ParsingCallbacks> makeUpdateCallbacks(
+ FileIndex *FIndex,
+ llvm::unique_function<void(PathRef, std::vector<Diag>)> OnDiags) {
+ using DiagsCallback = decltype(OnDiags);
struct CB : public ParsingCallbacks {
- CB(FileIndex *FIndex) : FIndex(FIndex) {}
- FileIndex *FIndex;
+ CB(FileIndex *FIndex, DiagsCallback OnDiags)
+ : FIndex(FIndex), OnDiags(std::move(OnDiags)) {}
void onPreambleAST(PathRef Path, ASTContext &Ctx,
std::shared_ptr<clang::Preprocessor> PP) override {
- FIndex->updatePreamble(Path, Ctx, std::move(PP));
+ if (FIndex)
+ FIndex->updatePreamble(Path, Ctx, std::move(PP));
}
void onMainAST(PathRef Path, ParsedAST &AST) override {
- FIndex->updateMain(Path, AST);
+ if (FIndex)
+ FIndex->updateMain(Path, AST);
+ }
+
+ void onDiagnostics(PathRef File, std::vector<Diag> Diags) override {
+ OnDiags(File, std::move(Diags));
}
+
+ private:
+ FileIndex *FIndex;
+ DiagsCallback OnDiags;
};
- return llvm::make_unique<CB>(FIndex);
+ return llvm::make_unique<CB>(FIndex, std::move(OnDiags));
}
ClangdServer::Options ClangdServer::optsForTest() {
@@ -117,8 +129,14 @@
// 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,
+ makeUpdateCallbacks(
+ DynamicIdx.get(),
+ [this](PathRef File, std::vector<Diag> Diags) {
+ DocVersion DocVer =
+ Context::current().getExisting(DocVersionKey);
+ this->consumeDiagnostics(File, DocVer,
+ std::move(Diags));
+ }),
Opts.UpdateDebounce, Opts.RetentionPolicy) {
if (DynamicIdx && Opts.StaticIndex) {
MergedIdx =
@@ -135,14 +153,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 +460,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
