[clang-tools-extra] r324990 - [clangd] Stop exposing Futures from ClangdServer operations.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 13 00:59:23 PST 2018
Author: sammccall
Date: Tue Feb 13 00:59:23 2018
New Revision: 324990
URL: http://llvm.org/viewvc/llvm-project?rev=324990&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Feb 13 00:59:23 2018
@@ -139,11 +139,11 @@ void ClangdServer::setRootPath(PathRef R
this->RootPath = NewRootPath;
}
-std::future<void> 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(PathRe
WorkScheduler.remove(File);
}
-std::future<void> 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<void> ClangdServer::forceRep
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(Pat
return blockingRunWithAST<RetType>(WorkScheduler, File, Action);
}
-std::future<void> ClangdServer::scheduleReparseAndDiags(
+void ClangdServer::scheduleReparseAndDiags(
PathRef File, VersionedDraft Contents,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS) {
tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
@@ -474,12 +473,7 @@ std::future<void> ClangdServer::schedule
Path FileStr = File.str();
VFSTag Tag = std::move(TaggedFS.Tag);
- std::promise<void> DonePromise;
- std::future<void> DoneFuture = DonePromise.get_future();
-
- auto Callback = [this, Version, FileStr, Tag](std::promise<void> 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<void> ClangdServer::schedule
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<std::pair<Path, std::size_t>
ClangdServer::getUsedBytesPerFile() const {
return WorkScheduler.getUsedBytesPerFile();
}
+
+LLVM_NODISCARD bool
+ClangdServer::blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds) {
+ return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds));
+}
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Feb 13 00:59:23 2018
@@ -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<void> 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<void> 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<std::pair<Path, std::size_t>> 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<double> 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<tooling::Range> Ranges);
- std::future<void>
+ void
scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
Tagged<IntrusiveRefCntPtr<vfs::FileSystem>> TaggedFS);
Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Tue Feb 13 00:59:23 2018
@@ -83,6 +83,7 @@ public:
UniqueFunction<void(llvm::Optional<std::vector<DiagWithFixIts>>)>
OnUpdated);
void runWithAST(UniqueFunction<void(llvm::Expected<InputsAndAST>)> Action);
+ bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr<const PreambleData> 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<CancellationFlag> 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<void()> Task, bool isUpdate,
@@ -282,7 +283,7 @@ void ASTWorker::startTask(UniqueFunction
}
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<Semaphore> BarrierLock(Barrier);
WithContext Guard(std::move(Req.second));
Req.first();
+
+ {
+ std::lock_guard<std::mutex> Lock(Mutex);
+ Requests.pop();
+ }
+ RequestsCV.notify_all();
}
}
+
+bool ASTWorker::blockUntilIdle(Deadline Timeout) const {
+ std::unique_lock<std::mutex> Lock(Mutex);
+ return wait(Lock, RequestsCV, Timeout, [&] { return Requests.empty(); });
+}
+
} // namespace
unsigned getDefaultAsyncThreadsCount() {
@@ -331,8 +344,10 @@ TUScheduler::TUScheduler(unsigned AsyncT
: StorePreamblesInMemory(StorePreamblesInMemory),
PCHOps(std::make_shared<PCHContainerOperations>()),
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<FileData>(new FileData{Inputs, std::move(Worker)});
} else {
@@ -392,7 +419,7 @@ void TUScheduler::runWithPreamble(
return;
}
- if (!Tasks) {
+ if (!PreambleTasks) {
std::shared_ptr<const PreambleData> 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)));
}
Modified: clang-tools-extra/trunk/clangd/TUScheduler.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.h?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/TUScheduler.h (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.h Tue Feb 13 00:59:23 2018
@@ -77,6 +77,10 @@ public:
PathRef File,
UniqueFunction<void(llvm::Expected<InputsAndPreamble>)> 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<std::unique_ptr<FileData>> Files;
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
- llvm::Optional<AsyncTaskRunner> Tasks;
+ llvm::Optional<AsyncTaskRunner> PreambleTasks;
+ llvm::Optional<AsyncTaskRunner> WorkerThreads;
};
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Tue Feb 13 00:59:23 2018
@@ -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<std::mutex> Lock(Mutex);
- TasksReachedZero.wait(Lock, [&]() { return InFlightTasks == 0; });
+ return clangd::wait(Lock, TasksReachedZero, D,
+ [&] { return InFlightTasks == 0; });
}
void AsyncTaskRunner::runAsync(UniqueFunction<void()> Action) {
{
- std::unique_lock<std::mutex> Lock(Mutex);
+ std::lock_guard<std::mutex> Lock(Mutex);
++InFlightTasks;
}
@@ -59,5 +60,14 @@ void AsyncTaskRunner::runAsync(UniqueFun
std::move(Action), std::move(CleanupTask))
.detach();
}
+
+Deadline timeoutSeconds(llvm::Optional<double> Seconds) {
+ using namespace std::chrono;
+ if (!Seconds)
+ return llvm::None;
+ return steady_clock::now() +
+ duration_cast<steady_clock::duration>(duration<double>(*Seconds));
+}
+
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/Threading.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.h?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Threading.h (original)
+++ clang-tools-extra/trunk/clangd/Threading.h Tue Feb 13 00:59:23 2018
@@ -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<std::chrono::steady_clock::time_point>;
+/// Makes a deadline from a timeout in seconds.
+Deadline timeoutSeconds(llvm::Optional<double> Seconds);
+/// Waits on a condition variable until F() is true or D expires.
+template <typename Func>
+LLVM_NODISCARD bool wait(std::unique_lock<std::mutex> &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<void()> 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
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Tue Feb 13 00:59:23 2018
@@ -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<DiagWithFixIts> 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<void> 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<void> 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, CheckSourceHeaderS
TEST_F(ClangdThreadingTest, NoConcurrentDiagnostics) {
class NoConcurrentAccessDiagConsumer : public DiagnosticsConsumer {
public:
- NoConcurrentAccessDiagConsumer(std::promise<void> StartSecondReparse)
- : StartSecondReparse(std::move(StartSecondReparse)) {}
+ std::atomic<int> Count = {0};
- void onDiagnosticsReady(
- PathRef File,
- Tagged<std::vector<DiagWithFixIts>> Diagnostics) override {
+ NoConcurrentAccessDiagConsumer(std::promise<void> StartSecondReparse)
+ : StartSecondReparse(std::move(StartSecondReparse)) {}
+ void onDiagnosticsReady(PathRef,
+ Tagged<std::vector<DiagWithFixIts>>) override {
+ ++Count;
std::unique_lock<std::mutex> 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<std::string> FileContents;
- FileContents[FooCpp] = "";
- ConstantFSProvider FS(buildTestFS(FileContents));
-
- std::promise<void> StartSecondReparsePromise;
- std::future<void> StartSecondReparse = StartSecondReparsePromise.get_future();
+ MockFSProvider FS;
+ FS.Files[FooCpp] = "";
- NoConcurrentAccessDiagConsumer DiagConsumer(
- std::move(StartSecondReparsePromise));
+ std::promise<void> StartSecondPromise;
+ std::future<void> 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
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Tue Feb 13 00:59:23 2018
@@ -121,7 +121,8 @@ CompletionList completions(StringRef Tex
/*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, IndexSuppressesPrea
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, IndexSuppressesPrea
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, DynamicIndexMultiFi
/*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, DynamicIndexMultiFi
}
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;
Modified: clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp?rev=324990&r1=324989&r2=324990&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp Tue Feb 13 00:59:23 2018
@@ -45,7 +45,7 @@ TEST_F(ThreadingTest, TaskRunner) {
scheduleIncrements();
}
- Tasks.waitForAll();
+ Tasks.wait();
{
std::lock_guard<std::mutex> Lock(Mutex);
ASSERT_EQ(Counter, TasksCnt * IncrementsPerTask);
More information about the cfe-commits
mailing list