Index: clangd/ClangdServer.h =================================================================== --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -331,7 +331,8 @@ std::future scheduleReparseAndDiags(Context Ctx, PathRef File, VersionedDraft Contents, std::shared_ptr Resources, - Tagged> TaggedFS); + Tagged> TaggedFS, + bool AllowCachedCompileFlags); std::future scheduleCancelRebuild(Context Ctx, std::shared_ptr Resources); Index: clangd/ClangdServer.cpp =================================================================== --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -32,6 +32,18 @@ namespace { +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); @@ -175,11 +187,12 @@ DocVersion Version = DraftMgr.updateDraft(File, Contents); auto TaggedFS = FSProvider.getTaggedFileSystem(File); - std::shared_ptr Resources = Units.getOrCreateFile( - File, ResourceDir, CDB, StorePreamblesInMemory, PCHs); + std::shared_ptr Resources = + Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs); return scheduleReparseAndDiags(std::move(Ctx), File, VersionedDraft{Version, Contents.str()}, - std::move(Resources), std::move(TaggedFS)); + std::move(Resources), std::move(TaggedFS), + /*AllowCachedCompileFlags=*/true); } std::future ClangdServer::removeDocument(Context Ctx, PathRef File) { @@ -194,15 +207,11 @@ "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)); + std::shared_ptr Resources = + Units.getOrCreateFile(File, ResourceDir, StorePreamblesInMemory, PCHs); + return scheduleReparseAndDiags(std::move(Ctx), File, FileContents, + std::move(Resources), std::move(TaggedFS), + /*AllowCachedCompileFlags=*/false); } std::future>> @@ -268,20 +277,24 @@ Path FileStr = File; // Copy PCHs to avoid accessing this->PCHs concurrently std::shared_ptr PCHs = this->PCHs; + + assert(Resources->getLastCommand() && + "CppFile is in inconsistent state, missing CompileCommand"); + tooling::CompileCommand CompileCommand = *Resources->getLastCommand(); + // 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 { + PCHs, CompileCommand](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. - CompletionList Result = clangd::codeComplete( - Ctx, FileStr, Resources->getCompileCommand(), + Ctx, FileStr, CompileCommand, Preamble ? &Preamble->Preamble : nullptr, Contents, Pos, TaggedFS.Value, PCHs, CodeCompleteOpts); @@ -319,9 +332,12 @@ "signatureHelp is called for non-added document", llvm::errc::invalid_argument); + assert(Resources->getLastCommand() && + "CppFile is in inconsistent state, missing CompileCommand"); + auto Preamble = Resources->getPossiblyStalePreamble(); auto Result = - clangd::signatureHelp(Ctx, File, Resources->getCompileCommand(), + clangd::signatureHelp(Ctx, File, *Resources->getLastCommand(), Preamble ? &Preamble->Preamble : nullptr, *OverridenContents, Pos, TaggedFS.Value, PCHs); return make_tagged(std::move(Result), TaggedFS.Tag); @@ -555,12 +571,20 @@ std::future ClangdServer::scheduleReparseAndDiags( Context Ctx, PathRef File, VersionedDraft Contents, std::shared_ptr Resources, - Tagged> TaggedFS) { + Tagged> TaggedFS, + bool AllowCachedCompileFlags) { + llvm::Optional ReusedCommand; + if (AllowCachedCompileFlags) + ReusedCommand = Resources->getLastCommand(); + tooling::CompileCommand Command = + ReusedCommand ? std::move(*ReusedCommand) + : getCompileCommand(CDB, File, ResourceDir); + ParseInputs Inputs = {std::move(Command), std::move(TaggedFS.Value), + *std::move(Contents.Draft)}; assert(Contents.Draft && "Draft must have contents"); UniqueFunction>(const Context &)> - DeferredRebuild = - Resources->deferRebuild(*Contents.Draft, TaggedFS.Value); + DeferredRebuild = Resources->deferRebuild(std::move(Inputs)); std::promise DonePromise; std::future DoneFuture = DonePromise.get_future(); Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -58,6 +58,13 @@ std::vector Diags; }; +/// Information required to run clang, e.g. to parse AST or do code completion. +struct ParseInputs { + tooling::CompileCommand CompileCommand; + IntrusiveRefCntPtr FS; + std::string Contents; +}; + /// Stores and provides access to parsed AST. class ParsedAST { public: @@ -147,14 +154,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 +182,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 +200,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 +216,13 @@ /// 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(). + // In practice we always call rebuild() when adding a CppFile to the + // CppFileCollection, and only `cancelRebuild()` after removing it. This means + // files in the CppFileCollection always have a compile command available. + llvm::Optional getLastCommand() const; private: /// A helper guard that manages the state of CppFile during rebuild. @@ -231,7 +240,6 @@ }; Path FileName; - tooling::CompileCommand Command; bool StorePreamblesInMemory; /// Mutex protects all fields, declared below it, FileName and Command are not @@ -243,6 +251,7 @@ bool RebuildInProgress; /// Condition variable to indicate changes to RebuildInProgress. std::condition_variable RebuildCond; + llvm::Optional LastCommand; /// 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 @@ -273,7 +273,6 @@ return Mgr.getMacroArgExpandedLocation(InputLoc); } - } // namespace void ParsedAST::ensurePreambleDeclsDeserialized() { @@ -359,27 +358,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 +425,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 +455,7 @@ this->ASTPromise = std::promise>(); this->ASTFuture = this->ASTPromise.get_future(); } + this->LastCommand = Inputs.CompileCommand; } // unlock Mutex. // Notify about changes to RebuildCounter. RebuildCond.notify_all(); @@ -473,10 +466,15 @@ // 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, " ")); + // 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 +483,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); + Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory); std::unique_ptr CI; { @@ -498,15 +496,15 @@ IntrusiveRefCntPtr CommandLineDiagsEngine = CompilerInstance::createDiagnostics(new DiagnosticOptions, &IgnoreDiagnostics, false); - CI = - createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine, VFS); + CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine, + Inputs.FS); // createInvocationFromCommandLine sets DisableFree. CI->getFrontendOpts().DisableFree = false; } assert(CI && "Couldn't create CompilerInvocation"); std::unique_ptr ContentsBuffer = - llvm::MemoryBuffer::getMemBufferCopy(NewContents, That->FileName); + llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, That->FileName); // A helper function to rebuild the preamble or reuse the existing one. Does // not mutate any fields of CppFile, only does the actual computation. @@ -515,8 +513,9 @@ [&]() mutable -> std::shared_ptr { auto Bounds = ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0); - if (OldPreamble && OldPreamble->Preamble.CanReuse( - *CI, ContentsBuffer.get(), Bounds, VFS.get())) { + if (OldPreamble && + OldPreamble->Preamble.CanReuse(*CI, ContentsBuffer.get(), Bounds, + Inputs.FS.get())) { log(Ctx, "Reusing preamble for file " + Twine(That->FileName)); return OldPreamble; } @@ -541,7 +540,8 @@ CppFilePreambleCallbacks SerializedDeclsCollector; auto BuiltPreamble = PrecompiledPreamble::Build( - *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, VFS, PCHs, + *CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine, Inputs.FS, + PCHs, /*StoreInMemory=*/That->StorePreamblesInMemory, SerializedDeclsCollector); @@ -590,7 +590,7 @@ trace::Span Tracer(Ctx, "Build"); SPAN_ATTACH(Tracer, "File", That->FileName); NewAST = ParsedAST::Build(Ctx, std::move(CI), std::move(NewPreamble), - std::move(ContentsBuffer), PCHs, VFS); + std::move(ContentsBuffer), PCHs, Inputs.FS); } if (NewAST) { @@ -617,7 +617,7 @@ return Diagnostics; }; - return BindWithForward(FinishRebuild, NewContents.str()); + return BindWithForward(FinishRebuild, std::move(Inputs)); } std::shared_future> @@ -636,8 +636,9 @@ return ASTFuture; } -tooling::CompileCommand const &CppFile::getCompileCommand() const { - return Command; +llvm::Optional CppFile::getLastCommand() const { + std::lock_guard Lock(Mutex); + return LastCommand; } CppFile::RebuildGuard::RebuildGuard(CppFile &File, Index: clangd/ClangdUnitStore.h =================================================================== --- clangd/ClangdUnitStore.h +++ clangd/ClangdUnitStore.h @@ -32,42 +32,19 @@ std::shared_ptr getOrCreateFile(PathRef File, PathRef ResourceDir, - GlobalCompilationDatabase &CDB, bool StorePreamblesInMemory, + bool StorePreamblesInMemory, std::shared_ptr PCHs) { 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, + .try_emplace(File, CppFile::Create(File, StorePreamblesInMemory, std::move(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::lock_guard Lock(Mutex); @@ -82,12 +59,6 @@ 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; llvm::StringMap> OpenedFiles; ASTParsedCallback ASTCallback; 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()); -}