From 0bb24cd4fabb15c4d17adb26db08bc1ef9de9920 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Tue, 13 Feb 2018 08:59:23 +0000 Subject: [PATCH] [clangd] Stop exposing Futures from ClangdServer operations. Summary: LSP has asynchronous semantics, being able to block on an async operation completing is unneccesary and leads to tighter coupling with the threading. In practice only tests depend on this, so we add a general-purpose "block until idle" function to the scheduler which will work for all operations. To get this working, fix a latent condition-variable bug in ASTWorker, and make AsyncTaskRunner const-correct. Reviewers: ilya-biryukov Subscribers: klimek, jkorous-apple, ioeric, cfe-commits Differential Revision: https://reviews.llvm.org/D43127 llvm-svn: 324990 --- clang-tools-extra/clangd/ClangdServer.cpp | 28 ++++--- clang-tools-extra/clangd/ClangdServer.h | 17 ++-- clang-tools-extra/clangd/TUScheduler.cpp | 49 +++++++++--- clang-tools-extra/clangd/TUScheduler.h | 7 +- clang-tools-extra/clangd/Threading.cpp | 18 ++++- clang-tools-extra/clangd/Threading.h | 22 +++++- clang-tools-extra/unittests/clangd/ClangdTests.cpp | 90 ++++++++-------------- .../unittests/clangd/CodeCompleteTests.cpp | 27 ++++--- .../unittests/clangd/ThreadingTests.cpp | 2 +- 9 files changed, 149 insertions(+), 111 deletions(-) diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 7090bc1..f8e07f9 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -139,11 +139,11 @@ void ClangdServer::setRootPath(PathRef RootPath) { this->RootPath = NewRootPath; } -std::future ClangdServer::addDocument(PathRef File, StringRef Contents) { +void ClangdServer::addDocument(PathRef File, StringRef Contents) { DocVersion Version = DraftMgr.updateDraft(File, Contents); auto TaggedFS = FSProvider.getTaggedFileSystem(File); - return scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()}, - std::move(TaggedFS)); + scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()}, + std::move(TaggedFS)); } void ClangdServer::removeDocument(PathRef File) { @@ -152,7 +152,7 @@ void ClangdServer::removeDocument(PathRef File) { WorkScheduler.remove(File); } -std::future ClangdServer::forceReparse(PathRef File) { +void ClangdServer::forceReparse(PathRef File) { auto FileContents = DraftMgr.getDraft(File); assert(FileContents.Draft && "forceReparse() was called for non-added document"); @@ -162,8 +162,7 @@ std::future ClangdServer::forceReparse(PathRef File) { CompileArgs.invalidate(File); auto TaggedFS = FSProvider.getTaggedFileSystem(File); - return scheduleReparseAndDiags(File, std::move(FileContents), - std::move(TaggedFS)); + scheduleReparseAndDiags(File, std::move(FileContents), std::move(TaggedFS)); } void ClangdServer::codeComplete( @@ -463,7 +462,7 @@ ClangdServer::findDocumentHighlights(PathRef File, Position Pos) { return blockingRunWithAST(WorkScheduler, File, Action); } -std::future ClangdServer::scheduleReparseAndDiags( +void ClangdServer::scheduleReparseAndDiags( PathRef File, VersionedDraft Contents, Tagged> TaggedFS) { tooling::CompileCommand Command = CompileArgs.getCompileCommand(File); @@ -474,12 +473,7 @@ std::future ClangdServer::scheduleReparseAndDiags( Path FileStr = File.str(); VFSTag Tag = std::move(TaggedFS.Tag); - std::promise DonePromise; - std::future DoneFuture = DonePromise.get_future(); - - auto Callback = [this, Version, FileStr, Tag](std::promise DonePromise, - OptDiags Diags) { - auto Guard = llvm::make_scope_exit([&]() { DonePromise.set_value(); }); + auto Callback = [this, Version, FileStr, Tag](OptDiags Diags) { if (!Diags) return; // A new reparse was requested before this one completed. @@ -503,8 +497,7 @@ std::future ClangdServer::scheduleReparseAndDiags( ParseInputs{std::move(Command), std::move(TaggedFS.Value), std::move(*Contents.Draft)}, - BindWithForward(Callback, std::move(DonePromise))); - return DoneFuture; + std::move(Callback)); } void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { @@ -516,3 +509,8 @@ std::vector> ClangdServer::getUsedBytesPerFile() const { return WorkScheduler.getUsedBytesPerFile(); } + +LLVM_NODISCARD bool +ClangdServer::blockUntilIdleForTest(llvm::Optional TimeoutSeconds) { + return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds)); +} diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index c3ceaa0..7445ff2 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -150,11 +150,7 @@ public: /// \p File is already tracked. Also schedules parsing of the AST for it on a /// separate thread. When the parsing is complete, DiagConsumer passed in /// constructor will receive onDiagnosticsReady callback. - /// \return A future that will become ready when the rebuild (including - /// diagnostics) is finished. - /// FIXME: don't return futures here, LSP does not require a response for this - /// request. - std::future addDocument(PathRef File, StringRef Contents); + void addDocument(PathRef File, StringRef Contents); /// Remove \p File from list of tracked files, schedule a request to free /// resources associated with it. @@ -164,9 +160,7 @@ public: /// Will also check if CompileCommand, provided by GlobalCompilationDatabase /// for \p File has changed. If it has, will remove currently stored Preamble /// and AST and rebuild them from scratch. - /// FIXME: don't return futures here, LSP does not require a response for this - /// request. - std::future forceReparse(PathRef File); + void forceReparse(PathRef File); /// Run code completion for \p File at \p Pos. /// Request is processed asynchronously. @@ -252,6 +246,11 @@ public: /// FIXME: those metrics might be useful too, we should add them. std::vector> getUsedBytesPerFile() const; + // Blocks the main thread until the server is idle. Only for use in tests. + // Returns false if the timeout expires. + LLVM_NODISCARD bool + blockUntilIdleForTest(llvm::Optional TimeoutSeconds = 10); + private: /// FIXME: This stats several files to find a .clang-format file. I/O can be /// slow. Think of a way to cache this. @@ -259,7 +258,7 @@ private: formatCode(llvm::StringRef Code, PathRef File, ArrayRef Ranges); - std::future + void scheduleReparseAndDiags(PathRef File, VersionedDraft Contents, Tagged> TaggedFS); diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index ed9cbd2..b2330ed 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -83,6 +83,7 @@ public: UniqueFunction>)> OnUpdated); void runWithAST(UniqueFunction)> Action); + bool blockUntilIdle(Deadline Timeout) const; std::shared_ptr getPossiblyStalePreamble() const; std::size_t getUsedBytes() const; @@ -117,7 +118,7 @@ private: // Only set when last request is an update. This allows us to cancel an update // that was never read, if a subsequent update comes in. llvm::Optional LastUpdateCF; /* GUARDED_BY(Mutex) */ - std::condition_variable RequestsCV; + mutable std::condition_variable RequestsCV; }; /// A smart-pointer-like class that points to an active ASTWorker. @@ -253,7 +254,7 @@ void ASTWorker::stop() { assert(!Done && "stop() called twice"); Done = true; } - RequestsCV.notify_one(); + RequestsCV.notify_all(); } void ASTWorker::startTask(UniqueFunction Task, bool isUpdate, @@ -282,7 +283,7 @@ void ASTWorker::startTask(UniqueFunction Task, bool isUpdate, } Requests.emplace(std::move(Task), Context::current().clone()); } // unlock Mutex. - RequestsCV.notify_one(); + RequestsCV.notify_all(); } void ASTWorker::run() { @@ -299,14 +300,26 @@ void ASTWorker::run() { // before exiting the processing loop. Req = std::move(Requests.front()); - Requests.pop(); + // Leave it on the queue for now, so waiters don't see an empty queue. } // unlock Mutex std::lock_guard BarrierLock(Barrier); WithContext Guard(std::move(Req.second)); Req.first(); + + { + std::lock_guard Lock(Mutex); + Requests.pop(); + } + RequestsCV.notify_all(); } } + +bool ASTWorker::blockUntilIdle(Deadline Timeout) const { + std::unique_lock Lock(Mutex); + return wait(Lock, RequestsCV, Timeout, [&] { return Requests.empty(); }); +} + } // namespace unsigned getDefaultAsyncThreadsCount() { @@ -331,8 +344,10 @@ TUScheduler::TUScheduler(unsigned AsyncThreadsCount, : StorePreamblesInMemory(StorePreamblesInMemory), PCHOps(std::make_shared()), ASTCallback(std::move(ASTCallback)), Barrier(AsyncThreadsCount) { - if (0 < AsyncThreadsCount) - Tasks.emplace(); + if (0 < AsyncThreadsCount) { + PreambleTasks.emplace(); + WorkerThreads.emplace(); + } } TUScheduler::~TUScheduler() { @@ -340,8 +355,20 @@ TUScheduler::~TUScheduler() { Files.clear(); // Wait for all in-flight tasks to finish. - if (Tasks) - Tasks->waitForAll(); + if (PreambleTasks) + PreambleTasks->wait(); + if (WorkerThreads) + WorkerThreads->wait(); +} + +bool TUScheduler::blockUntilIdle(Deadline D) const { + for (auto &File : Files) + if (!File.getValue()->Worker->blockUntilIdle(D)) + return false; + if (PreambleTasks) + if (!PreambleTasks->wait(D)) + return false; + return true; } void TUScheduler::update( @@ -352,7 +379,7 @@ void TUScheduler::update( if (!FD) { // Create a new worker to process the AST-related tasks. ASTWorkerHandle Worker = ASTWorker::Create( - Tasks ? Tasks.getPointer() : nullptr, Barrier, + WorkerThreads ? WorkerThreads.getPointer() : nullptr, Barrier, CppFile(File, StorePreamblesInMemory, PCHOps, ASTCallback)); FD = std::unique_ptr(new FileData{Inputs, std::move(Worker)}); } else { @@ -392,7 +419,7 @@ void TUScheduler::runWithPreamble( return; } - if (!Tasks) { + if (!PreambleTasks) { std::shared_ptr Preamble = It->second->Worker->getPossiblyStalePreamble(); Action(InputsAndPreamble{It->second->Inputs, Preamble.get()}); @@ -410,7 +437,7 @@ void TUScheduler::runWithPreamble( Action(InputsAndPreamble{InputsCopy, Preamble.get()}); }; - Tasks->runAsync( + PreambleTasks->runAsync( BindWithForward(Task, Context::current().clone(), std::move(Action))); } diff --git a/clang-tools-extra/clangd/TUScheduler.h b/clang-tools-extra/clangd/TUScheduler.h index c984b71..33151c9 100644 --- a/clang-tools-extra/clangd/TUScheduler.h +++ b/clang-tools-extra/clangd/TUScheduler.h @@ -77,6 +77,10 @@ public: PathRef File, UniqueFunction)> Action); + /// Wait until there are no scheduled or running tasks. + /// Mostly useful for synchronizing tests. + bool blockUntilIdle(Deadline D) const; + private: /// This class stores per-file data in the Files map. struct FileData; @@ -88,7 +92,8 @@ private: llvm::StringMap> Files; // None when running tasks synchronously and non-None when running tasks // asynchronously. - llvm::Optional Tasks; + llvm::Optional PreambleTasks; + llvm::Optional WorkerThreads; }; } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Threading.cpp b/clang-tools-extra/clangd/Threading.cpp index b067758..a238a47 100644 --- a/clang-tools-extra/clangd/Threading.cpp +++ b/clang-tools-extra/clangd/Threading.cpp @@ -26,16 +26,17 @@ void Semaphore::unlock() { SlotsChanged.notify_one(); } -AsyncTaskRunner::~AsyncTaskRunner() { waitForAll(); } +AsyncTaskRunner::~AsyncTaskRunner() { wait(); } -void AsyncTaskRunner::waitForAll() { +bool AsyncTaskRunner::wait(Deadline D) const { std::unique_lock Lock(Mutex); - TasksReachedZero.wait(Lock, [&]() { return InFlightTasks == 0; }); + return clangd::wait(Lock, TasksReachedZero, D, + [&] { return InFlightTasks == 0; }); } void AsyncTaskRunner::runAsync(UniqueFunction Action) { { - std::unique_lock Lock(Mutex); + std::lock_guard Lock(Mutex); ++InFlightTasks; } @@ -59,5 +60,14 @@ void AsyncTaskRunner::runAsync(UniqueFunction Action) { std::move(Action), std::move(CleanupTask)) .detach(); } + +Deadline timeoutSeconds(llvm::Optional Seconds) { + using namespace std::chrono; + if (!Seconds) + return llvm::None; + return steady_clock::now() + + duration_cast(duration(*Seconds)); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/Threading.h b/clang-tools-extra/clangd/Threading.h index a24eed7..9f674df 100644 --- a/clang-tools-extra/clangd/Threading.h +++ b/clang-tools-extra/clangd/Threading.h @@ -56,6 +56,21 @@ private: std::size_t FreeSlots; }; +/// A point in time we may wait for, or None to wait forever. +/// (Not time_point::max(), because many std::chrono implementations overflow). +using Deadline = llvm::Optional; +/// Makes a deadline from a timeout in seconds. +Deadline timeoutSeconds(llvm::Optional Seconds); +/// Waits on a condition variable until F() is true or D expires. +template +LLVM_NODISCARD bool wait(std::unique_lock &Lock, + std::condition_variable &CV, Deadline D, Func F) { + if (D) + return CV.wait_until(Lock, *D, F); + CV.wait(Lock, F); + return true; +} + /// Runs tasks on separate (detached) threads and wait for all tasks to finish. /// Objects that need to spawn threads can own an AsyncTaskRunner to ensure they /// all complete on destruction. @@ -64,12 +79,13 @@ public: /// Destructor waits for all pending tasks to finish. ~AsyncTaskRunner(); - void waitForAll(); + void wait() const { (void) wait(llvm::None); } + LLVM_NODISCARD bool wait(Deadline D) const; void runAsync(UniqueFunction Action); private: - std::mutex Mutex; - std::condition_variable TasksReachedZero; + mutable std::mutex Mutex; + mutable std::condition_variable TasksReachedZero; std::size_t InFlightTasks = 0; }; } // namespace clangd diff --git a/clang-tools-extra/unittests/clangd/ClangdTests.cpp b/clang-tools-extra/unittests/clangd/ClangdTests.cpp index 6e6f223..65e5ea8 100644 --- a/clang-tools-extra/unittests/clangd/ClangdTests.cpp +++ b/clang-tools-extra/unittests/clangd/ClangdTests.cpp @@ -40,11 +40,6 @@ using ::testing::UnorderedElementsAre; namespace { -// Don't wait for async ops in clangd test more than that to avoid blocking -// indefinitely in case of bugs. -static const std::chrono::seconds DefaultFutureTimeout = - std::chrono::seconds(10); - static bool diagsContainErrors(ArrayRef Diagnostics) { for (const auto &DiagAndFixIts : Diagnostics) { // FIXME: severities returned by clangd should have a descriptive @@ -142,15 +137,9 @@ protected: FS.ExpectedFile = SourceFilename; - // Have to sync reparses because requests are processed on the calling - // thread. - auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents); - + Server.addDocument(SourceFilename, SourceContents); auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename); - - // Wait for reparse to finish before checking for errors. - EXPECT_EQ(AddDocFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); return Result; } @@ -210,25 +199,19 @@ int b = a; FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; - // To sync reparses before checking for errors. - std::future ParseFuture; - - ParseFuture = Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); - ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); - ParseFuture = Server.addDocument(FooCpp, ""); + Server.addDocument(FooCpp, ""); auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp); - ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); - ParseFuture = Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); - ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); EXPECT_EQ(DumpParse1, DumpParse2); @@ -255,27 +238,21 @@ int b = a; FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; - // To sync reparses before checking for errors. - std::future ParseFuture; - - ParseFuture = Server.addDocument(FooCpp, SourceContents); + Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); - ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); FS.Files[FooH] = ""; - ParseFuture = Server.forceReparse(FooCpp); + Server.forceReparse(FooCpp); auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp); - ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); FS.Files[FooH] = "int a;"; - ParseFuture = Server.forceReparse(FooCpp); + Server.forceReparse(FooCpp); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); - EXPECT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), - std::future_status::ready); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); EXPECT_EQ(DumpParse1, DumpParse2); @@ -803,20 +780,24 @@ TEST_F(ClangdVFSTest, CheckSourceHeaderSwitch) { TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) { class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer { public: - NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) - : StartSecondReparse(std::move(StartSecondReparse)) {} + std::atomic Count = {0}; - void onDiagnosticsReady( - PathRef File, - Tagged> Diagnostics) override { + NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse) + : StartSecondReparse(std::move(StartSecondReparse)) {} + void onDiagnosticsReady(PathRef, + Tagged>) override { + ++Count; std::unique_lock Lock(Mutex, std::try_to_lock_t()); ASSERT_TRUE(Lock.owns_lock()) << "Detected concurrent onDiagnosticsReady calls for the same file."; + + // If we started the second parse immediately, it might cancel the first. + // So we don't allow it to start until the first has delivered diags... if (FirstRequest) { FirstRequest = false; StartSecondReparse.set_value(); - // Sleep long enough for the second request to be processed. + // ... but then we wait long enough that the callbacks would overlap. std::this_thread::sleep_for(std::chrono::milliseconds(50)); } } @@ -842,24 +823,21 @@ int d; )cpp"; auto FooCpp = getVirtualTestFilePath("foo.cpp"); - llvm::StringMap FileContents; - FileContents[FooCpp] = ""; - ConstantFSProvider FS(buildTestFS(FileContents)); - - std::promise StartSecondReparsePromise; - std::future StartSecondReparse = StartSecondReparsePromise.get_future(); + MockFSProvider FS; + FS.Files[FooCpp] = ""; - NoConcurrentAccessDiagConsumer DiagConsumer( - std::move(StartSecondReparsePromise)); + std::promise StartSecondPromise; + std::future StartSecond = StartSecondPromise.get_future(); + NoConcurrentAccessDiagConsumer DiagConsumer(std::move(StartSecondPromise)); MockCompilationDatabase CDB; - ClangdServer Server(CDB, DiagConsumer, FS, 4, + ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/4, /*StorePreamblesInMemory=*/true); Server.addDocument(FooCpp, SourceContentsWithErrors); - StartSecondReparse.wait(); - - auto Future = Server.addDocument(FooCpp, SourceContentsWithoutErrors); - Future.wait(); + StartSecond.wait(); + Server.addDocument(FooCpp, SourceContentsWithoutErrors); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics"; + ASSERT_EQ(DiagConsumer.Count, 2); // Sanity check - we actually ran both? } } // namespace clangd diff --git a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp index 9f456ee..b563761 100644 --- a/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp +++ b/clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp @@ -121,7 +121,8 @@ CompletionList completions(StringRef Text, /*StorePreamblesInMemory=*/true); auto File = getVirtualTestFilePath("foo.cpp"); Annotations Test(Text); - Server.addDocument(File, Test.code()).wait(); + Server.addDocument(File, Test.code()); + EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble"; auto CompletionList = runCodeComplete(Server, File, Test.point(), Opts).Value; // Sanity-check that filterText is valid. EXPECT_THAT(CompletionList.items, Each(NameContainsFilter())); @@ -551,13 +552,10 @@ TEST(CompletionTest, IndexSuppressesPreambleCompletions) { void f() { ns::^; } void f() { ns::preamble().$2^; } )cpp"); - Server.addDocument(File, Test.code()).wait(); + Server.addDocument(File, Test.code()); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble"; clangd::CodeCompleteOptions Opts = {}; - auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value; - EXPECT_THAT(WithoutIndex.items, - UnorderedElementsAre(Named("local"), Named("preamble"))); - auto I = memIndex({var("ns::index")}); Opts.Index = I.get(); auto WithIndex = runCodeComplete(Server, File, Test.point(), Opts).Value; @@ -566,6 +564,12 @@ TEST(CompletionTest, IndexSuppressesPreambleCompletions) { auto ClassFromPreamble = runCodeComplete(Server, File, Test.point("2"), Opts).Value; EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member"))); + + Opts.Index = nullptr; + auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value; + EXPECT_THAT(WithoutIndex.items, + UnorderedElementsAre(Named("local"), Named("preamble"))); + } TEST(CompletionTest, DynamicIndexMultiFile) { @@ -576,11 +580,10 @@ TEST(CompletionTest, DynamicIndexMultiFile) { /*StorePreamblesInMemory=*/true, /*BuildDynamicSymbolIndex=*/true); - Server - .addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp( + Server.addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp( namespace ns { class XYZ {}; void foo(int x) {} } - )cpp") - .wait(); + )cpp"); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble"; auto File = getVirtualTestFilePath("bar.cpp"); Annotations Test(R"cpp( @@ -591,7 +594,8 @@ TEST(CompletionTest, DynamicIndexMultiFile) { } void f() { ns::^ } )cpp"); - Server.addDocument(File, Test.code()).wait(); + Server.addDocument(File, Test.code()); + ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble"; auto Results = runCodeComplete(Server, File, Test.point(), {}).Value; // "XYZ" and "foo" are not included in the file being completed but are still @@ -621,6 +625,7 @@ SignatureHelp signatures(StringRef Text) { auto File = getVirtualTestFilePath("foo.cpp"); Annotations Test(Text); Server.addDocument(File, Test.code()); + EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble"; auto R = Server.signatureHelp(File, Test.point()); assert(R); return R.get().Value; diff --git a/clang-tools-extra/unittests/clangd/ThreadingTests.cpp b/clang-tools-extra/unittests/clangd/ThreadingTests.cpp index 84e6512..49c3ebe 100644 --- a/clang-tools-extra/unittests/clangd/ThreadingTests.cpp +++ b/clang-tools-extra/unittests/clangd/ThreadingTests.cpp @@ -45,7 +45,7 @@ TEST_F(ThreadingTest, TaskRunner) { scheduleIncrements(); } - Tasks.waitForAll(); + Tasks.wait(); { std::lock_guard Lock(Mutex); ASSERT_EQ(Counter, TasksCnt * IncrementsPerTask); -- 2.7.4