Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -108,21 +108,17 @@ ClangdScheduler(bool RunSynchronously); ~ClangdScheduler(); - /// Add \p Request to the start of the queue. \p Request will be run on a - /// separate worker thread. - /// \p Request is scheduled to be executed before all currently added - /// requests. - void addToFront(std::function Request); - /// Add \p Request to the end of the queue. \p Request will be run on a - /// separate worker thread. - /// \p Request is scheduled to be executed after all currently added - /// requests. - void addToEnd(std::function Request); + /// 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. + template void addToFront(Func &&F, Args &&... As); + /// Add a new request to run function \p F with args \p As to the end of the + /// queue. The request will be run on a separate thread. + template void addToEnd(Func &&F, Args &&... As); private: bool RunSynchronously; std::mutex Mutex; - /// We run some tasks on a separate thread(parsing, ClangdUnit cleanup). + /// We run some tasks on a separate threads(parsing, CppFile cleanup). /// This thread looks into RequestQueue to find requests to handle and /// terminates when Done is set to true. std::thread Worker; @@ -131,7 +127,7 @@ /// A queue of requests. /// FIXME(krasimir): code completion should always have priority over parsing /// for diagnostics. - std::deque> RequestQueue; + std::deque> RequestQueue; /// Condition variable to wake up the worker thread. std::condition_variable RequestCV; }; @@ -163,12 +159,16 @@ /// \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. - void addDocument(PathRef File, StringRef Contents); + /// \return A future that will become ready when the rebuild (including + /// diagnostics) is finished. + std::future addDocument(PathRef File, StringRef Contents); /// Remove \p File from list of tracked files, schedule a request to free /// resources associated with it. - void removeDocument(PathRef File); + /// \return A future that will become ready the file is removed and all + /// associated reosources are freed. + std::future removeDocument(PathRef File); /// Force \p File to be reparsed using the latest contents. - void forceReparse(PathRef File); + std::future forceReparse(PathRef File); /// Run code completion for \p File at \p Pos. If \p OverridenContents is not /// None, they will used only for code completion, i.e. no diagnostics update @@ -209,7 +209,7 @@ DiagnosticsConsumer &DiagConsumer; FileSystemProvider &FSProvider; DraftStore DraftMgr; - ClangdUnitStore Units; + CppFileCollection Units; std::string ResourceDir; std::shared_ptr PCHs; // WorkScheduler has to be the last member, because its destructor has to be Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -23,6 +23,16 @@ namespace { +class FulfillPromiseGuard { +public: + FulfillPromiseGuard(std::promise &Promise) : Promise(Promise) {} + + ~FulfillPromiseGuard() { Promise.set_value(); } + +private: + std::promise &Promise; +}; + std::vector formatCode(StringRef Code, StringRef Filename, ArrayRef Ranges) { // Call clang-format. @@ -79,7 +89,7 @@ // using not-yet-initialized members Worker = std::thread([this]() { while (true) { - std::function Request; + std::future Request; // Pick request from the queue { @@ -99,7 +109,7 @@ RequestQueue.pop_front(); } // unlock Mutex - Request(); + Request.get(); } }); } @@ -117,28 +127,34 @@ Worker.join(); } -void ClangdScheduler::addToFront(std::function Request) { +template +void ClangdScheduler::addToFront(Func &&F, Args &&... As) { if (RunSynchronously) { - Request(); + std::forward(F)(std::forward(As)...); return; } { std::lock_guard Lock(Mutex); - RequestQueue.push_front(Request); + RequestQueue.push_front(std::async(std::launch::deferred, + std::forward(F), + std::forward(As)...)); } RequestCV.notify_one(); } -void ClangdScheduler::addToEnd(std::function Request) { +template +void ClangdScheduler::addToEnd(Func &&F, Args &&... As) { if (RunSynchronously) { - Request(); + std::forward(F)(std::forward(As)...); return; } { std::lock_guard Lock(Mutex); - RequestQueue.push_back(Request); + RequestQueue.push_back(std::async(std::launch::deferred, + std::forward(F), + std::forward(As)...)); } RequestCV.notify_one(); } @@ -153,41 +169,68 @@ PCHs(std::make_shared()), WorkScheduler(RunSynchronously) {} -void ClangdServer::addDocument(PathRef File, StringRef Contents) { +std::future ClangdServer::addDocument(PathRef File, StringRef Contents) { DocVersion Version = DraftMgr.updateDraft(File, Contents); - Path FileStr = File; - WorkScheduler.addToFront([this, FileStr, Version]() { - auto FileContents = DraftMgr.getDraft(FileStr); - if (FileContents.Version != Version) - return; // This request is outdated, do nothing - assert(FileContents.Draft && - "No contents inside a file that was scheduled for reparse"); - auto TaggedFS = FSProvider.getTaggedFileSystem(FileStr); - Units.runOnUnit( - FileStr, *FileContents.Draft, ResourceDir, CDB, PCHs, TaggedFS.Value, - [&](ClangdUnit const &Unit) { - DiagConsumer.onDiagnosticsReady( - FileStr, make_tagged(Unit.getLocalDiagnostics(), TaggedFS.Tag)); - }); - }); + auto TaggedFS = FSProvider.getTaggedFileSystem(File); + CppFile &Resources = + Units.getOrCreateFile(File, ResourceDir, CDB, PCHs, TaggedFS.Value); + + std::future>> DeferredRebuild = + Resources.deferRebuild(Contents, TaggedFS.Value); + std::promise DonePromise; + std::future DoneFuture = DonePromise.get_future(); + + Path FileStr = File; + VFSTag Tag = TaggedFS.Tag; + auto ReparseAndPublishDiags = + [this, FileStr, Version, + Tag](std::future>> + DeferredRebuild, + std::promise DonePromise) -> void { + FulfillPromiseGuard Guard(DonePromise); + + auto CurrentVersion = DraftMgr.getVersion(FileStr); + if (CurrentVersion != Version) + return; // This request is outdated + + auto Diags = DeferredRebuild.get(); + if (!Diags) + return; // A new reparse was requested before this one completed. + DiagConsumer.onDiagnosticsReady(FileStr, + make_tagged(std::move(*Diags), Tag)); + }; + + WorkScheduler.addToFront(std::move(ReparseAndPublishDiags), + std::move(DeferredRebuild), std::move(DonePromise)); + return DoneFuture; } -void ClangdServer::removeDocument(PathRef File) { +std::future ClangdServer::removeDocument(PathRef File) { auto Version = DraftMgr.removeDraft(File); Path FileStr = File; - WorkScheduler.addToFront([this, FileStr, Version]() { + + std::promise DonePromise; + std::future DoneFuture = DonePromise.get_future(); + + auto RemoveDocFromCollection = [this, FileStr, + Version](std::promise DonePromise) { + FulfillPromiseGuard Guard(DonePromise); + if (Version != DraftMgr.getVersion(FileStr)) return; // This request is outdated, do nothing - Units.removeUnitIfPresent(FileStr); - }); + Units.removeIfPresent(FileStr); + }; + WorkScheduler.addToFront(std::move(RemoveDocFromCollection), + std::move(DonePromise)); + return DoneFuture; } -void ClangdServer::forceReparse(PathRef File) { +std::future ClangdServer::forceReparse(PathRef File) { // The addDocument schedules the reparse even if the contents of the file // never changed, so we just call it here. - addDocument(File, getDocument(File)); + return addDocument(File, getDocument(File)); } Tagged> @@ -208,12 +251,14 @@ if (UsedFS) *UsedFS = TaggedFS.Value; - std::vector Result; - Units.runOnUnitWithoutReparse(File, *OverridenContents, ResourceDir, CDB, - PCHs, TaggedFS.Value, [&](ClangdUnit &Unit) { - Result = Unit.codeComplete( - *OverridenContents, Pos, TaggedFS.Value); - }); + CppFile *Resources = Units.getFile(File); + assert(Resources && "Calling completion on non-added file"); + + auto Preamble = Resources->getPossiblyStalePreamble(); + std::vector Result = + clangd::codeComplete(File, Resources->getCompileCommand(), + Preamble ? &Preamble->Preamble : nullptr, + *OverridenContents, Pos, TaggedFS.Value, PCHs); return make_tagged(std::move(Result), TaggedFS.Tag); } @@ -253,37 +298,38 @@ } std::string ClangdServer::dumpAST(PathRef File) { - std::promise DumpPromise; - auto DumpFuture = DumpPromise.get_future(); - auto Version = DraftMgr.getVersion(File); - - WorkScheduler.addToEnd([this, &DumpPromise, File, Version]() { - assert(DraftMgr.getVersion(File) == Version && "Version has changed"); - (void)Version; - - Units.runOnExistingUnit(File, [&DumpPromise](ClangdUnit &Unit) { - std::string Result; - - llvm::raw_string_ostream ResultOS(Result); - Unit.dumpAST(ResultOS); - ResultOS.flush(); - - DumpPromise.set_value(std::move(Result)); - }); + CppFile *Resources = Units.getFile(File); + assert(Resources && "dumpAST is called for non-added document"); + + std::string Result; + Resources->getAST().get().runUnderLock([&Result](ParsedAST *AST) { + llvm::raw_string_ostream ResultOS(Result); + if (AST) { + clangd::dumpAST(*AST, ResultOS); + } else { + ResultOS << ""; + } + ResultOS.flush(); }); - return DumpFuture.get(); + return Result; } -Tagged> -ClangdServer::findDefinitions(PathRef File, Position Pos) { +Tagged> ClangdServer::findDefinitions(PathRef File, + Position Pos) { auto FileContents = DraftMgr.getDraft(File); - assert(FileContents.Draft && "findDefinitions is called for non-added document"); + assert(FileContents.Draft && + "findDefinitions is called for non-added document"); - std::vector Result; auto TaggedFS = FSProvider.getTaggedFileSystem(File); - Units.runOnUnit(File, *FileContents.Draft, ResourceDir, CDB, PCHs, - TaggedFS.Value, [&](ClangdUnit &Unit) { - Result = Unit.findDefinitions(Pos); - }); + + CppFile *Resources = Units.getFile(File); + assert(Resources && "Calling findDefinitions on non-added file"); + + std::vector Result; + Resources->getAST().get().runUnderLock([Pos, &Result](ParsedAST *AST) { + if (!AST) + return; + Result = clangd::findDefinitions(*AST, Pos); + }); return make_tagged(std::move(Result), TaggedFS.Tag); } Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -17,7 +17,10 @@ #include "clang/Serialization/ASTBitCodes.h" #include "clang/Tooling/CompilationDatabase.h" #include "clang/Tooling/Core/Replacement.h" +#include +#include #include +#include namespace llvm { class raw_ostream; @@ -43,114 +46,196 @@ llvm::SmallVector FixIts; }; -/// Stores parsed C++ AST and provides implementations of all operations clangd -/// would want to perform on parsed C++ files. -class ClangdUnit { +/// Stores and provides access to parsed AST. +class ParsedAST { public: - ClangdUnit(PathRef FileName, StringRef Contents, StringRef ResourceDir, - std::shared_ptr PCHs, - std::vector Commands, - IntrusiveRefCntPtr VFS); - - /// Reparse with new contents. - void reparse(StringRef Contents, IntrusiveRefCntPtr VFS); - - /// Get code completions at a specified \p Line and \p Column in \p File. - /// - /// This function is thread-safe and returns completion items that own the - /// data they contain. - std::vector - codeComplete(StringRef Contents, Position Pos, - IntrusiveRefCntPtr VFS); - /// Get definition of symbol at a specified \p Line and \p Column in \p File. - std::vector findDefinitions(Position Pos); - /// Returns diagnostics and corresponding FixIts for each diagnostic that are - /// located in the current file. - std::vector getLocalDiagnostics() const; - - /// For testing/debugging purposes. Note that this method deserializes all - /// unserialized Decls, so use with care. - void dumpAST(llvm::raw_ostream &OS) const; + /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null + /// it is reused during parsing. + static llvm::Optional + Build(std::unique_ptr CI, + const PrecompiledPreamble *Preamble, + ArrayRef PreambleDeclIDs, + std::unique_ptr Buffer, + std::shared_ptr PCHs, + IntrusiveRefCntPtr VFS); + + ParsedAST(ParsedAST &&Other); + ParsedAST &operator=(ParsedAST &&Other); + + ~ParsedAST(); + + ASTContext &getASTContext(); + const ASTContext &getASTContext() const; + + Preprocessor &getPreprocessor(); + const Preprocessor &getPreprocessor() const; + + /// This function returns all top-level decls, including those that come + /// from Preamble. Decls, coming from Preamble, have to be deserialized, so + /// this call might be expensive. + ArrayRef getTopLevelDecls(); + + const std::vector &getDiagnostics() const; private: - /// Stores and provides access to parsed AST. - class ParsedAST { - public: - /// Attempts to run Clang and store parsed AST. If \p Preamble is non-null - /// it is reused during parsing. - static llvm::Optional - Build(std::unique_ptr CI, - const PrecompiledPreamble *Preamble, - ArrayRef PreambleDeclIDs, - std::unique_ptr Buffer, - std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS); + ParsedAST(std::unique_ptr Clang, + std::unique_ptr Action, + std::vector TopLevelDecls, + std::vector PendingTopLevelDecls, + std::vector Diags); - ParsedAST(ParsedAST &&Other); - ParsedAST &operator=(ParsedAST &&Other); +private: + void ensurePreambleDeclsDeserialized(); + + // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called + // on it) and CompilerInstance used to run it. That way we don't have to do + // complex memory management of all Clang structures on our own. (They are + // stored in CompilerInstance and cleaned up by + // FrontendAction.EndSourceFile). + std::unique_ptr Clang; + std::unique_ptr Action; + + // Data, stored after parsing. + std::vector Diags; + std::vector TopLevelDecls; + std::vector PendingTopLevelDecls; +}; - ~ParsedAST(); +// Providse thread-safe access to ParsedAST. +class ParsedASTWrapper { +public: + ParsedASTWrapper(ParsedASTWrapper &&Wrapper); + ParsedASTWrapper(llvm::Optional AST); - ASTContext &getASTContext(); - const ASTContext &getASTContext() const; + /// Runs \p F on wrapped ParsedAST under lock. Ensures it is not accessed + /// concurrently. + template void runUnderLock(Func F) const { + std::lock_guard Lock(Mutex); + F(AST ? AST.getPointer() : nullptr); + } - Preprocessor &getPreprocessor(); - const Preprocessor &getPreprocessor() const; +private: + // This wrapper is used as an argument to std::shared_future (and it returns a + // const ref in get()), but we need to have non-const ref in order to + // implement some features. + mutable std::mutex Mutex; + mutable llvm::Optional AST; +}; - /// This function returns all top-level decls, including those that come - /// from Preamble. Decls, coming from Preamble, have to be deserialized, so - /// this call might be expensive. - ArrayRef getTopLevelDecls(); +// Stores Preamble and associated data. +struct PreambleData { + PreambleData(PrecompiledPreamble Preamble, + std::vector TopLevelDeclIDs, + std::vector Diags); - const std::vector &getDiagnostics() const; + PrecompiledPreamble Preamble; + std::vector TopLevelDeclIDs; + std::vector Diags; +}; - private: - ParsedAST(std::unique_ptr Clang, - std::unique_ptr Action, - std::vector TopLevelDecls, - std::vector PendingTopLevelDecls, - std::vector Diags); +/// Manages resources, required by clangd. Allows to rebuild file with new +/// contents, and provides AST and Preamble for it. +class CppFile { +public: + CppFile(PathRef FileName, tooling::CompileCommand Command, + std::shared_ptr PCHs); + CppFile(CppFile &&) = delete; + ~CppFile(); + + /// Rebuild AST and Preamble synchronously on the calling thread. + /// 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(StringRef NewContents, IntrusiveRefCntPtr VFS); + + /// Schedule a rebuild and return a deferred computation that will finish the + /// rebuild, that can be called on a different thread. + /// After calling this method, resources, available via futures returned by + /// getPreamble() and getAST(), will be waiting for rebuild to finish. A + /// future fininshing rebuild, returned by this function, must be + /// computed(i.e. get() should be called on it) in order to make those + /// resources ready. If deferRebuild is called again before the rebuild is + /// finished (either because returned future had not been called or because it + /// had not returned yet), the previous rebuild request is cancelled and the + /// resource futures (returned by getPreamble() or getAST()) that were not + /// ready will be waiting for the last rebuild to finish instead. + /// The future to finish rebuild returns a list of diagnostics built during + /// reparse, or None, if another deferRebuild was called before this + /// rebuild was finished. + std::future>> + deferRebuild(StringRef NewContents, IntrusiveRefCntPtr VFS); + + /// Returns a future to get the most fresh PreambleData for a file. The + /// future will wait until the Preamble is rebuilt. + std::shared_future> getPreamble() const; + /// Return some preamble for a file. It might be stale, but won't wait for + /// rebuild to finish. + std::shared_ptr getPossiblyStalePreamble() const; + + /// Returns a future to get the most fresh AST for a file. Returned AST is + /// wrapped to prevent concurrent accesses. + std::shared_future getAST() const; + + /// Get CompileCommand used to build this CppFile. + tooling::CompileCommand const &getCompileCommand() const; - private: - void ensurePreambleDeclsDeserialized(); - - // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called - // on it) and CompilerInstance used to run it. That way we don't have to do - // complex memory management of all Clang structures on our own. (They are - // stored in CompilerInstance and cleaned up by - // FrontendAction.EndSourceFile). - std::unique_ptr Clang; - std::unique_ptr Action; - - // Data, stored after parsing. - std::vector Diags; - std::vector TopLevelDecls; - std::vector PendingTopLevelDecls; - }; +private: + /// A helper guard that manages the state of CppFile during rebuild. + class RebuildGuard { + public: + RebuildGuard(CppFile &File, unsigned RequestRebuildCounter); + ~RebuildGuard(); - // Store Preamble and all associated data - struct PreambleData { - PreambleData(PrecompiledPreamble Preamble, - std::vector TopLevelDeclIDs, - std::vector Diags); + bool wasCancelledBeforeConstruction() const; - PrecompiledPreamble Preamble; - std::vector TopLevelDeclIDs; - std::vector Diags; + private: + CppFile &File; + unsigned RequestRebuildCounter; + bool WasCancelledBeforeConstruction; }; - SourceLocation getBeginningOfIdentifier(const Position &Pos, - const FileEntry *FE) const; - Path FileName; tooling::CompileCommand Command; - llvm::Optional Preamble; - llvm::Optional Unit; - + /// Mutex protects all fields, declared below it, FileName and Command are not + /// mutated. + mutable std::mutex Mutex; + /// A counter to cancel old rebuilds. + unsigned RebuildCounter; + /// Used to wait when rebuild is finished before starting another one. + bool RebuildInProgress; + /// Condition variable to indicate changes to RebuildInProgress. + std::condition_variable RebuildCond; + + /// Promise and future for the latests AST. Fulfilled during rebuild. + std::promise ASTPromise; + std::shared_future ASTFuture; + + /// Promise and future for the latests Preamble. Fulfilled during rebuild. + std::promise> PreamblePromise; + std::shared_future> PreambleFuture; + /// Latest preamble that was built. May be stale, but always available without + /// waiting for rebuild to finish. + std::shared_ptr LatestAvailablePreamble; + /// Utility class, required by clang. std::shared_ptr PCHs; }; +/// Get code completions at a specified \p Pos in \p FileName. +std::vector +codeComplete(PathRef FileName, tooling::CompileCommand Command, + PrecompiledPreamble const *Preamble, StringRef Contents, + Position Pos, IntrusiveRefCntPtr VFS, + std::shared_ptr PCHs); + +/// Get definition of symbol at a specified \p Pos. +std::vector findDefinitions(ParsedAST &AST, Position Pos); + +/// For testing/debugging purposes. Note that this method deserializes all +/// unserialized Decls, so use with care. +void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS); + } // namespace clangd } // namespace clang #endif Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -28,6 +28,7 @@ #include "llvm/Support/Format.h" #include +#include using namespace clang::clangd; using namespace clang; @@ -70,7 +71,7 @@ std::vector TopLevelDecls; }; -class ClangdUnitPreambleCallbacks : public PreambleCallbacks { +class CppFilePreambleCallbacks : public PreambleCallbacks { public: std::vector takeTopLevelDeclIDs() { return std::move(TopLevelDeclIDs); @@ -220,72 +221,11 @@ return Clang; } -} // namespace - -ClangdUnit::ClangdUnit(PathRef FileName, StringRef Contents, - StringRef ResourceDir, - std::shared_ptr PCHs, - std::vector Commands, - IntrusiveRefCntPtr VFS) - : FileName(FileName), PCHs(PCHs) { - assert(!Commands.empty() && "No compile commands provided"); - - // Inject the resource dir. - // FIXME: Don't overwrite it if it's already there. - Commands.front().CommandLine.push_back("-resource-dir=" + - std::string(ResourceDir)); - - Command = std::move(Commands.front()); - reparse(Contents, VFS); +template bool futureIsReady(std::shared_future const &Future) { + return Future.wait_for(std::chrono::seconds(0)) == std::future_status::ready; } -void ClangdUnit::reparse(StringRef Contents, - IntrusiveRefCntPtr VFS) { - std::vector ArgStrs; - for (const auto &S : Command.CommandLine) - ArgStrs.push_back(S.c_str()); - - VFS->setCurrentWorkingDirectory(Command.Directory); - - std::unique_ptr CI; - { - // FIXME(ibiryukov): store diagnostics from CommandLine when we start - // reporting them. - EmptyDiagsConsumer CommandLineDiagsConsumer; - IntrusiveRefCntPtr CommandLineDiagsEngine = - CompilerInstance::createDiagnostics(new DiagnosticOptions, - &CommandLineDiagsConsumer, false); - CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS); - } - assert(CI && "Couldn't create CompilerInvocation"); - - std::unique_ptr ContentsBuffer = - llvm::MemoryBuffer::getMemBufferCopy(Contents, FileName); - - // Rebuild the preamble if it is missing or can not be reused. - auto Bounds = - ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); - if (!Preamble || !Preamble->Preamble.CanReuse(*CI, ContentsBuffer.get(), - Bounds, VFS.get())) { - std::vector PreambleDiags; - StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags); - IntrusiveRefCntPtr PreambleDiagsEngine = - CompilerInstance::createDiagnostics( - &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false); - ClangdUnitPreambleCallbacks SerializedDeclsCollector; - auto BuiltPreamble = PrecompiledPreamble::Build( - *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs, - SerializedDeclsCollector); - if (BuiltPreamble) - Preamble = PreambleData(std::move(*BuiltPreamble), - SerializedDeclsCollector.takeTopLevelDeclIDs(), - std::move(PreambleDiags)); - } - Unit = ParsedAST::Build( - std::move(CI), Preamble ? &Preamble->Preamble : nullptr, - Preamble ? llvm::makeArrayRef(Preamble->TopLevelDeclIDs) : llvm::None, - std::move(ContentsBuffer), PCHs, VFS); -} +} // namespace namespace { @@ -390,8 +330,10 @@ } // namespace std::vector -ClangdUnit::codeComplete(StringRef Contents, Position Pos, - IntrusiveRefCntPtr VFS) { +clangd::codeComplete(PathRef FileName, tooling::CompileCommand Command, + PrecompiledPreamble const *Preamble, StringRef Contents, + Position Pos, IntrusiveRefCntPtr VFS, + std::shared_ptr PCHs) { std::vector ArgStrs; for (const auto &S : Command.CommandLine) ArgStrs.push_back(S.c_str()); @@ -412,16 +354,14 @@ llvm::MemoryBuffer::getMemBufferCopy(Contents, FileName); // Attempt to reuse the PCH from precompiled preamble, if it was built. - const PrecompiledPreamble *PreambleForCompletion = nullptr; if (Preamble) { auto Bounds = ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); - if (Preamble->Preamble.CanReuse(*CI, ContentsBuffer.get(), Bounds, - VFS.get())) - PreambleForCompletion = &Preamble->Preamble; + if (!Preamble->CanReuse(*CI, ContentsBuffer.get(), Bounds, VFS.get())) + Preamble = nullptr; } - auto Clang = prepareCompilerInstance(std::move(CI), PreambleForCompletion, + auto Clang = prepareCompilerInstance(std::move(CI), Preamble, std::move(ContentsBuffer), PCHs, VFS, DummyDiagsConsumer); auto &DiagOpts = Clang->getDiagnosticOpts(); @@ -457,36 +397,17 @@ return Items; } -std::vector ClangdUnit::getLocalDiagnostics() const { - if (!Unit) - return {}; // Parsing failed. - - std::vector Result; - auto PreambleDiagsSize = Preamble ? Preamble->Diags.size() : 0; - const auto &Diags = Unit->getDiagnostics(); - Result.reserve(PreambleDiagsSize + Diags.size()); - - if (Preamble) - Result.insert(Result.end(), Preamble->Diags.begin(), Preamble->Diags.end()); - Result.insert(Result.end(), Diags.begin(), Diags.end()); - return Result; +void clangd::dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) { + AST.getASTContext().getTranslationUnitDecl()->dump(OS, true); } -void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const { - if (!Unit) { - OS << ""; - return; // Parsing failed. - } - Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true); -} - -llvm::Optional -ClangdUnit::ParsedAST::Build(std::unique_ptr CI, - const PrecompiledPreamble *Preamble, - ArrayRef PreambleDeclIDs, - std::unique_ptr Buffer, - std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS) { +llvm::Optional +ParsedAST::Build(std::unique_ptr CI, + const PrecompiledPreamble *Preamble, + ArrayRef PreambleDeclIDs, + std::unique_ptr Buffer, + std::shared_ptr PCHs, + IntrusiveRefCntPtr VFS) { std::vector ASTDiags; StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags); @@ -563,10 +484,11 @@ return std::move(DeclarationLocations); } - bool handleDeclOccurence(const Decl* D, index::SymbolRoleSet Roles, - ArrayRef Relations, FileID FID, unsigned Offset, - index::IndexDataConsumer::ASTNodeInfo ASTNode) override - { + bool + handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, + ArrayRef Relations, FileID FID, + unsigned Offset, + index::IndexDataConsumer::ASTNodeInfo ASTNode) override { if (isSearchedLocation(FID, Offset)) { addDeclarationLocation(D->getSourceRange()); } @@ -622,48 +544,20 @@ PP.getMacroDefinitionAtLoc(IdentifierInfo, BeforeSearchedLocation); MacroInfo *MacroInf = MacroDef.getMacroInfo(); if (MacroInf) { - addDeclarationLocation( - SourceRange(MacroInf->getDefinitionLoc(), - MacroInf->getDefinitionEndLoc())); + addDeclarationLocation(SourceRange(MacroInf->getDefinitionLoc(), + MacroInf->getDefinitionEndLoc())); } } } } }; -} // namespace - -std::vector ClangdUnit::findDefinitions(Position Pos) { - if (!Unit) - return {}; // Parsing failed. - - const SourceManager &SourceMgr = Unit->getASTContext().getSourceManager(); - const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); - if (!FE) - return {}; - - SourceLocation SourceLocationBeg = getBeginningOfIdentifier(Pos, FE); - - auto DeclLocationsFinder = std::make_shared( - llvm::errs(), SourceLocationBeg, Unit->getASTContext(), - Unit->getPreprocessor()); - index::IndexingOptions IndexOpts; - IndexOpts.SystemSymbolFilter = - index::IndexingOptions::SystemSymbolFilterKind::All; - IndexOpts.IndexFunctionLocals = true; - - indexTopLevelDecls(Unit->getASTContext(), Unit->getTopLevelDecls(), - DeclLocationsFinder, IndexOpts); - - return DeclLocationsFinder->takeLocations(); -} - -SourceLocation ClangdUnit::getBeginningOfIdentifier(const Position &Pos, - const FileEntry *FE) const { +SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos, + const FileEntry *FE) { // The language server protocol uses zero-based line and column numbers. // Clang uses one-based numbers. - const ASTContext &AST = Unit->getASTContext(); + const ASTContext &AST = Unit.getASTContext(); const SourceManager &SourceMgr = AST.getSourceManager(); SourceLocation InputLocation = @@ -691,13 +585,36 @@ if (Result.is(tok::raw_identifier)) { return Lexer::GetBeginningOfToken(PeekBeforeLocation, SourceMgr, - Unit->getASTContext().getLangOpts()); + AST.getLangOpts()); } return InputLocation; } +} // namespace + +std::vector clangd::findDefinitions(ParsedAST &AST, Position Pos) { + const SourceManager &SourceMgr = AST.getASTContext().getSourceManager(); + const FileEntry *FE = SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()); + if (!FE) + return {}; + + SourceLocation SourceLocationBeg = getBeginningOfIdentifier(AST, Pos, FE); + + auto DeclLocationsFinder = std::make_shared( + llvm::errs(), SourceLocationBeg, AST.getASTContext(), + AST.getPreprocessor()); + index::IndexingOptions IndexOpts; + IndexOpts.SystemSymbolFilter = + index::IndexingOptions::SystemSymbolFilterKind::All; + IndexOpts.IndexFunctionLocals = true; -void ClangdUnit::ParsedAST::ensurePreambleDeclsDeserialized() { + indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(), + DeclLocationsFinder, IndexOpts); + + return DeclLocationsFinder->takeLocations(); +} + +void ParsedAST::ensurePreambleDeclsDeserialized() { if (PendingTopLevelDecls.empty()) return; @@ -718,49 +635,42 @@ PendingTopLevelDecls.clear(); } -ClangdUnit::ParsedAST::ParsedAST(ParsedAST &&Other) = default; +ParsedAST::ParsedAST(ParsedAST &&Other) = default; -ClangdUnit::ParsedAST &ClangdUnit::ParsedAST:: -operator=(ParsedAST &&Other) = default; +ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default; -ClangdUnit::ParsedAST::~ParsedAST() { +ParsedAST::~ParsedAST() { if (Action) { Action->EndSourceFile(); } } -ASTContext &ClangdUnit::ParsedAST::getASTContext() { - return Clang->getASTContext(); -} +ASTContext &ParsedAST::getASTContext() { return Clang->getASTContext(); } -const ASTContext &ClangdUnit::ParsedAST::getASTContext() const { +const ASTContext &ParsedAST::getASTContext() const { return Clang->getASTContext(); } -Preprocessor &ClangdUnit::ParsedAST::getPreprocessor() { - return Clang->getPreprocessor(); -} +Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); } -const Preprocessor &ClangdUnit::ParsedAST::getPreprocessor() const { +const Preprocessor &ParsedAST::getPreprocessor() const { return Clang->getPreprocessor(); } -ArrayRef ClangdUnit::ParsedAST::getTopLevelDecls() { +ArrayRef ParsedAST::getTopLevelDecls() { ensurePreambleDeclsDeserialized(); return TopLevelDecls; } -const std::vector & -ClangdUnit::ParsedAST::getDiagnostics() const { +const std::vector &ParsedAST::getDiagnostics() const { return Diags; } -ClangdUnit::ParsedAST::ParsedAST( - std::unique_ptr Clang, - std::unique_ptr Action, - std::vector TopLevelDecls, - std::vector PendingTopLevelDecls, - std::vector Diags) +ParsedAST::ParsedAST(std::unique_ptr Clang, + std::unique_ptr Action, + std::vector TopLevelDecls, + std::vector PendingTopLevelDecls, + std::vector Diags) : Clang(std::move(Clang)), Action(std::move(Action)), Diags(std::move(Diags)), TopLevelDecls(std::move(TopLevelDecls)), PendingTopLevelDecls(std::move(PendingTopLevelDecls)) { @@ -768,9 +678,255 @@ assert(this->Action); } -ClangdUnit::PreambleData::PreambleData( - PrecompiledPreamble Preamble, - std::vector TopLevelDeclIDs, - std::vector Diags) +ParsedASTWrapper::ParsedASTWrapper(ParsedASTWrapper &&Wrapper) + : AST(std::move(Wrapper.AST)) {} + +ParsedASTWrapper::ParsedASTWrapper(llvm::Optional AST) + : AST(std::move(AST)) {} + +PreambleData::PreambleData(PrecompiledPreamble Preamble, + std::vector TopLevelDeclIDs, + std::vector Diags) : Preamble(std::move(Preamble)), TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {} + +CppFile::CppFile(PathRef FileName, tooling::CompileCommand Command, + std::shared_ptr PCHs) + : FileName(FileName), Command(std::move(Command)), RebuildCounter(0), + RebuildInProgress(false), PCHs(std::move(PCHs)) { + + std::lock_guard Lock(Mutex); + LatestAvailablePreamble = nullptr; + PreamblePromise.set_value(nullptr); + PreambleFuture = PreamblePromise.get_future(); + + ASTPromise.set_value(ParsedASTWrapper(llvm::None)); + ASTFuture = ASTPromise.get_future(); +} + +CppFile::~CppFile() { + std::unique_lock Lock(Mutex); + // Increase RebuildCounter to cancel any ongoing rebuilds and wait for the + // processing to finish. + ++RebuildCounter; + RebuildCond.wait(Lock, [this]() { return !RebuildInProgress; }); + + // Make sure anyone waiting for these will get a response. + if (!futureIsReady(PreambleFuture)) + PreamblePromise.set_value(nullptr); + if (!futureIsReady(ASTFuture)) + ASTPromise.set_value(ParsedASTWrapper(llvm::None)); +} + +llvm::Optional> +CppFile::rebuild(StringRef NewContents, + IntrusiveRefCntPtr VFS) { + return deferRebuild(NewContents, std::move(VFS)).get(); +} + +std::future>> +CppFile::deferRebuild(StringRef NewContents, + IntrusiveRefCntPtr VFS) { + std::shared_ptr OldPreamble; + std::shared_ptr PCHs; + unsigned RequestRebuildCounter; + { + std::unique_lock Lock(Mutex); + // Increase RebuildCounter to cancel all ongoing FinishRebuild operations. + // They will try to exit as early as possible and won't call set_value on + // our promises. + RequestRebuildCounter = ++this->RebuildCounter; + PCHs = this->PCHs; + + // Remember the preamble to be used during rebuild. + OldPreamble = this->LatestAvailablePreamble; + // Setup std::promises and std::futures for Preamble and AST. Corresponding + // futures will wait until the rebuild process is finished. + if (futureIsReady(PreambleFuture)) { + this->PreamblePromise = + std::promise>(); + this->PreambleFuture = this->PreamblePromise.get_future(); + } + if (futureIsReady(this->ASTFuture)) { + this->ASTPromise = std::promise(); + this->ASTFuture = this->ASTPromise.get_future(); + } + } // unlock Mutex. + + // A helper to function to finish the rebuild. May be run on a different + // thread. + auto FinishRebuild = [this, OldPreamble, VFS, RequestRebuildCounter, + PCHs](std::string NewContents) + -> llvm::Optional> { + // 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. + RebuildGuard Rebuild(*this, RequestRebuildCounter); + if (Rebuild.wasCancelledBeforeConstruction()) + return llvm::None; + + std::vector ArgStrs; + for (const auto &S : Command.CommandLine) + ArgStrs.push_back(S.c_str()); + + VFS->setCurrentWorkingDirectory(Command.Directory); + + std::unique_ptr CI; + { + // FIXME(ibiryukov): store diagnostics from CommandLine when we start + // reporting them. + EmptyDiagsConsumer CommandLineDiagsConsumer; + IntrusiveRefCntPtr CommandLineDiagsEngine = + CompilerInstance::createDiagnostics(new DiagnosticOptions, + &CommandLineDiagsConsumer, false); + CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS); + } + assert(CI && "Couldn't create CompilerInvocation"); + + std::unique_ptr ContentsBuffer = + llvm::MemoryBuffer::getMemBufferCopy(NewContents, FileName); + + // A helper function to rebuild the preamble or reuse the existing one. Does + // not mutate any fields, only does the actual computation. + auto DoRebuildPreamble = [&]() -> std::shared_ptr { + auto Bounds = + ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); + if (OldPreamble && OldPreamble->Preamble.CanReuse( + *CI, ContentsBuffer.get(), Bounds, VFS.get())) { + return OldPreamble; + } + + std::vector PreambleDiags; + StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags); + IntrusiveRefCntPtr PreambleDiagsEngine = + CompilerInstance::createDiagnostics( + &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false); + CppFilePreambleCallbacks SerializedDeclsCollector; + auto BuiltPreamble = PrecompiledPreamble::Build( + *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs, + SerializedDeclsCollector); + + if (BuiltPreamble) { + return std::make_shared( + std::move(*BuiltPreamble), + SerializedDeclsCollector.takeTopLevelDeclIDs(), + std::move(PreambleDiags)); + } else { + return nullptr; + } + }; + + // Compute updated Preamble. + std::shared_ptr NewPreamble = DoRebuildPreamble(); + // Publish the new Preamble. + { + std::lock_guard Lock(Mutex); + // We always set LatestAvailablePreamble to the new value, hoping that it + // will still be usable in the further requests. + this->LatestAvailablePreamble = NewPreamble; + if (RequestRebuildCounter != this->RebuildCounter) + return llvm::None; // Our rebuild request was cancelled, do nothing. + PreamblePromise.set_value(NewPreamble); + } // unlock Mutex + + // Prepare the Preamble and supplementary data for rebuilding AST. + const PrecompiledPreamble *PreambleForAST = nullptr; + ArrayRef SerializedPreambleDecls = llvm::None; + std::vector Diagnostics; + if (NewPreamble) { + PreambleForAST = &NewPreamble->Preamble; + SerializedPreambleDecls = NewPreamble->TopLevelDeclIDs; + Diagnostics.insert(Diagnostics.begin(), NewPreamble->Diags.begin(), + NewPreamble->Diags.end()); + } + + // Compute updated AST. + llvm::Optional NewAST = + ParsedAST::Build(std::move(CI), PreambleForAST, SerializedPreambleDecls, + std::move(ContentsBuffer), PCHs, VFS); + + if (NewAST) { + Diagnostics.insert(Diagnostics.end(), NewAST->getDiagnostics().begin(), + NewAST->getDiagnostics().end()); + } else { + // Don't report even Preamble diagnostics if we coulnd't build AST. + Diagnostics.clear(); + } + + // Publish the new AST. + { + std::lock_guard Lock(Mutex); + if (RequestRebuildCounter != this->RebuildCounter) + return Diagnostics; // Our rebuild request was cancelled, don't set + // ASTPromise. + ASTPromise.set_value(ParsedASTWrapper(std::move(NewAST))); + } // unlock Mutex + + return Diagnostics; + }; + + return std::async(std::launch::deferred, FinishRebuild, NewContents.str()); +} + +std::shared_future> +CppFile::getPreamble() const { + std::lock_guard Lock(Mutex); + return PreambleFuture; +} + +std::shared_ptr CppFile::getPossiblyStalePreamble() const { + std::lock_guard Lock(Mutex); + return LatestAvailablePreamble; +} + +std::shared_future CppFile::getAST() const { + std::lock_guard Lock(Mutex); + return ASTFuture; +} + +tooling::CompileCommand const &CppFile::getCompileCommand() const { + return Command; +} + +CppFile::RebuildGuard::RebuildGuard(CppFile &File, + unsigned RequestRebuildCounter) + : File(File), RequestRebuildCounter(RequestRebuildCounter) { + std::unique_lock Lock(File.Mutex); + WasCancelledBeforeConstruction = File.RebuildCounter != RequestRebuildCounter; + if (WasCancelledBeforeConstruction) + return; + + File.RebuildCond.wait(Lock, [&File]() { return !File.RebuildInProgress; }); + + WasCancelledBeforeConstruction = File.RebuildCounter != RequestRebuildCounter; + if (WasCancelledBeforeConstruction) + return; + + File.RebuildInProgress = true; +} + +bool CppFile::RebuildGuard::wasCancelledBeforeConstruction() const { + return WasCancelledBeforeConstruction; +} + +CppFile::RebuildGuard::~RebuildGuard() { + if (WasCancelledBeforeConstruction) + return; + + std::unique_lock Lock(File.Mutex); + assert(File.RebuildInProgress); + File.RebuildInProgress = false; + + if (File.RebuildCounter == RequestRebuildCounter) { + // Our rebuild request was successful. + assert(futureIsReady(File.ASTFuture)); + assert(futureIsReady(File.PreambleFuture)); + } else { + // Our rebuild request was cancelled, because further reparse was requested. + assert(!futureIsReady(File.ASTFuture)); + assert(!futureIsReady(File.PreambleFuture)); + } + + Lock.unlock(); + File.RebuildCond.notify_all(); +} Index: clangd/ClangdUnitStore.h =================================================================== --- clangd/ClangdUnitStore.h +++ clangd/ClangdUnitStore.h @@ -1,4 +1,4 @@ -//===--- ClangdUnitStore.h - A ClangdUnits container -------------*-C++-*-===// +//===--- ClangdUnitStore.h - A container of CppFiles -------------*-C++-*-===// // // The LLVM Compiler Infrastructure // @@ -20,87 +20,43 @@ namespace clang { namespace clangd { -/// Thread-safe collection of ASTs built for specific files. Provides -/// synchronized access to ASTs. -class ClangdUnitStore { +/// Thread-safe mapping from FileNames to CppFile. +class CppFileCollection { public: - /// Run specified \p Action on the ClangdUnit for \p File. - /// If the file is not present in ClangdUnitStore, a new ClangdUnit will be - /// created from the \p FileContents. If the file is already present in the - /// store, ClangdUnit::reparse will be called with the new contents before - /// running \p Action. - template - void runOnUnit(PathRef File, StringRef FileContents, StringRef ResourceDir, - GlobalCompilationDatabase &CDB, - std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS, Func Action) { - runOnUnitImpl(File, FileContents, ResourceDir, CDB, PCHs, - /*ReparseBeforeAction=*/true, VFS, - std::forward(Action)); - } - - /// Run specified \p Action on the ClangdUnit for \p File. - /// If the file is not present in ClangdUnitStore, a new ClangdUnit will be - /// created from the \p FileContents. If the file is already present in the - /// store, the \p Action will be run directly on it. - template - void runOnUnitWithoutReparse(PathRef File, StringRef FileContents, - StringRef ResourceDir, - GlobalCompilationDatabase &CDB, - std::shared_ptr PCHs, - IntrusiveRefCntPtr VFS, - Func Action) { - runOnUnitImpl(File, FileContents, ResourceDir, CDB, PCHs, - /*ReparseBeforeAction=*/false, VFS, - std::forward(Action)); - } - - /// Run the specified \p Action on the ClangdUnit for \p File. - /// Unit for \p File should exist in the store. - template void runOnExistingUnit(PathRef File, Func Action) { + CppFile &getOrCreateFile(PathRef File, PathRef ResourceDir, + GlobalCompilationDatabase &CDB, + std::shared_ptr PCHs, + IntrusiveRefCntPtr VFS) { std::lock_guard Lock(Mutex); auto It = OpenedFiles.find(File); - assert(It != OpenedFiles.end() && "File is not in OpenedFiles"); + if (It == OpenedFiles.end()) { + auto Command = getCompileCommand(CDB, File, ResourceDir); - Action(It->second); + It = OpenedFiles + .try_emplace(File, File, std::move(Command), std::move(PCHs)) + .first; + } + return It->second; } - /// Remove ClangdUnit for \p File, if any - void removeUnitIfPresent(PathRef File); - -private: - /// Run specified \p Action on the ClangdUnit for \p File. - template - void runOnUnitImpl(PathRef File, StringRef FileContents, - StringRef ResourceDir, GlobalCompilationDatabase &CDB, - std::shared_ptr PCHs, - bool ReparseBeforeAction, - IntrusiveRefCntPtr VFS, Func Action) { + CppFile *getFile(PathRef File) { std::lock_guard Lock(Mutex); auto It = OpenedFiles.find(File); - if (It == OpenedFiles.end()) { - auto Commands = getCompileCommands(CDB, File); - assert(!Commands.empty() && - "getCompileCommands should add default command"); - - It = OpenedFiles - .insert(std::make_pair(File, ClangdUnit(File, FileContents, - ResourceDir, PCHs, - Commands, VFS))) - .first; - } else if (ReparseBeforeAction) { - It->second.reparse(FileContents, VFS); - } - return Action(It->second); + if (It == OpenedFiles.end()) + return nullptr; + return &It->second; } - std::vector - getCompileCommands(GlobalCompilationDatabase &CDB, PathRef File); + void removeIfPresent(PathRef File); + +private: + tooling::CompileCommand getCompileCommand(GlobalCompilationDatabase &CDB, + PathRef File, PathRef ResourceDir); std::mutex Mutex; - llvm::StringMap OpenedFiles; + llvm::StringMap OpenedFiles; }; } // namespace clangd } // namespace clang Index: clangd/ClangdUnitStore.cpp =================================================================== --- clangd/ClangdUnitStore.cpp +++ clangd/ClangdUnitStore.cpp @@ -13,7 +13,7 @@ using namespace clang::clangd; using namespace clang; -void ClangdUnitStore::removeUnitIfPresent(PathRef File) { +void CppFileCollection::removeIfPresent(PathRef File) { std::lock_guard Lock(Mutex); auto It = OpenedFiles.find(File); @@ -22,12 +22,17 @@ OpenedFiles.erase(It); } -std::vector -ClangdUnitStore::getCompileCommands(GlobalCompilationDatabase &CDB, - PathRef File) { +tooling::CompileCommand +CppFileCollection::getCompileCommand(GlobalCompilationDatabase &CDB, PathRef File, + PathRef ResourceDir) { std::vector Commands = CDB.getCompileCommands(File); if (Commands.empty()) // Add a fake command line if we know nothing. Commands.push_back(getDefaultCompileCommand(File)); - return Commands; + + // Inject the resource dir. + // FIXME: Don't overwrite it if it's already there. + Commands.front().CommandLine.push_back("-resource-dir=" + + std::string(ResourceDir)); + return std::move(Commands.front()); } Index: clangd/GlobalCompilationDatabase.h =================================================================== --- clangd/GlobalCompilationDatabase.h +++ clangd/GlobalCompilationDatabase.h @@ -28,7 +28,7 @@ /// Returns a default compile command to use for \p File. tooling::CompileCommand getDefaultCompileCommand(PathRef File); -/// Provides compilation arguments used for building ClangdUnit. +/// Provides compilation arguments used for parsing C and C++ files. class GlobalCompilationDatabase { public: virtual ~GlobalCompilationDatabase() = default; Index: unittests/clangd/ClangdTests.cpp =================================================================== --- unittests/clangd/ClangdTests.cpp +++ unittests/clangd/ClangdTests.cpp @@ -17,6 +17,7 @@ #include "llvm/Support/Regex.h" #include "gtest/gtest.h" #include +#include #include #include #include @@ -130,10 +131,16 @@ namespace clangd { 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); + class ErrorCheckingDiagConsumer : public DiagnosticsConsumer { public: - void onDiagnosticsReady(PathRef File, - Tagged> Diagnostics) override { + void + onDiagnosticsReady(PathRef File, + Tagged> Diagnostics) override { bool HadError = false; for (const auto &DiagAndFixIts : Diagnostics.Value) { // FIXME: severities returned by clangd should have a descriptive @@ -152,9 +159,7 @@ return HadErrorInLastDiags; } - VFSTag lastVFSTag() { - return LastVFSTag; - } + VFSTag lastVFSTag() { return LastVFSTag; } private: std::mutex Mutex; @@ -276,9 +281,15 @@ auto SourceFilename = getVirtualTestFilePath(SourceFileRelPath); FS.ExpectedFile = SourceFilename; - Server.addDocument(SourceFilename, SourceContents); + + // Have to sync reparses because RunSynchronously is false. + auto AddDocFuture = 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_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags()); return Result; } @@ -338,16 +349,25 @@ FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; - Server.addDocument(FooCpp, SourceContents); + // To sync reparses before checking for errors. + std::future ParseFuture; + + ParseFuture = Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); + ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), + std::future_status::ready); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); - Server.addDocument(FooCpp, ""); + ParseFuture = Server.addDocument(FooCpp, ""); auto DumpParseEmpty = dumpASTWithoutMemoryLocs(Server, FooCpp); + ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), + std::future_status::ready); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); - Server.addDocument(FooCpp, SourceContents); + ParseFuture = Server.addDocument(FooCpp, SourceContents); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); + ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), + std::future_status::ready); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); EXPECT_EQ(DumpParse1, DumpParse2); @@ -374,18 +394,27 @@ FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; - Server.addDocument(FooCpp, SourceContents); + // To sync reparses before checking for errors. + std::future ParseFuture; + + ParseFuture = Server.addDocument(FooCpp, SourceContents); auto DumpParse1 = dumpASTWithoutMemoryLocs(Server, FooCpp); + ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), + std::future_status::ready); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); FS.Files[FooH] = ""; - Server.forceReparse(FooCpp); + ParseFuture = Server.forceReparse(FooCpp); auto DumpParseDifferent = dumpASTWithoutMemoryLocs(Server, FooCpp); + ASSERT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), + std::future_status::ready); EXPECT_TRUE(DiagConsumer.hadErrorInLastDiags()); FS.Files[FooH] = "int a;"; - Server.forceReparse(FooCpp); + ParseFuture = Server.forceReparse(FooCpp); auto DumpParse2 = dumpASTWithoutMemoryLocs(Server, FooCpp); + EXPECT_EQ(ParseFuture.wait_for(DefaultFutureTimeout), + std::future_status::ready); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); EXPECT_EQ(DumpParse1, DumpParse2); @@ -404,10 +433,12 @@ FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; + // No need to sync reparses, because RunSynchronously is set + // to true. FS.Tag = "123"; Server.addDocument(FooCpp, SourceContents); - EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); EXPECT_EQ(Server.codeComplete(FooCpp, Position{0, 0}).Tag, FS.Tag); + EXPECT_EQ(DiagConsumer.lastVFSTag(), FS.Tag); FS.Tag = "321"; Server.addDocument(FooCpp, SourceContents); @@ -455,6 +486,8 @@ )cpp"; FS.Files[FooCpp] = SourceContents; + // No need to sync reparses, because RunSynchronously is set + // to true. Server.addDocument(FooCpp, SourceContents); EXPECT_FALSE(DiagConsumer.hadErrorInLastDiags()); @@ -505,6 +538,8 @@ FS.Files[FooCpp] = SourceContents; FS.ExpectedFile = FooCpp; + // No need to sync reparses here as there are no asserts on diagnostics (or + // other async operations). Server.addDocument(FooCpp, SourceContents); {