diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 27c9089e2f3c..24363f1eeac4 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -65,26 +65,32 @@ public: Optional> Result; }; -} // namespace -// Returns callbacks that can be used to update the FileIndex with new ASTs. -static std::unique_ptr -makeUpdateCallbacks(FileIndex *FIndex) { - struct CB : public ParsingCallbacks { - CB(FileIndex *FIndex) : FIndex(FIndex) {} - FileIndex *FIndex; +// Update the FileIndex with new ASTs and plumb the diagnostics responses. +struct UpdateIndexCallbacks : public ParsingCallbacks { + UpdateIndexCallbacks(FileIndex *FIndex, DiagnosticsConsumer &DiagConsumer) + : FIndex(FIndex), DiagConsumer(DiagConsumer) {} - void onPreambleAST(PathRef Path, ASTContext &Ctx, - std::shared_ptr PP) override { + void onPreambleAST(PathRef Path, ASTContext &Ctx, + std::shared_ptr PP) override { + if (FIndex) FIndex->updatePreamble(Path, Ctx, std::move(PP)); - } + } - void onMainAST(PathRef Path, ParsedAST &AST) override { + void onMainAST(PathRef Path, ParsedAST &AST) override { + if (FIndex) FIndex->updateMain(Path, AST); - } - }; - return llvm::make_unique(FIndex); -} + } + + void onDiagnostics(PathRef File, std::vector Diags) override { + DiagConsumer.onDiagnosticsReady(File, std::move(Diags)); + } + +private: + FileIndex *FIndex; + DiagnosticsConsumer &DiagConsumer; +}; +} // namespace ClangdServer::Options ClangdServer::optsForTest() { ClangdServer::Options Opts; @@ -98,7 +104,7 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, const FileSystemProvider &FSProvider, DiagnosticsConsumer &DiagConsumer, const Options &Opts) - : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), + : CDB(CDB), FSProvider(FSProvider), ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir : getStandardResourceDir()), DynamicIdx(Opts.BuildDynamicSymbolIndex @@ -112,8 +118,8 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, // 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, + llvm::make_unique(DynamicIdx.get(), + DiagConsumer), Opts.UpdateDebounce, Opts.RetentionPolicy) { if (DynamicIdx && Opts.StaticIndex) { MergedIdx = @@ -129,14 +135,10 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB, void ClangdServer::addDocument(PathRef File, StringRef Contents, WantDiagnostics WantDiags) { - ParseInputs Inputs = {getCompileCommand(File), FSProvider.getFileSystem(), - Contents.str()}; - Path FileStr = File.str(); - WorkScheduler.update(File, std::move(Inputs), WantDiags, - [this, FileStr](std::vector Diags) { - DiagConsumer.onDiagnosticsReady(FileStr, - std::move(Diags)); - }); + WorkScheduler.update(File, + ParseInputs{getCompileCommand(File), + FSProvider.getFileSystem(), Contents.str()}, + WantDiags); } void ClangdServer::removeDocument(PathRef File) { diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index 5c829ae83132..d67ad1e5e29f 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -226,7 +226,6 @@ private: tooling::CompileCommand getCompileCommand(PathRef File); const GlobalCompilationDatabase &CDB; - DiagnosticsConsumer &DiagConsumer; const FileSystemProvider &FSProvider; Path ResourceDir; diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 8bd3b8b41dab..c33e3a51b0de 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -176,8 +176,7 @@ public: 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; @@ -231,7 +230,7 @@ private: 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; @@ -340,9 +339,8 @@ ASTWorker::~ASTWorker() { #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) == @@ -436,7 +434,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, { std::lock_guard Lock(DiagsMu); if (ReportDiagnostics) - OnUpdated((*AST)->getDiagnostics()); + Callbacks.onDiagnostics(FileName, (*AST)->getDiagnostics()); } trace::Span Span("Running main AST callback"); Callbacks.onMainAST(FileName, **AST); @@ -446,7 +444,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, IdleASTs.put(this, std::move(*AST)); }; - startTask("Update", Bind(Task, std::move(OnUpdated)), WantDiags); + startTask("Update", std::move(Task), WantDiags); } void ASTWorker::runWithAST( @@ -742,8 +740,7 @@ bool TUScheduler::blockUntilIdle(Deadline D) const { } 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. @@ -756,7 +753,7 @@ void TUScheduler::update(PathRef File, ParseInputs Inputs, 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) { diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index 9dd4f3732a81..2606d51c0a62 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -72,6 +72,9 @@ public: /// 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., @@ -103,9 +106,7 @@ public: /// If diagnostics are requested (Yes), and the context is cancelled before /// they are prepared, they may be skipped if eventual-consistency permits it /// (i.e. WantDiagnostics is downgraded to Auto). - /// 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. Pending diagnostics for closed files may not be delivered, even diff --git a/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp b/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp index e5f29141ebab..3908ae6d2ce9 100644 --- a/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp +++ b/clang-tools-extra/unittests/clangd/TUSchedulerTests.cpp @@ -11,6 +11,7 @@ #include "Matchers.h" #include "TUScheduler.h" #include "TestFS.h" +#include "llvm/ADT/ScopeExit.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include @@ -29,8 +30,6 @@ using ::testing::Pair; using ::testing::Pointee; using ::testing::UnorderedElementsAre; -void ignoreUpdate(Optional>) {} - class TUSchedulerTests : public ::testing::Test { protected: ParseInputs getInputs(PathRef File, std::string Contents) { @@ -38,11 +37,64 @@ protected: 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, @@ -55,7 +107,7 @@ TEST_F(TUSchedulerTests, MissingFiles) { 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). @@ -96,25 +148,25 @@ TEST_F(TUSchedulerTests, WantDiagnostics) { 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(); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); @@ -126,21 +178,22 @@ TEST_F(TUSchedulerTests, Debounce) { 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; }); ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10))); } @@ -168,22 +221,20 @@ TEST_F(TUSchedulerTests, PreambleConsistency) { // 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. + 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, - [&](std::vector Diags) {}); + // 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) { @@ -220,7 +271,7 @@ TEST_F(TUSchedulerTests, Cancellation) { Notification Proceed; // Ensure we schedule everything. TUScheduler S( getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true, - /*ASTCallbacks=*/nullptr, + /*ASTCallbacks=*/captureDiags(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); auto Path = testPath("foo.cpp"); @@ -228,8 +279,9 @@ TEST_F(TUSchedulerTests, Cancellation) { auto Update = [&](std::string ID) -> Canceler { auto T = cancelableTask(); WithContext C(std::move(T.first)); - S.update(Path, getInputs(Path, "//" + ID), WantDiagnostics::Yes, - [&, ID](std::vector Diags) { DiagsSeen.push_back(ID); }); + updateWithDiags( + S, Path, "//" + ID, WantDiagnostics::Yes, + [&, ID](std::vector Diags) { DiagsSeen.push_back(ID); }); return std::move(T.second); }; // Helper to schedule a named read and return a function to cancel it. @@ -252,8 +304,8 @@ TEST_F(TUSchedulerTests, Cancellation) { return std::move(T.second); }; - S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes, - [&](std::vector Diags) { Proceed.wait(); }); + updateWithCallback(S, Path, "", WantDiagnostics::Yes, + [&]() { Proceed.wait(); }); // The second parens indicate cancellation, where present. Update("U1")(); Read("R1")(); @@ -288,7 +340,7 @@ TEST_F(TUSchedulerTests, ManyUpdates) { // Run TUScheduler and collect some stats. { TUScheduler S(getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/true, /*ASTCallbacks=*/nullptr, + /*StorePreamblesInMemory=*/true, captureDiags(), /*UpdateDebounce=*/std::chrono::milliseconds(50), ASTRetentionPolicy()); @@ -317,22 +369,18 @@ TEST_F(TUSchedulerTests, ManyUpdates) { 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)); + 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()); - }); + std::lock_guard Lock(Mut); + ++TotalUpdates; + EXPECT_EQ(File, *TUScheduler::getFileBeingProcessedInContext()); + }); } - { WithContextValue WithNonce(NonceKey, ++Nonce); S.runWithAST("CheckAST", File, @@ -404,17 +452,17 @@ TEST_F(TUSchedulerTests, EvictedAST) { // 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); @@ -422,8 +470,8 @@ TEST_F(TUSchedulerTests, EvictedAST) { 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); @@ -450,8 +498,7 @@ TEST_F(TUSchedulerTests, EmptyPreamble) { 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. @@ -464,8 +511,7 @@ TEST_F(TUSchedulerTests, EmptyPreamble) { 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, @@ -496,8 +542,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) { 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, @@ -516,7 +561,7 @@ TEST_F(TUSchedulerTests, RunWaitsForPreamble) { TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { TUScheduler S( /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(), - /*StorePreambleInMemory=*/true, /*ASTCallbacks=*/nullptr, + /*StorePreambleInMemory=*/true, captureDiags(), /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), ASTRetentionPolicy()); @@ -532,11 +577,11 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { )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?"; @@ -544,40 +589,41 @@ TEST_F(TUSchedulerTests, NoopOnEmptyChanges) { }; // 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)); @@ -587,15 +633,15 @@ TEST_F(TUSchedulerTests, NoChangeDiags) { // 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))); }