Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -227,6 +227,9 @@ ArrayRef 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 DocVersionKey; void consumeDiagnostics(PathRef File, DocVersion Version, std::vector 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 Diags) { + DocVersion DocVer = Context::current().getExisting(DocVersionKey); + this->consumeDiagnostics(File, DocVer, std::move(Diags)); + }) { if (DynamicIdx && Opts.StaticIndex) { MergedIdx = llvm::make_unique(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 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::DocVersionKey; + void ClangdServer::consumeDiagnostics(PathRef File, DocVersion Version, std::vector Diags) { // We need to serialize access to resulting diagnostics to avoid calling 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)>; TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, std::unique_ptr 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)> 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 PCHOps; + DiagsCallback OnDiags; std::unique_ptr Callbacks; // not nullptr Semaphore Barrier; llvm::StringMap> Files; Index: clangd/TUScheduler.cpp =================================================================== --- clangd/TUScheduler.cpp +++ clangd/TUScheduler.cpp @@ -153,12 +153,17 @@ /// signals the worker to exit its run loop and gives up shared ownership of the /// worker. class ASTWorker { +public: + using DiagsCallbackRef = llvm::function_ref)>; + +private: friend class ASTWorkerHandle; ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync, steady_clock::duration UpdateDebounce, std::shared_ptr PCHs, - bool StorePreamblesInMemory, ParsingCallbacks &Callbacks); + bool StorePreamblesInMemory, ParsingCallbacks &Callbacks, + DiagsCallbackRef OnDiags); public: /// Create a new ASTWorker and return a handle to it. @@ -166,17 +171,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 PCHs, - bool StorePreamblesInMemory, - ParsingCallbacks &Callbacks); + static ASTWorkerHandle create( + PathRef FileName, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks, + Semaphore &Barrier, steady_clock::duration UpdateDebounce, + std::shared_ptr PCHs, bool StorePreamblesInMemory, + ParsingCallbacks &Callbacks, DiagsCallbackRef OnDiags); ~ASTWorker(); - void update(ParseInputs Inputs, WantDiagnostics, - llvm::unique_function)> OnUpdated); + void update(ParseInputs Inputs, WantDiagnostics); void runWithAST(StringRef Name, unique_function)> 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 PCHs; @@ -295,16 +299,14 @@ std::shared_ptr Worker; }; -ASTWorkerHandle ASTWorker::create(PathRef FileName, - TUScheduler::ASTCache &IdleASTs, - AsyncTaskRunner *Tasks, Semaphore &Barrier, - steady_clock::duration UpdateDebounce, - std::shared_ptr PCHs, - bool StorePreamblesInMemory, - ParsingCallbacks &Callbacks) { +ASTWorkerHandle ASTWorker::create( + PathRef FileName, TUScheduler::ASTCache &IdleASTs, AsyncTaskRunner *Tasks, + Semaphore &Barrier, steady_clock::duration UpdateDebounce, + std::shared_ptr PCHs, bool StorePreamblesInMemory, + ParsingCallbacks &Callbacks, DiagsCallbackRef OnDiags) { std::shared_ptr 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 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)> 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,7 +419,7 @@ // 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; @@ -426,7 +428,7 @@ 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 Callbacks, std::chrono::steady_clock::duration UpdateDebounce, - ASTRetentionPolicy RetentionPolicy) + ASTRetentionPolicy RetentionPolicy, + DiagsCallback OnDiags) : StorePreamblesInMemory(StorePreamblesInMemory), PCHOps(std::make_shared()), + OnDiags(OnDiags ? std::move(OnDiags) : [](PathRef, std::vector) {}), Callbacks(Callbacks ? move(Callbacks) : llvm::make_unique()), Barrier(AsyncThreadsCount), @@ -696,21 +700,21 @@ } void TUScheduler::update(PathRef File, ParseInputs Inputs, - WantDiagnostics WantDiags, - unique_function)> OnUpdated) { + WantDiagnostics WantDiags) { std::unique_ptr &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(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: 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 @@ -28,11 +29,49 @@ using ::testing::Pointee; using ::testing::UnorderedElementsAre; -void ignoreUpdate(Optional>) {} 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 CB) { + WithContextValue Ctx(llvm::make_scope_exit(std::move(CB))); + S.update(File, std::move(Inputs), WD); +} + +static Key)>> + DiagsCallbackKey; + +/// A diagnostics callback that should be passed to TUScheduler when it's used +/// in updateWithDiags. +void captureDiags(PathRef File, std::vector Diags) { + auto D = Context::current().get(DiagsCallbackKey); + if (!D) + return; + const_cast)> &> (*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)> CB) { + Path OrigFile = File.str(); + WithContextValue Ctx( + DiagsCallbackKey, + Bind( + [OrigFile](decltype(CB) CB, PathRef File, std::vector 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) { Ready.wait(); }); - - S.update(Path, getInputs(Path, "request diags"), WantDiagnostics::Yes, - [&](std::vector Diags) { ++CallbackCount; }); - S.update(Path, getInputs(Path, "auto (clobbered)"), WantDiagnostics::Auto, - [&](std::vector Diags) { - ADD_FAILURE() << "auto should have been cancelled by auto"; - }); - S.update(Path, getInputs(Path, "request no diags"), WantDiagnostics::No, - [&](std::vector Diags) { - ADD_FAILURE() << "no diags should not be called back"; - }); - S.update(Path, getInputs(Path, "auto (produces)"), WantDiagnostics::Auto, - [&](std::vector Diags) { ++CallbackCount; }); + updateWithDiags(S, Path, getInputs(Path, ""), WantDiagnostics::Yes, + [&](std::vector) { Ready.wait(); }); + updateWithDiags(S, Path, getInputs(Path, "request diags"), + WantDiagnostics::Yes, + [&](std::vector) { ++CallbackCount; }); + updateWithDiags(S, Path, getInputs(Path, "auto (clobbered)"), + WantDiagnostics::Auto, [&](std::vector) { + ADD_FAILURE() + << "auto should have been cancelled by auto"; + }); + updateWithDiags(S, Path, getInputs(Path, "request no diags"), + WantDiagnostics::No, [&](std::vector) { + ADD_FAILURE() << "no diags should not be called back"; + }); + updateWithDiags(S, Path, getInputs(Path, "auto (produces)"), + WantDiagnostics::Auto, + [&](std::vector) { ++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 Diags) { - ADD_FAILURE() << "auto should have been debounced and canceled"; - }); + updateWithDiags(S, Path, getInputs(Path, "auto (debounced)"), + WantDiagnostics::Auto, [&](std::vector) { + 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 Diags) { ++CallbackCount; }); + updateWithDiags(S, Path, getInputs(Path, "auto (timed out)"), + WantDiagnostics::Auto, + [&](std::vector) { ++CallbackCount; }); std::this_thread::sleep_for(std::chrono::seconds(2)); - S.update(Path, getInputs(Path, "auto (shut down)"), WantDiagnostics::Auto, - [&](std::vector Diags) { ++CallbackCount; }); + updateWithDiags(S, Path, getInputs(Path, "auto (shut down)"), + WantDiagnostics::Auto, + [&](std::vector) { ++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 "), WantDiagnostics::Yes, - [&](std::vector 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 "), WantDiagnostics::Yes, - [&](std::vector Diags) {}); + updateWithCallback( + S, Path, getInputs(Path, "#include "), 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 "), WantDiagnostics::Yes); S.runWithPreamble("StaleRead", Path, TUScheduler::Stale, [&](Expected Pre) { @@ -223,7 +266,7 @@ TUScheduler S(getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr, /*UpdateDebounce=*/std::chrono::milliseconds(50), - ASTRetentionPolicy()); + ASTRetentionPolicy(), captureDiags); std::vector 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> Diags) { - EXPECT_THAT(Context::current().get(NonceKey), - Pointee(Nonce)); - - std::lock_guard Lock(Mut); - ++TotalUpdates; - EXPECT_EQ(File, - *TUScheduler::getFileBeingProcessedInContext()); - }); + updateWithDiags( + S, File, Inputs, WantDiagnostics::Auto, + [File, Nonce, &Mut, &TotalUpdates](std::vector) { + EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce)); + + std::lock_guard Lock(Mut); + ++TotalUpdates; + EXPECT_EQ(File, *TUScheduler::getFileBeingProcessedInContext()); + }); } { @@ -336,17 +377,20 @@ // 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 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 Diags) { ++BuiltASTCounter; }); - S.update(Baz, getInputs(Baz, SourceContents), WantDiagnostics::Yes, - [&BuiltASTCounter](std::vector 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); @@ -354,8 +398,9 @@ ASSERT_THAT(S.getFilesWithCachedAST(), UnorderedElementsAre(Bar, Baz)); // Access the old file again. - S.update(Foo, getInputs(Foo, OtherSourceContents), WantDiagnostics::Yes, - [&BuiltASTCounter](std::vector 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) {}); + S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto); S.runWithPreamble("getNonEmptyPreamble", Foo, TUScheduler::Stale, [&](Expected 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) {}); + 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 Preambles(ReadsToSchedule, nullptr); - S.update(Foo, getInputs(Foo, NonEmptyPreamble), WantDiagnostics::Auto, - [](std::vector) {}); + 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 Updated(false); Updated = false; - S.update(Source, std::move(Inputs), WantDiagnostics::Yes, - [&Updated](std::vector) { Updated = true; }); + updateWithDiags(S, Source, std::move(Inputs), WantDiagnostics::Yes, + [&Updated](std::vector) { 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) { ADD_FAILURE() << "Should not be called."; }); + updateWithDiags( + S, FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::No, + [](std::vector) { ADD_FAILURE() << "Should not be called."; }); S.runWithAST("touchAST", FooCpp, [](Expected 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 SeenDiags(false); - S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto, - [&](std::vector) { SeenDiags = true; }); + updateWithDiags(S, FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto, + [&](std::vector) { 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) { ADD_FAILURE() << "Should not be called."; }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); }