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 @@ -72,22 +72,34 @@ } // namespace // Returns callbacks that can be used to update the FileIndex with new ASTs. -static std::unique_ptr -makeUpdateCallbacks(FileIndex *FIndex) { +static std::unique_ptr makeUpdateCallbacks( + FileIndex *FIndex, + llvm::unique_function)> 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 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 Diags) override { + OnDiags(File, std::move(Diags)); } + + private: + FileIndex *FIndex; + DiagsCallback OnDiags; }; - return llvm::make_unique(FIndex); + return llvm::make_unique(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 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 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::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 @@ -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 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)> 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 @@ -153,6 +153,7 @@ /// signals the worker to exit its run loop and gives up shared ownership of the /// worker. class ASTWorker { +private: friend class ASTWorkerHandle; ASTWorker(PathRef FileName, TUScheduler::ASTCache &LRUCache, Semaphore &Barrier, bool RunSync, @@ -166,17 +167,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); ~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,7 +228,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 PCHs; @@ -332,9 +330,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 +414,7 @@ // 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; @@ -426,7 +423,7 @@ 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 +693,7 @@ } 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. @@ -710,7 +706,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: 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,7 +29,6 @@ using ::testing::Pointee; using ::testing::UnorderedElementsAre; -void ignoreUpdate(Optional>) {} 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 CB) { + WithContextValue Ctx(llvm::make_scope_exit(std::move(CB))); + S.update(File, getInputs(File, Contents), WD); +} + +static Key)>> + DiagsCallbackKey; + +/// A diagnostics callback that should be passed to TUScheduler when it's used +/// in updateWithDiags. +static std::unique_ptr captureDiags() { + class CaptureDiags : public ParsingCallbacks { + void onDiagnostics(PathRef File, std::vector Diags) override { + auto D = Context::current().get(DiagsCallbackKey); + if (!D) + return; + const_cast)> &> ( + *D)(File, Diags); + } + }; + return llvm::make_unique(); +} + +/// 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); +} + + void updateWithDiags(TUScheduler &S, PathRef File, llvm::StringRef Contents, + WantDiagnostics WD, + llvm::unique_function)> CB) { + return updateWithDiags(S, File, getInputs(File, Contents), WD, std::move(CB)); + } + StringMap Files; StringMap Timestamps; MockCompilationDatabase CDB; }; +Key)>> + 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) { 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, "", WantDiagnostics::Yes, + [&](std::vector) { Ready.wait(); }); + updateWithDiags(S, Path, "request diags", WantDiagnostics::Yes, + [&](std::vector) { ++CallbackCount; }); + updateWithDiags(S, Path, "auto (clobbered)", + WantDiagnostics::Auto, [&](std::vector) { + ADD_FAILURE() + << "auto should have been cancelled by auto"; + }); + updateWithDiags(S, Path, "request no diags", WantDiagnostics::No, + [&](std::vector) { + ADD_FAILURE() << "no diags should not be called back"; + }); + updateWithDiags(S, Path, "auto (produces)", WantDiagnostics::Auto, + [&](std::vector) { ++CallbackCount; }); Ready.notify(); } EXPECT_EQ(2, CallbackCount); @@ -132,21 +183,23 @@ std::atomic 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 Diags) { - ADD_FAILURE() << "auto should have been debounced and canceled"; - }); + updateWithDiags(S, 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, "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, "auto (shut down)", WantDiagnostics::Auto, + [&](std::vector) { ++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 "), 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, "#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) { @@ -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> 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()); + }); + } { WithContextValue WithNonce(NonceKey, ++Nonce); S.runWithAST("CheckAST", File, @@ -336,17 +384,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, 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, SourceContents, + WantDiagnostics::Yes, + [&BuiltASTCounter]() { ++BuiltASTCounter; }); + updateWithCallback(S, Baz, SourceContents, + WantDiagnostics::Yes, + [&BuiltASTCounter]() { ++BuiltASTCounter; }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); ASSERT_EQ(BuiltASTCounter.load(), 3); @@ -354,8 +405,8 @@ 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, 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) {}); + 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 +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) {}); + 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 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, @@ -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,11 +512,11 @@ )cpp"; // Return value indicates if the updated callback was received. - auto DoUpdate = [&](ParseInputs Inputs) -> bool { + auto DoUpdate = [&](std::string Contents) -> bool { std::atomic Updated(false); Updated = false; - S.update(Source, std::move(Inputs), WantDiagnostics::Yes, - [&Updated](std::vector) { Updated = true; }); + updateWithDiags(S, Source, Contents, 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?"; @@ -476,40 +524,41 @@ }; // 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) { ADD_FAILURE() << "Should not be called."; }); + updateWithDiags( + S, 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 +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 SeenDiags(false); - S.update(FooCpp, getInputs(FooCpp, Contents), WantDiagnostics::Auto, - [&](std::vector) { SeenDiags = true; }); + updateWithDiags(S, 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, Contents, WantDiagnostics::Auto, [&](std::vector) { ADD_FAILURE() << "Should not be called."; }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); }