Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -98,21 +98,20 @@ class ClangdServer; -/// Returns a number of a default async threads to use for ClangdScheduler. +/// Returns a number of a default async threads to use for Scheduler. /// Returned value is always >= 1 (i.e. will not cause requests to be processed /// synchronously). unsigned getDefaultAsyncThreadsCount(); -/// Handles running WorkerRequests of ClangdServer on a number of worker -/// threads. -class ClangdScheduler { +/// A simple fixed-size thread pool implementation. +class SimpleThreadPool { public: /// If \p AsyncThreadsCount is 0, requests added using addToFront and addToEnd /// will be processed synchronously on the calling thread. // Otherwise, \p AsyncThreadsCount threads will be created to schedule the // requests. - ClangdScheduler(unsigned AsyncThreadsCount); - ~ClangdScheduler(); + SimpleThreadPool(unsigned AsyncThreadsCount); + ~SimpleThreadPool(); /// Add a new request to run function \p F with args \p As to the start of the /// queue. The request will be run on a separate thread. @@ -149,20 +148,64 @@ private: bool RunSynchronously; - std::mutex Mutex; + mutable std::mutex Mutex; /// We run some tasks on separate threads(parsing, CppFile cleanup). /// These threads looks into RequestQueue to find requests to handle and /// terminate when Done is set to true. std::vector Workers; /// Setting Done to true will make the worker threads terminate. bool Done = false; - /// A queue of requests. Elements of this vector are async computations (i.e. - /// results of calling std::async(std::launch::deferred, ...)). + /// A queue of requests. std::deque> RequestQueue; /// Condition variable to wake up worker threads. std::condition_variable RequestCV; }; +/// Handles running tasks for ClangdServer and managing the resources (e.g., +/// preambles and ASTs) for opened files. +class Scheduler { +public: + Scheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, + std::shared_ptr PCHs, + ASTParsedCallback ASTCallback); + + /// Get the CompileCommand passed at the last scheduleUpdate call for \p File. + llvm::Optional getLatestCommand(PathRef File) const; + + /// Schedule an update for \p File. Adds \p File to a list of tracked files if + /// \p File was not part of it before. + void scheduleUpdate( + Context Ctx, PathRef File, ParseInputs Inputs, + UniqueFunction>)> + OnUpdated); + + /// Remove \p File from the list of tracked files and schedule removal of its + /// resources. \p Action will be called when resources are freed. + /// If an error occurs during processing, it is forwarded to the \p Action + /// callback. + void scheduleRemove(PathRef File, UniqueFunction Action); + + /// Schedule an async read of the AST. \p Action will be called when AST is + /// ready. + /// If an error occurs during processing, it is forwarded to the \p Action + /// callback. If there was no error, AST is never null. + void + scheduleASTRead(PathRef File, + UniqueFunction)> Action); + + /// Schedule an async read of the Preamble. Preamble passed to \p Action may + /// be stale. + /// If an error occurs during processing, it is forwarded to the \p Action + /// callback. + void schedulePreambleRead( + PathRef File, + UniqueFunction)> Action); + +private: + CppFileCollection Units; + SimpleThreadPool Executor; +}; + /// Provides API to manage ASTs for a collection of C++ files and request /// various language features. /// Currently supports async diagnostics, code completion, formatting and goto @@ -330,11 +373,8 @@ std::future scheduleReparseAndDiags(Context Ctx, PathRef File, VersionedDraft Contents, - std::shared_ptr Resources, - Tagged> TaggedFS); - - std::future - scheduleCancelRebuild(Context Ctx, std::shared_ptr Resources); + Tagged> TaggedFS, + bool UpdateCompileCommand); GlobalCompilationDatabase &CDB; DiagnosticsConsumer &DiagConsumer; @@ -350,12 +390,10 @@ std::unique_ptr FileIdx; // If present, a merged view of FileIdx and an external index. Read via Index. std::unique_ptr MergedIndex; - CppFileCollection Units; std::string ResourceDir; // If set, this represents the workspace path. llvm::Optional RootPath; std::shared_ptr PCHs; - bool StorePreamblesInMemory; /// Used to serialize diagnostic callbacks. /// FIXME(ibiryukov): get rid of an extra map and put all version counters /// into CppFile. @@ -364,8 +402,8 @@ llvm::StringMap ReportedDiagnosticVersions; // WorkScheduler has to be the last member, because its destructor has to be // called before all other members to stop the worker thread that references - // ClangdServer - ClangdScheduler WorkScheduler; + // ClangdServer. + Scheduler WorkScheduler; }; } // namespace clangd Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -32,6 +32,38 @@ namespace { +// Issues an async read of AST and waits for results. +template +Ret waitForASTAction(Scheduler &S, PathRef File, Func &&F) { + std::packaged_task)> Task( + std::forward(F)); + auto Future = Task.get_future(); + S.scheduleASTRead(File, std::move(Task)); + return Future.get(); +} + +// Issues an async read of preamble and waits for results. +template +Ret waitForPreambleAction(Scheduler &S, PathRef File, Func &&F) { + std::packaged_task)> Task( + std::forward(F)); + auto Future = Task.get_future(); + S.schedulePreambleRead(File, std::move(Task)); + return Future.get(); +} + +tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB, + PathRef File, PathRef ResourceDir) { + llvm::Optional C = CDB.getCompileCommand(File); + if (!C) // FIXME: Suppress diagnostics? Let the user know? + C = CDB.getFallbackCommand(File); + + // Inject the resource dir. + // FIXME: Don't overwrite it if it's already there. + C->CommandLine.push_back("-resource-dir=" + ResourceDir.str()); + return std::move(*C); +} + std::string getStandardResourceDir() { static int Dummy; // Just an address in this process. return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy); @@ -75,8 +107,7 @@ return 1; return HardwareConcurrency; } - -ClangdScheduler::ClangdScheduler(unsigned AsyncThreadsCount) +SimpleThreadPool::SimpleThreadPool(unsigned AsyncThreadsCount) : RunSynchronously(AsyncThreadsCount == 0) { if (RunSynchronously) { // Don't start the worker thread if we're running synchronously @@ -102,7 +133,7 @@ assert(!RequestQueue.empty() && "RequestQueue was empty"); // We process requests starting from the front of the queue. Users of - // ClangdScheduler have a way to prioritise their requests by putting + // SimpleThreadPool have a way to prioritise their requests by putting // them to the either side of the queue (using either addToEnd or // addToFront). Request = std::move(RequestQueue.front()); @@ -115,7 +146,7 @@ } } -ClangdScheduler::~ClangdScheduler() { +SimpleThreadPool::~SimpleThreadPool() { if (RunSynchronously) return; // no worker thread is running in that case @@ -130,6 +161,99 @@ Worker.join(); } +Scheduler::Scheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory, + std::shared_ptr PCHs, + ASTParsedCallback ASTCallback) + : Units(StorePreamblesInMemory, std::move(PCHs), std::move(ASTCallback)), + Executor(AsyncThreadsCount) {} + +llvm::Optional +Scheduler::getLatestCommand(PathRef File) const { + auto Resources = Units.getFile(File); + if (!Resources) + return llvm::None; + return Resources->getLatestCommand(); +} + +void Scheduler::scheduleUpdate( + Context Ctx, PathRef File, ParseInputs Inputs, + UniqueFunction>)> + OnUpdated) { + auto Resources = Units.getOrCreateFile(File); + auto DeferredRebuild = Resources->deferRebuild(std::move(Inputs)); + + Executor.addToFront( + [](Context Ctx, decltype(OnUpdated) OnUpdated, + decltype(DeferredRebuild) DeferredRebuild) { + auto Diags = DeferredRebuild(Ctx); + OnUpdated(std::move(Ctx), Diags); + }, + std::move(Ctx), std::move(OnUpdated), std::move(DeferredRebuild)); +} + +void Scheduler::scheduleRemove(PathRef File, + UniqueFunction Action) { + auto Resources = Units.removeIfPresent(File); + if (!Resources) { + Action(llvm::make_error( + "trying to remove non-added document", llvm::errc::invalid_argument)); + return; + } + + auto DeferredCancel = Resources->deferCancelRebuild(); + Executor.addToFront( + [](decltype(Action) Action, decltype(DeferredCancel) DeferredCancel) { + DeferredCancel(); + Action(llvm::Error::success()); + }, + std::move(Action), std::move(DeferredCancel)); +} + +void Scheduler::scheduleASTRead( + PathRef File, UniqueFunction)> Action) { + auto Resources = Units.getFile(File); + if (!Resources) { + Action(llvm::make_error( + "trying to get AST for non-added document", + llvm::errc::invalid_argument)); + return; + } + // We currently do all the reads of the AST on a running thread to avoid + // inconsistent states coming from subsequent updates. + Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) { + if (AST) + Action(*AST); + else + Action(llvm::make_error( + "Could not build AST for the latest file update", + llvm::errc::invalid_argument)); + }); +} + +void Scheduler::schedulePreambleRead( + PathRef File, + UniqueFunction)> Action) { + std::shared_ptr Resources = Units.getFile(File); + if (!Resources) { + Action(llvm::make_error( + "trying to get preamble for non-added document", + llvm::errc::invalid_argument)); + return; + } + + std::shared_ptr Preamble = + Resources->getPossiblyStalePreamble(); + Executor.addToFront( + [Resources, Preamble](decltype(Action) Action) mutable { + if (!Preamble) + Preamble = Resources->getPossiblyStalePreamble(); + + Action(Preamble.get()); + }, + std::move(Action)); +} + ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB, DiagnosticsConsumer &DiagConsumer, FileSystemProvider &FSProvider, @@ -139,19 +263,18 @@ llvm::Optional ResourceDir) : CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider), FileIdx(BuildDynamicSymbolIndex ? new FileIndex() : nullptr), - // Pass a callback into `Units` to extract symbols from a newly parsed - // file and rebuild the file index synchronously each time an AST is - // parsed. - // FIXME(ioeric): this can be slow and we may be able to index on less - // critical paths. - Units(FileIdx - ? [this](const Context &Ctx, PathRef Path, - ParsedAST *AST) { FileIdx->update(Ctx, Path, AST); } - : ASTParsedCallback()), ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()), PCHs(std::make_shared()), - StorePreamblesInMemory(StorePreamblesInMemory), - WorkScheduler(AsyncThreadsCount) { + // Pass a callback into `WorkScheduler` to extract symbols from a newly + // parsed file and rebuild the file index synchronously each time an AST + // is parsed. + // FIXME(ioeric): this can be slow and we may be able to index on less + // critical paths. + WorkScheduler( + AsyncThreadsCount, StorePreamblesInMemory, this->PCHs, + FileIdx ? [this](const Context &Ctx, PathRef Path, + ParsedAST *AST) { FileIdx->update(Ctx, Path, AST); } + : ASTParsedCallback()) { if (FileIdx && StaticIdx) { MergedIndex = mergeIndex(FileIdx.get(), StaticIdx); Index = MergedIndex.get(); @@ -173,19 +296,29 @@ std::future ClangdServer::addDocument(Context Ctx, PathRef File, StringRef Contents) { DocVersion Version = DraftMgr.updateDraft(File, Contents); - auto TaggedFS = FSProvider.getTaggedFileSystem(File); - std::shared_ptr Resources = Units.getOrCreateFile( - File, ResourceDir, CDB, StorePreamblesInMemory, PCHs); return scheduleReparseAndDiags(std::move(Ctx), File, VersionedDraft{Version, Contents.str()}, - std::move(Resources), std::move(TaggedFS)); + TaggedFS, /*UpdateCompileCommand=*/false); } std::future ClangdServer::removeDocument(Context Ctx, PathRef File) { DraftMgr.removeDraft(File); - std::shared_ptr Resources = Units.removeIfPresent(File); - return scheduleCancelRebuild(std::move(Ctx), std::move(Resources)); + + std::promise DonePromise; + std::future DoneFuture = DonePromise.get_future(); + + auto Callback = BindWithForward( + [](Context Ctx, std::promise DonePromise, llvm::Error Err) { + // FIXME(ibiryukov): allow to report error to the caller. + // Discard error, if any. + (void)(bool) Err; + DonePromise.set_value(std::move(Ctx)); + }, + std::move(Ctx), std::move(DonePromise)); + + WorkScheduler.scheduleRemove(File, std::move(Callback)); + return DoneFuture; } std::future ClangdServer::forceReparse(Context Ctx, PathRef File) { @@ -194,15 +327,8 @@ "forceReparse() was called for non-added document"); auto TaggedFS = FSProvider.getTaggedFileSystem(File); - auto Recreated = Units.recreateFileIfCompileCommandChanged( - File, ResourceDir, CDB, StorePreamblesInMemory, PCHs); - - // Note that std::future from this cleanup action is ignored. - scheduleCancelRebuild(Ctx.clone(), std::move(Recreated.RemovedFile)); - // Schedule a reparse. return scheduleReparseAndDiags(std::move(Ctx), File, std::move(FileContents), - std::move(Recreated.FileInCollection), - std::move(TaggedFS)); + TaggedFS, /*UpdateCompileCommand=*/true); } std::future>> @@ -234,97 +360,92 @@ IntrusiveRefCntPtr *UsedFS) { using CallbackType = UniqueFunction)>; - std::string Contents; - if (OverridenContents) { - Contents = *OverridenContents; - } else { - auto FileContents = DraftMgr.getDraft(File); - assert(FileContents.Draft && - "codeComplete is called for non-added document"); - - Contents = std::move(*FileContents.Draft); - } - auto TaggedFS = FSProvider.getTaggedFileSystem(File); if (UsedFS) *UsedFS = TaggedFS.Value; - std::shared_ptr Resources = Units.getFile(File); - assert(Resources && "Calling completion on non-added file"); - - // Remember the current Preamble and use it when async task starts executing. - // At the point when async task starts executing, we may have a different - // Preamble in Resources. However, we assume the Preamble that we obtain here - // is reusable in completion more often. - std::shared_ptr Preamble = - Resources->getPossiblyStalePreamble(); // Copy completion options for passing them to async task handler. auto CodeCompleteOpts = Opts; if (!CodeCompleteOpts.Index) // Respect overridden index. CodeCompleteOpts.Index = Index; - // Copy File, as it is a PathRef that will go out of scope before Task is - // executed. - Path FileStr = File; - // Copy PCHs to avoid accessing this->PCHs concurrently - std::shared_ptr PCHs = this->PCHs; - // A task that will be run asynchronously. - auto Task = - // 'mutable' to reassign Preamble variable. - [FileStr, Preamble, Resources, Contents, Pos, CodeCompleteOpts, TaggedFS, - PCHs](Context Ctx, CallbackType Callback) mutable { - if (!Preamble) { - // Maybe we built some preamble before processing this request. - Preamble = Resources->getPossiblyStalePreamble(); - } - // FIXME(ibiryukov): even if Preamble is non-null, we may want to check - // both the old and the new version in case only one of them matches. + auto Command = WorkScheduler.getLatestCommand(File); + assert(Command && "codeComplete called for non-added document"); - CompletionList Result = clangd::codeComplete( - Ctx, FileStr, Resources->getCompileCommand(), - Preamble ? &Preamble->Preamble : nullptr, Contents, Pos, - TaggedFS.Value, PCHs, CodeCompleteOpts); + std::string Contents; + if (OverridenContents) { + Contents = OverridenContents->str(); + } else { + VersionedDraft Latest = DraftMgr.getDraft(File); + assert(Latest.Draft && "codeComplete called for non-added document"); + Contents = *Latest.Draft; + } - Callback(std::move(Ctx), - make_tagged(std::move(Result), std::move(TaggedFS.Tag))); - }; + // Copy PCHs to avoid accessing this->PCHs concurrently + std::shared_ptr PCHs = this->PCHs; + auto Task = [PCHs, Pos, TaggedFS, Command, CodeCompleteOpts]( + Context Ctx, std::string Contents, Path File, + CallbackType Callback, + llvm::Expected PreambleErr) { + assert(PreambleErr && + "error when trying to read preamble for codeComplete"); + auto Preamble = *PreambleErr; + + // FIXME(ibiryukov): even if Preamble is non-null, we may want to check + // both the old and the new version in case only one of them matches. + CompletionList Result = clangd::codeComplete( + Ctx, File, *Command, Preamble ? &Preamble->Preamble : nullptr, Contents, + Pos, TaggedFS.Value, PCHs, CodeCompleteOpts); + + Callback(std::move(Ctx), + make_tagged(std::move(Result), std::move(TaggedFS.Tag))); + }; - WorkScheduler.addToFront(std::move(Task), std::move(Ctx), - std::move(Callback)); + WorkScheduler.schedulePreambleRead( + File, BindWithForward(Task, std::move(Ctx), std::move(Contents), + File.str(), std::move(Callback))); } llvm::Expected> ClangdServer::signatureHelp(const Context &Ctx, PathRef File, Position Pos, llvm::Optional OverridenContents, IntrusiveRefCntPtr *UsedFS) { - std::string DraftStorage; - if (!OverridenContents) { - auto FileContents = DraftMgr.getDraft(File); - if (!FileContents.Draft) + auto TaggedFS = FSProvider.getTaggedFileSystem(File); + if (UsedFS) + *UsedFS = TaggedFS.Value; + + std::string Contents; + if (OverridenContents) { + Contents = OverridenContents->str(); + } else { + VersionedDraft Latest = DraftMgr.getDraft(File); + if (!Latest.Draft) return llvm::make_error( "signatureHelp is called for non-added document", llvm::errc::invalid_argument); - - DraftStorage = std::move(*FileContents.Draft); - OverridenContents = DraftStorage; + Contents = std::move(*Latest.Draft); } - auto TaggedFS = FSProvider.getTaggedFileSystem(File); - if (UsedFS) - *UsedFS = TaggedFS.Value; - - std::shared_ptr Resources = Units.getFile(File); - if (!Resources) + auto Command = WorkScheduler.getLatestCommand(File); + if (!Command) return llvm::make_error( "signatureHelp is called for non-added document", llvm::errc::invalid_argument); - auto Preamble = Resources->getPossiblyStalePreamble(); - auto Result = - clangd::signatureHelp(Ctx, File, Resources->getCompileCommand(), - Preamble ? &Preamble->Preamble : nullptr, - *OverridenContents, Pos, TaggedFS.Value, PCHs); - return make_tagged(std::move(Result), TaggedFS.Tag); + auto Action = [=, &Ctx](llvm::Expected PreambleErr) + -> Expected> { + if (!PreambleErr) + return PreambleErr.takeError(); + auto Preamble = *PreambleErr; + + return make_tagged( + clangd::signatureHelp(Ctx, File, *Command, + Preamble ? &Preamble->Preamble : nullptr, + Contents, Pos, TaggedFS.Value, PCHs), + TaggedFS.Tag); + }; + return waitForPreambleAction>>(WorkScheduler, + File, Action); } llvm::Expected @@ -356,14 +477,18 @@ Expected> ClangdServer::rename(const Context &Ctx, PathRef File, Position Pos, llvm::StringRef NewName) { - std::shared_ptr Resources = Units.getFile(File); - RefactoringResultCollector ResultCollector; - Resources->getAST().get()->runUnderLock([&](ParsedAST *AST) { + using RetType = Expected>; + auto Action = [=](Expected AST) -> RetType { + if (!AST) + return AST.takeError(); + + RefactoringResultCollector ResultCollector; const SourceManager &SourceMgr = AST->getASTContext().getSourceManager(); const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); if (!FE) - return; + return llvm::make_error( + "rename called for non-added document", llvm::errc::invalid_argument); SourceLocation SourceLocationBeg = clangd::getBeginningOfIdentifier(*AST, Pos, FE); tooling::RefactoringRuleContext Context( @@ -371,34 +496,35 @@ Context.setASTContext(AST->getASTContext()); auto Rename = clang::tooling::RenameOccurrences::initiate( Context, SourceRange(SourceLocationBeg), NewName.str()); - if (!Rename) { - ResultCollector.Result = Rename.takeError(); - return; - } + if (!Rename) + return Rename.takeError(); + Rename->invoke(ResultCollector, Context); - }); - assert(ResultCollector.Result.hasValue()); - if (!ResultCollector.Result.getValue()) - return ResultCollector.Result->takeError(); - - std::vector Replacements; - for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) { - tooling::Replacements ChangeReps = Change.getReplacements(); - for (const auto &Rep : ChangeReps) { - // FIXME: Right now we only support renaming the main file, so we drop - // replacements not for the main file. In the future, we might consider to - // support: - // * rename in any included header - // * rename only in the "main" header - // * provide an error if there are symbols we won't rename (e.g. - // std::vector) - // * rename globally in project - // * rename in open files - if (Rep.getFilePath() == File) - Replacements.push_back(Rep); + + assert(ResultCollector.Result.hasValue()); + if (!ResultCollector.Result.getValue()) + return ResultCollector.Result->takeError(); + + std::vector Replacements; + for (const tooling::AtomicChange &Change : ResultCollector.Result->get()) { + tooling::Replacements ChangeReps = Change.getReplacements(); + for (const auto &Rep : ChangeReps) { + // FIXME: Right now we only support renaming the main file, so we + // drop replacements not for the main file. In the future, we might + // consider to support: + // * rename in any included header + // * rename only in the "main" header + // * provide an error if there are symbols we won't rename (e.g. + // std::vector) + // * rename globally in project + // * rename in open files + if (Rep.getFilePath() == File) + Replacements.push_back(Rep); + } } - } - return Replacements; + return Replacements; + }; + return waitForASTAction(WorkScheduler, File, std::move(Action)); } llvm::Optional ClangdServer::getDocument(PathRef File) { @@ -409,40 +535,33 @@ } std::string ClangdServer::dumpAST(PathRef File) { - std::shared_ptr Resources = Units.getFile(File); - if (!Resources) - return ""; + auto Action = [](llvm::Expected AST) -> std::string { + if (!AST) + return ""; + + std::string Result; - std::string Result; - Resources->getAST().get()->runUnderLock([&Result](ParsedAST *AST) { llvm::raw_string_ostream ResultOS(Result); - if (AST) { - clangd::dumpAST(*AST, ResultOS); - } else { - ResultOS << ""; - } + clangd::dumpAST(*AST, ResultOS); ResultOS.flush(); - }); - return Result; + + return Result; + }; + return waitForASTAction(WorkScheduler, File, std::move(Action)); } llvm::Expected>> ClangdServer::findDefinitions(const Context &Ctx, PathRef File, Position Pos) { auto TaggedFS = FSProvider.getTaggedFileSystem(File); - std::shared_ptr Resources = Units.getFile(File); - if (!Resources) - return llvm::make_error( - "findDefinitions called on non-added file", - llvm::errc::invalid_argument); - - std::vector Result; - Resources->getAST().get()->runUnderLock([Pos, &Result, &Ctx](ParsedAST *AST) { + using RetType = llvm::Expected>>; + auto Action = [=, &Ctx](llvm::Expected AST) -> RetType { if (!AST) - return; - Result = clangd::findDefinitions(Ctx, *AST, Pos); - }); - return make_tagged(std::move(Result), TaggedFS.Tag); + return AST.takeError(); + auto Result = clangd::findDefinitions(Ctx, *AST, Pos); + return make_tagged(std::move(Result), TaggedFS.Tag); + }; + return waitForASTAction(WorkScheduler, File, Action); } llvm::Optional ClangdServer::switchSourceHeader(PathRef Path) { @@ -529,102 +648,64 @@ auto TaggedFS = FSProvider.getTaggedFileSystem(File); - std::shared_ptr Resources = Units.getFile(File); - if (!Resources) - return llvm::make_error( - "findDocumentHighlights called on non-added file", - llvm::errc::invalid_argument); - - std::vector Result; - llvm::Optional Err; - Resources->getAST().get()->runUnderLock([Pos, &Ctx, &Err, - &Result](ParsedAST *AST) { - if (!AST) { - Err = llvm::make_error("Invalid AST", - llvm::errc::invalid_argument); - return; - } - Result = clangd::findDocumentHighlights(Ctx, *AST, Pos); - }); - - if (Err) - return std::move(*Err); - return make_tagged(Result, TaggedFS.Tag); + using RetType = llvm::Expected>>; + auto Action = [=, &Ctx](llvm::Expected AST) -> RetType { + if (!AST) + return AST.takeError(); + auto Result = clangd::findDocumentHighlights(Ctx, *AST, Pos); + return make_tagged(std::move(Result), TaggedFS.Tag); + }; + return waitForASTAction(WorkScheduler, File, Action); } std::future ClangdServer::scheduleReparseAndDiags( Context Ctx, PathRef File, VersionedDraft Contents, - std::shared_ptr Resources, - Tagged> TaggedFS) { + Tagged> TaggedFS, + bool UpdateCompileCommand) { + DocVersion Version = Contents.Version; + + auto PrevCommand = + !UpdateCompileCommand ? WorkScheduler.getLatestCommand(File) : llvm::None; + tooling::CompileCommand Command = + UpdateCompileCommand || !PrevCommand + ? getCompileCommand(CDB, File, ResourceDir) + : std::move(*PrevCommand); + + using OptDiags = llvm::Optional>; + Path FileStr = File.str(); + VFSTag Tag = std::move(TaggedFS.Tag); - assert(Contents.Draft && "Draft must have contents"); - UniqueFunction>(const Context &)> - DeferredRebuild = - Resources->deferRebuild(*Contents.Draft, TaggedFS.Value); std::promise DonePromise; std::future DoneFuture = DonePromise.get_future(); - DocVersion Version = Contents.Version; - Path FileStr = File; - VFSTag Tag = TaggedFS.Tag; - auto ReparseAndPublishDiags = - [this, FileStr, Version, - Tag](UniqueFunction>( - const Context &)> - DeferredRebuild, - std::promise DonePromise, Context Ctx) -> void { + auto Callback = [this, Version, FileStr, + Tag](std::promise DonePromise, Context Ctx, + OptDiags Diags) { auto Guard = onScopeExit([&]() { DonePromise.set_value(std::move(Ctx)); }); - - auto CurrentVersion = DraftMgr.getVersion(FileStr); - if (CurrentVersion != Version) - return; // This request is outdated - - auto Diags = DeferredRebuild(Ctx); if (!Diags) - return; // A new reparse was requested before this one completed. + return; - // We need to serialize access to resulting diagnostics to avoid calling - // `onDiagnosticsReady` in the wrong order. + // We need to serialize access to resulting diagnostics to avoid + // calling `onDiagnosticsReady` in the wrong order. std::lock_guard DiagsLock(DiagnosticsMutex); DocVersion &LastReportedDiagsVersion = ReportedDiagnosticVersions[FileStr]; // FIXME(ibiryukov): get rid of '<' comparison here. In the current - // implementation diagnostics will not be reported after version counters' - // overflow. This should not happen in practice, since DocVersion is a - // 64-bit unsigned integer. + // implementation diagnostics will not be reported after version + // counters' overflow. This should not happen in practice, since + // DocVersion is a 64-bit unsigned integer. if (Version < LastReportedDiagsVersion) return; LastReportedDiagsVersion = Version; - DiagConsumer.onDiagnosticsReady(Ctx, FileStr, - make_tagged(std::move(*Diags), Tag)); + DiagConsumer.onDiagnosticsReady( + Ctx, FileStr, make_tagged(std::move(*Diags), std::move(Tag))); }; - WorkScheduler.addToFront(std::move(ReparseAndPublishDiags), - std::move(DeferredRebuild), std::move(DonePromise), - std::move(Ctx)); - return DoneFuture; -} - -std::future -ClangdServer::scheduleCancelRebuild(Context Ctx, - std::shared_ptr Resources) { - std::promise DonePromise; - std::future DoneFuture = DonePromise.get_future(); - if (!Resources) { - // No need to schedule any cleanup. - DonePromise.set_value(std::move(Ctx)); - return DoneFuture; - } - - UniqueFunction DeferredCancel = Resources->deferCancelRebuild(); - auto CancelReparses = [Resources](std::promise DonePromise, - UniqueFunction DeferredCancel, - Context Ctx) { - DeferredCancel(); - DonePromise.set_value(std::move(Ctx)); - }; - WorkScheduler.addToFront(std::move(CancelReparses), std::move(DonePromise), - std::move(DeferredCancel), std::move(Ctx)); + WorkScheduler.scheduleUpdate( + std::move(Ctx), File, + ParseInputs(Command, std::move(TaggedFS.Value), + std::move(*Contents.Draft)), + BindWithForward(Callback, std::move(DonePromise))); return DoneFuture; } Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -58,6 +58,17 @@ std::vector Diags; }; +/// Information required to run clang (e.g., to parse AST or do code +/// completion). +struct ParseInputs { + ParseInputs(tooling::CompileCommand CompileCommand, + IntrusiveRefCntPtr FS, std::string Contents); + + tooling::CompileCommand CompileCommand; + IntrusiveRefCntPtr FS; + std::string Contents; +}; + /// Stores and provides access to parsed AST. class ParsedAST { public: @@ -147,14 +158,12 @@ // We only allow to create CppFile as shared_ptr, because a future returned by // deferRebuild will hold references to it. static std::shared_ptr - Create(PathRef FileName, tooling::CompileCommand Command, - bool StorePreamblesInMemory, + Create(PathRef FileName, bool StorePreamblesInMemory, std::shared_ptr PCHs, ASTParsedCallback ASTCallback); private: - CppFile(PathRef FileName, tooling::CompileCommand Command, - bool StorePreamblesInMemory, + CppFile(PathRef FileName, bool StorePreamblesInMemory, std::shared_ptr PCHs, ASTParsedCallback ASTCallback); @@ -177,9 +186,8 @@ /// Returns a list of diagnostics or a llvm::None, if another rebuild was /// requested in parallel (effectively cancelling this rebuild) before /// diagnostics were produced. - llvm::Optional> - rebuild(const Context &Ctx, StringRef NewContents, - IntrusiveRefCntPtr VFS); + llvm::Optional> rebuild(const Context &Ctx, + ParseInputs Inputs); /// Schedule a rebuild and return a deferred computation that will finish the /// rebuild, that can be called on a different thread. @@ -196,7 +204,7 @@ /// reparse, or None, if another deferRebuild was called before this /// rebuild was finished. UniqueFunction>(const Context &)> - deferRebuild(StringRef NewContents, IntrusiveRefCntPtr VFS); + deferRebuild(ParseInputs Inputs); /// Returns a future to get the most fresh PreambleData for a file. The /// future will wait until the Preamble is rebuilt. @@ -212,8 +220,10 @@ /// always be non-null. std::shared_future> getAST() const; - /// Get CompileCommand used to build this CppFile. - tooling::CompileCommand const &getCompileCommand() const; + /// Get the latest CompileCommand used to build this CppFile. Returns + /// llvm::None before first call to rebuild() or after calls to + /// cancelRebuild(). + llvm::Optional getLatestCommand() const; private: /// A helper guard that manages the state of CppFile during rebuild. @@ -231,7 +241,6 @@ }; Path FileName; - tooling::CompileCommand Command; bool StorePreamblesInMemory; /// Mutex protects all fields, declared below it, FileName and Command are not @@ -243,6 +252,7 @@ bool RebuildInProgress; /// Condition variable to indicate changes to RebuildInProgress. std::condition_variable RebuildCond; + llvm::Optional LatestCommand; /// Promise and future for the latests AST. Fulfilled during rebuild. /// We use std::shared_ptr here because MVSC fails to compile non-copyable Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -220,6 +220,12 @@ } // namespace +ParseInputs::ParseInputs(tooling::CompileCommand CompileCommand, + IntrusiveRefCntPtr FS, + std::string Contents) + : CompileCommand(std::move(CompileCommand)), FS(std::move(FS)), + Contents(std::move(Contents)) {} + void clangd::dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) { AST.getASTContext().getTranslationUnitDecl()->dump(OS, true); } @@ -273,7 +279,6 @@ return Mgr.getMacroArgExpandedLocation(InputLoc); } - } // namespace void ParsedAST::ensurePreambleDeclsDeserialized() { @@ -359,27 +364,22 @@ : AST(std::move(AST)) {} std::shared_ptr -CppFile::Create(PathRef FileName, tooling::CompileCommand Command, - bool StorePreamblesInMemory, +CppFile::Create(PathRef FileName, bool StorePreamblesInMemory, std::shared_ptr PCHs, ASTParsedCallback ASTCallback) { - return std::shared_ptr( - new CppFile(FileName, std::move(Command), StorePreamblesInMemory, - std::move(PCHs), std::move(ASTCallback))); + return std::shared_ptr(new CppFile(FileName, StorePreamblesInMemory, + std::move(PCHs), + std::move(ASTCallback))); } -CppFile::CppFile(PathRef FileName, tooling::CompileCommand Command, - bool StorePreamblesInMemory, +CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory, std::shared_ptr PCHs, ASTParsedCallback ASTCallback) - : FileName(FileName), Command(std::move(Command)), - StorePreamblesInMemory(StorePreamblesInMemory), RebuildCounter(0), - RebuildInProgress(false), PCHs(std::move(PCHs)), + : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory), + RebuildCounter(0), RebuildInProgress(false), PCHs(std::move(PCHs)), ASTCallback(std::move(ASTCallback)) { // FIXME(ibiryukov): we should pass a proper Context here. - log(Context::empty(), "Opened file " + FileName + " with command [" + - this->Command.Directory + "] " + - llvm::join(this->Command.CommandLine, " ")); + log(Context::empty(), "Created CppFile for " + FileName); std::lock_guard Lock(Mutex); LatestAvailablePreamble = nullptr; @@ -431,14 +431,12 @@ } llvm::Optional> -CppFile::rebuild(const Context &Ctx, StringRef NewContents, - IntrusiveRefCntPtr VFS) { - return deferRebuild(NewContents, std::move(VFS))(Ctx); +CppFile::rebuild(const Context &Ctx, ParseInputs Inputs) { + return deferRebuild(std::move(Inputs))(Ctx); } UniqueFunction>(const Context &)> -CppFile::deferRebuild(StringRef NewContents, - IntrusiveRefCntPtr VFS) { +CppFile::deferRebuild(ParseInputs Inputs) { std::shared_ptr OldPreamble; std::shared_ptr PCHs; unsigned RequestRebuildCounter; @@ -463,6 +461,7 @@ this->ASTPromise = std::promise>(); this->ASTFuture = this->ASTPromise.get_future(); } + this->LatestCommand = Inputs.CompileCommand; } // unlock Mutex. // Notify about changes to RebuildCounter. RebuildCond.notify_all(); @@ -473,10 +472,18 @@ // Don't let this CppFile die before rebuild is finished. std::shared_ptr That = shared_from_this(); auto FinishRebuild = - [OldPreamble, VFS, RequestRebuildCounter, PCHs, - That](std::string NewContents, + [OldPreamble, RequestRebuildCounter, PCHs, + That](ParseInputs Inputs, const Context &Ctx) mutable /* to allow changing OldPreamble. */ -> llvm::Optional> { + log(Context::empty(), + "Rebuilding file " + That->FileName + " with command [" + + Inputs.CompileCommand.Directory + "] " + + llvm::join(Inputs.CompileCommand.CommandLine, " ")); + + auto &VFS = Inputs.FS; + auto &NewContents = Inputs.Contents; + // Only one execution of this method is possible at a time. // RebuildGuard will wait for any ongoing rebuilds to finish and will put us // into a state for doing a rebuild. @@ -485,10 +492,10 @@ return llvm::None; std::vector ArgStrs; - for (const auto &S : That->Command.CommandLine) + for (const auto &S : Inputs.CompileCommand.CommandLine) ArgStrs.push_back(S.c_str()); - VFS->setCurrentWorkingDirectory(That->Command.Directory); + VFS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory); std::unique_ptr CI; { @@ -617,7 +624,7 @@ return Diagnostics; }; - return BindWithForward(FinishRebuild, NewContents.str()); + return BindWithForward(FinishRebuild, std::move(Inputs)); } std::shared_future> @@ -636,8 +643,9 @@ return ASTFuture; } -tooling::CompileCommand const &CppFile::getCompileCommand() const { - return Command; +llvm::Optional CppFile::getLatestCommand() const { + std::lock_guard Lock(Mutex); + return LatestCommand; } CppFile::RebuildGuard::RebuildGuard(CppFile &File, Index: clangd/ClangdUnitStore.h =================================================================== --- clangd/ClangdUnitStore.h +++ clangd/ClangdUnitStore.h @@ -27,50 +27,26 @@ public: /// \p ASTCallback is called when a file is parsed synchronously. This should /// not be expensive since it blocks diagnostics. - explicit CppFileCollection(ASTParsedCallback ASTCallback) - : ASTCallback(std::move(ASTCallback)) {} + explicit CppFileCollection(bool StorePreamblesInMemory, + std::shared_ptr PCHs, + ASTParsedCallback ASTCallback) + : ASTCallback(std::move(ASTCallback)), PCHs(std::move(PCHs)), + StorePreamblesInMemory(StorePreamblesInMemory) {} - std::shared_ptr - getOrCreateFile(PathRef File, PathRef ResourceDir, - GlobalCompilationDatabase &CDB, bool StorePreamblesInMemory, - std::shared_ptr PCHs) { + std::shared_ptr getOrCreateFile(PathRef File) { std::lock_guard Lock(Mutex); - auto It = OpenedFiles.find(File); if (It == OpenedFiles.end()) { - auto Command = getCompileCommand(CDB, File, ResourceDir); - It = OpenedFiles - .try_emplace(File, CppFile::Create(File, std::move(Command), - StorePreamblesInMemory, - std::move(PCHs), ASTCallback)) + .try_emplace(File, CppFile::Create(File, StorePreamblesInMemory, + PCHs, ASTCallback)) .first; } return It->second; } - struct RecreateResult { - /// A CppFile, stored in this CppFileCollection for the corresponding - /// filepath after calling recreateFileIfCompileCommandChanged. - std::shared_ptr FileInCollection; - /// If a new CppFile had to be created to account for changed - /// CompileCommand, a previous CppFile instance will be returned in this - /// field. - std::shared_ptr RemovedFile; - }; - - /// Similar to getOrCreateFile, but will replace a current CppFile for \p File - /// with a new one if CompileCommand, provided by \p CDB has changed. - /// If a currently stored CppFile had to be replaced, the previous instance - /// will be returned in RecreateResult.RemovedFile. - RecreateResult recreateFileIfCompileCommandChanged( - PathRef File, PathRef ResourceDir, GlobalCompilationDatabase &CDB, - bool StorePreamblesInMemory, - std::shared_ptr PCHs); - - std::shared_ptr getFile(PathRef File) { + std::shared_ptr getFile(PathRef File) const { std::lock_guard Lock(Mutex); - auto It = OpenedFiles.find(File); if (It == OpenedFiles.end()) return nullptr; @@ -82,15 +58,11 @@ std::shared_ptr removeIfPresent(PathRef File); private: - tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB, - PathRef File, PathRef ResourceDir); - - bool compileCommandsAreEqual(tooling::CompileCommand const &LHS, - tooling::CompileCommand const &RHS); - - std::mutex Mutex; + mutable std::mutex Mutex; llvm::StringMap> OpenedFiles; ASTParsedCallback ASTCallback; + std::shared_ptr PCHs; + bool StorePreamblesInMemory; }; } // namespace clangd } // namespace clang Index: clangd/ClangdUnitStore.cpp =================================================================== --- clangd/ClangdUnitStore.cpp +++ clangd/ClangdUnitStore.cpp @@ -25,53 +25,3 @@ OpenedFiles.erase(It); return Result; } - -CppFileCollection::RecreateResult -CppFileCollection::recreateFileIfCompileCommandChanged( - PathRef File, PathRef ResourceDir, GlobalCompilationDatabase &CDB, - bool StorePreamblesInMemory, std::shared_ptr PCHs) { - auto NewCommand = getCompileCommand(CDB, File, ResourceDir); - - std::lock_guard Lock(Mutex); - - RecreateResult Result; - - auto It = OpenedFiles.find(File); - if (It == OpenedFiles.end()) { - It = OpenedFiles - .try_emplace(File, CppFile::Create(File, std::move(NewCommand), - StorePreamblesInMemory, - std::move(PCHs), ASTCallback)) - .first; - } else if (!compileCommandsAreEqual(It->second->getCompileCommand(), - NewCommand)) { - Result.RemovedFile = std::move(It->second); - It->second = - CppFile::Create(File, std::move(NewCommand), StorePreamblesInMemory, - std::move(PCHs), ASTCallback); - } - Result.FileInCollection = It->second; - return Result; -} - -tooling::CompileCommand -CppFileCollection::getCompileCommand(GlobalCompilationDatabase &CDB, - PathRef File, PathRef ResourceDir) { - llvm::Optional C = CDB.getCompileCommand(File); - if (!C) // FIXME: Suppress diagnostics? Let the user know? - C = CDB.getFallbackCommand(File); - - // Inject the resource dir. - // FIXME: Don't overwrite it if it's already there. - C->CommandLine.push_back("-resource-dir=" + ResourceDir.str()); - return std::move(*C); -} - -bool CppFileCollection::compileCommandsAreEqual( - tooling::CompileCommand const &LHS, tooling::CompileCommand const &RHS) { - // tooling::CompileCommand.Output is ignored, it's not relevant for clangd. - return LHS.Directory == RHS.Directory && - LHS.CommandLine.size() == RHS.CommandLine.size() && - std::equal(LHS.CommandLine.begin(), LHS.CommandLine.end(), - RHS.CommandLine.begin()); -}