kuganv updated this revision to Diff 522523. kuganv added a comment. Fixed missing merge that caused build error.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148088/new/ https://reviews.llvm.org/D148088 Files: clang-tools-extra/clangd/ClangdServer.cpp clang-tools-extra/clangd/ClangdServer.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/Preamble.h clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd/TUScheduler.h clang-tools-extra/clangd/tool/Check.cpp clang-tools-extra/clangd/unittests/FileIndexTests.cpp clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp clang-tools-extra/clangd/unittests/TestWorkspace.cpp clang/include/clang/Frontend/CompilerInstance.h
Index: clang/include/clang/Frontend/CompilerInstance.h =================================================================== --- clang/include/clang/Frontend/CompilerInstance.h +++ clang/include/clang/Frontend/CompilerInstance.h @@ -12,6 +12,7 @@ #include "clang/AST/ASTConsumer.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/SourceManager.h" +#include "clang/Basic/TargetInfo.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Frontend/Utils.h" @@ -233,6 +234,8 @@ return *Invocation; } + std::shared_ptr<CompilerInvocation> getInvocationPtr() { return Invocation; } + /// setInvocation - Replace the current invocation. void setInvocation(std::shared_ptr<CompilerInvocation> Value); @@ -338,6 +341,11 @@ return *Diagnostics; } + IntrusiveRefCntPtr<DiagnosticsEngine> getDiagnosticsPtr() const { + assert(Diagnostics && "Compiler instance has no diagnostics!"); + return Diagnostics; + } + /// setDiagnostics - Replace the current diagnostics engine. void setDiagnostics(DiagnosticsEngine *Value); @@ -373,6 +381,11 @@ return *Target; } + IntrusiveRefCntPtr<TargetInfo> getTargetPtr() const { + assert(Target && "Compiler instance has no target!"); + return Target; + } + /// Replace the current Target. void setTarget(TargetInfo *Value); @@ -406,6 +419,11 @@ return *FileMgr; } + IntrusiveRefCntPtr<FileManager> getFileManagerPtr() const { + assert(FileMgr && "Compiler instance has no file manager!"); + return FileMgr; + } + void resetAndLeakFileManager() { llvm::BuryPointer(FileMgr.get()); FileMgr.resetWithoutRelease(); @@ -426,6 +444,11 @@ return *SourceMgr; } + IntrusiveRefCntPtr<SourceManager> getSourceManagerPtr() const { + assert(SourceMgr && "Compiler instance has no source manager!"); + return SourceMgr; + } + void resetAndLeakSourceManager() { llvm::BuryPointer(SourceMgr.get()); SourceMgr.resetWithoutRelease(); @@ -466,6 +489,11 @@ return *Context; } + IntrusiveRefCntPtr<ASTContext> getASTContextPtr() const { + assert(Context && "Compiler instance has no AST context!"); + return Context; + } + void resetAndLeakASTContext() { llvm::BuryPointer(Context.get()); Context.resetWithoutRelease(); Index: clang-tools-extra/clangd/unittests/TestWorkspace.cpp =================================================================== --- clang-tools-extra/clangd/unittests/TestWorkspace.cpp +++ clang-tools-extra/clangd/unittests/TestWorkspace.cpp @@ -21,10 +21,12 @@ continue; TU.Code = Input.second.Code; TU.Filename = Input.first().str(); - TU.preamble([&](ASTContext &Ctx, Preprocessor &PP, - const CanonicalIncludes &CanonIncludes) { + TU.preamble([&](CapturedASTCtx ASTCtx, + const std::shared_ptr<CanonicalIncludes> CanonIncludes) { + auto &Ctx = ASTCtx.getASTContext(); + auto &PP = ASTCtx.getPreprocessor(); Index->updatePreamble(testPath(Input.first()), "null", Ctx, PP, - CanonIncludes); + *CanonIncludes); }); ParsedAST MainAST = TU.build(); Index->updateMain(testPath(Input.first()), MainAST); Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp +++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp @@ -1129,9 +1129,8 @@ public: BlockPreambleThread(llvm::StringRef BlockVersion, Notification &N) : BlockVersion(BlockVersion), N(N) {} - void onPreambleAST(PathRef Path, llvm::StringRef Version, - const CompilerInvocation &, ASTContext &Ctx, - Preprocessor &, const CanonicalIncludes &) override { + void onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx, + const std::shared_ptr<CanonicalIncludes>) override { if (Version == BlockVersion) N.wait(); } @@ -1208,9 +1207,8 @@ BlockPreambleThread(Notification &UnblockPreamble, DiagsCB CB) : UnblockPreamble(UnblockPreamble), CB(std::move(CB)) {} - void onPreambleAST(PathRef Path, llvm::StringRef Version, - const CompilerInvocation &, ASTContext &Ctx, - Preprocessor &, const CanonicalIncludes &) override { + void onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx, + const std::shared_ptr<CanonicalIncludes>) override { if (BuildBefore) ASSERT_TRUE(UnblockPreamble.wait(timeoutSeconds(5))) << "Expected notification"; @@ -1562,9 +1560,8 @@ std::vector<std::string> &Filenames; CaptureBuiltFilenames(std::vector<std::string> &Filenames) : Filenames(Filenames) {} - void onPreambleAST(PathRef Path, llvm::StringRef Version, - const CompilerInvocation &CI, ASTContext &Ctx, - Preprocessor &PP, const CanonicalIncludes &) override { + void onPreambleAST(PathRef Path, llvm::StringRef Version, CapturedASTCtx, + const std::shared_ptr<CanonicalIncludes>) override { // Deliberately no synchronization. // The PreambleThrottler should serialize these calls, if not then tsan // will find a bug here. Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/FileIndexTests.cpp +++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp @@ -307,13 +307,15 @@ bool IndexUpdated = false; buildPreamble(FooCpp, *CI, PI, /*StoreInMemory=*/true, - [&](ASTContext &Ctx, Preprocessor &PP, - const CanonicalIncludes &CanonIncludes) { + [&](CapturedASTCtx ASTCtx, + const std::shared_ptr<CanonicalIncludes> CanonIncludes) { + auto &Ctx = ASTCtx.getASTContext(); + auto &PP = ASTCtx.getPreprocessor(); EXPECT_FALSE(IndexUpdated) << "Expected only a single index update"; IndexUpdated = true; Index.updatePreamble(FooCpp, /*Version=*/"null", Ctx, PP, - CanonIncludes); + *CanonIncludes); }); ASSERT_TRUE(IndexUpdated); Index: clang-tools-extra/clangd/tool/Check.cpp =================================================================== --- clang-tools-extra/clangd/tool/Check.cpp +++ clang-tools-extra/clangd/tool/Check.cpp @@ -206,15 +206,16 @@ // Build preamble and AST, and index them. bool buildAST() { log("Building preamble..."); - Preamble = buildPreamble(File, *Invocation, Inputs, /*StoreInMemory=*/true, - [&](ASTContext &Ctx, Preprocessor &PP, - const CanonicalIncludes &Includes) { - if (!Opts.BuildDynamicSymbolIndex) - return; - log("Indexing headers..."); - Index.updatePreamble(File, /*Version=*/"null", - Ctx, PP, Includes); - }); + Preamble = buildPreamble( + File, *Invocation, Inputs, /*StoreInMemory=*/true, + [&](CapturedASTCtx Ctx, + const std::shared_ptr<CanonicalIncludes> Includes) { + if (!Opts.BuildDynamicSymbolIndex) + return; + log("Indexing headers..."); + Index.updatePreamble(File, /*Version=*/"null", Ctx.getASTContext(), + Ctx.getPreprocessor(), *Includes); + }); if (!Preamble) { elog("Failed to build preamble"); return false; Index: clang-tools-extra/clangd/TUScheduler.h =================================================================== --- clang-tools-extra/clangd/TUScheduler.h +++ clang-tools-extra/clangd/TUScheduler.h @@ -162,8 +162,8 @@ /// contains only AST nodes from the #include directives at the start of the /// file. AST node in the current file should be observed on onMainAST call. virtual void onPreambleAST(PathRef Path, llvm::StringRef Version, - const CompilerInvocation &CI, ASTContext &Ctx, - Preprocessor &PP, const CanonicalIncludes &) {} + CapturedASTCtx Ctx, + const std::shared_ptr<CanonicalIncludes>) {} /// The argument function is run under the critical section guarding against /// races when closing the files. Index: clang-tools-extra/clangd/TUScheduler.cpp =================================================================== --- clang-tools-extra/clangd/TUScheduler.cpp +++ clang-tools-extra/clangd/TUScheduler.cpp @@ -1084,9 +1084,9 @@ bool IsFirstPreamble = !LatestBuild; LatestBuild = clang::clangd::buildPreamble( FileName, *Req.CI, Inputs, StoreInMemory, - [&](ASTContext &Ctx, Preprocessor &PP, - const CanonicalIncludes &CanonIncludes) { - Callbacks.onPreambleAST(FileName, Inputs.Version, *Req.CI, Ctx, PP, + [&](CapturedASTCtx ASTCtx, + std::shared_ptr<CanonicalIncludes> CanonIncludes) { + Callbacks.onPreambleAST(FileName, Inputs.Version, std::move(ASTCtx), CanonIncludes); }, &Stats); Index: clang-tools-extra/clangd/Preamble.h =================================================================== --- clang-tools-extra/clangd/Preamble.h +++ clang-tools-extra/clangd/Preamble.h @@ -44,6 +44,38 @@ namespace clang { namespace clangd { +/// The captured AST conext. +/// +/// As we index the preamble in a different thread, we extend the life of +/// associated data by capturing and storing references here. +struct CapturedASTCtx { +public: + CapturedASTCtx(CompilerInstance &Clang) + : Invocation(Clang.getInvocationPtr()), + Diagnostics(Clang.getDiagnosticsPtr()), Target(Clang.getTargetPtr()), + AuxTarget(Clang.getAuxTarget()), FileMgr(Clang.getFileManagerPtr()), + SourceMgr(Clang.getSourceManagerPtr()), PP(Clang.getPreprocessorPtr()), + Context(Clang.getASTContextPtr()) {} + + ASTContext &getASTContext() { return *Context; } + Preprocessor &getPreprocessor() { return *PP; } + CompilerInvocation &getCompilerInvocation() { return *Invocation; } + void setStatCache(std::shared_ptr<PreambleFileStatusCache> StatCache) { + this->StatCache = StatCache; + } + +private: + std::shared_ptr<CompilerInvocation> Invocation; + IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics; + IntrusiveRefCntPtr<TargetInfo> Target; + IntrusiveRefCntPtr<TargetInfo> AuxTarget; + IntrusiveRefCntPtr<FileManager> FileMgr; + IntrusiveRefCntPtr<SourceManager> SourceMgr; + std::shared_ptr<Preprocessor> PP; + IntrusiveRefCntPtr<ASTContext> Context; + std::shared_ptr<PreambleFileStatusCache> StatCache; +}; + /// The parsed preamble and associated data. /// /// As we must avoid re-parsing the preamble, any information that can only @@ -69,15 +101,15 @@ std::vector<PragmaMark> Marks; // Cache of FS operations performed when building the preamble. // When reusing a preamble, this cache can be consumed to save IO. - std::unique_ptr<PreambleFileStatusCache> StatCache; - CanonicalIncludes CanonIncludes; + std::shared_ptr<PreambleFileStatusCache> StatCache; + std::shared_ptr<CanonicalIncludes> CanonIncludes; // Whether there was a (possibly-incomplete) include-guard on the main file. // We need to propagate this information "by hand" to subsequent parses. bool MainIsIncludeGuarded = false; }; -using PreambleParsedCallback = std::function<void(ASTContext &, Preprocessor &, - const CanonicalIncludes &)>; +using PreambleParsedCallback = std::function<void( + CapturedASTCtx ASTCtx, std::shared_ptr<CanonicalIncludes> CanonIncludes)>; /// Timings and statistics from the premble build. Unlike PreambleData, these /// do not need to be stored for later, but can be useful for logging, metrics, Index: clang-tools-extra/clangd/Preamble.cpp =================================================================== --- clang-tools-extra/clangd/Preamble.cpp +++ clang-tools-extra/clangd/Preamble.cpp @@ -26,7 +26,9 @@ #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" +#include "clang/Basic/TargetInfo.h" #include "clang/Basic/TokenKinds.h" +#include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Frontend/PrecompiledPreamble.h" @@ -35,6 +37,8 @@ #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" +#include "clang/Sema/Sema.h" +#include "clang/Serialization/ASTReader.h" #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" @@ -77,10 +81,9 @@ class CppFilePreambleCallbacks : public PreambleCallbacks { public: CppFilePreambleCallbacks( - PathRef File, PreambleParsedCallback ParsedCallback, - PreambleBuildStats *Stats, bool ParseForwardingFunctions, + PathRef File, PreambleBuildStats *Stats, bool ParseForwardingFunctions, std::function<void(CompilerInstance &)> BeforeExecuteCallback) - : File(File), ParsedCallback(ParsedCallback), Stats(Stats), + : File(File), Stats(Stats), ParseForwardingFunctions(ParseForwardingFunctions), BeforeExecuteCallback(std::move(BeforeExecuteCallback)) {} @@ -95,13 +98,20 @@ } CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); } + CapturedASTCtx takeLife() { return std::move(*CapturedCtx); } + bool isMainFileIncludeGuarded() const { return IsMainFileIncludeGuarded; } void AfterExecute(CompilerInstance &CI) override { - if (ParsedCallback) { - trace::Span Tracer("Running PreambleCallback"); - ParsedCallback(CI.getASTContext(), CI.getPreprocessor(), CanonIncludes); + CI.setSema(nullptr); + CI.setASTConsumer(nullptr); + if (CI.getASTReader()) { + CI.getASTReader()->setDeserializationListener(nullptr); + /* This just sets consumer to null when DeserializationListener is null */ + CI.getASTReader()->StartTranslationUnit(nullptr); } + CI.getASTContext().setASTMutationListener(nullptr); + CapturedCtx.emplace(CI); const SourceManager &SM = CI.getSourceManager(); const FileEntry *MainFE = SM.getFileEntryForID(SM.getMainFileID()); @@ -202,7 +212,6 @@ private: PathRef File; - PreambleParsedCallback ParsedCallback; IncludeStructure Includes; CanonicalIncludes CanonIncludes; include_cleaner::PragmaIncludes Pragmas; @@ -216,6 +225,7 @@ PreambleBuildStats *Stats; bool ParseForwardingFunctions; std::function<void(CompilerInstance &)> BeforeExecuteCallback; + std::optional<CapturedASTCtx> CapturedCtx; }; // Represents directives other than includes, where basic textual information is @@ -635,8 +645,7 @@ CI.getPreprocessorOpts().WriteCommentListToPCH = false; CppFilePreambleCallbacks CapturedInfo( - FileName, PreambleCallback, Stats, - Inputs.Opts.PreambleParseForwardingFunctions, + FileName, Stats, Inputs.Opts.PreambleParseForwardingFunctions, [&ASTListeners](CompilerInstance &CI) { for (const auto &L : ASTListeners) L->beforeExecute(CI); @@ -644,7 +653,7 @@ auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory); llvm::SmallString<32> AbsFileName(FileName); VFS->makeAbsolute(AbsFileName); - auto StatCache = std::make_unique<PreambleFileStatusCache>(AbsFileName); + auto StatCache = std::make_shared<PreambleFileStatusCache>(AbsFileName); auto StatCacheFS = StatCache->getProducingFS(VFS); llvm::IntrusiveRefCntPtr<TimerFS> TimedFS(new TimerFS(StatCacheFS)); @@ -679,9 +688,18 @@ Result->Pragmas = CapturedInfo.takePragmaIncludes(); Result->Macros = CapturedInfo.takeMacros(); Result->Marks = CapturedInfo.takeMarks(); - Result->CanonIncludes = CapturedInfo.takeCanonicalIncludes(); - Result->StatCache = std::move(StatCache); + Result->CanonIncludes = std::make_shared<CanonicalIncludes>( + CapturedInfo.takeCanonicalIncludes()); + Result->StatCache = StatCache; Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); + if (PreambleCallback) { + trace::Span Tracer("Running PreambleCallback"); + auto Ctx = CapturedInfo.takeLife(); + // While extending the life of FileMgr and VFS, StatCache should also be + // extended. + Ctx.setStatCache(Result->StatCache); + PreambleCallback(std::move(Ctx), Result->CanonIncludes); + } return Result; } Index: clang-tools-extra/clangd/ParsedAST.cpp =================================================================== --- clang-tools-extra/clangd/ParsedAST.cpp +++ clang-tools-extra/clangd/ParsedAST.cpp @@ -622,7 +622,7 @@ // non-preamble includes below. CanonicalIncludes CanonIncludes; if (Preamble) - CanonIncludes = Preamble->CanonIncludes; + CanonIncludes = *Preamble->CanonIncludes; else CanonIncludes.addSystemHeadersMapping(Clang->getLangOpts()); std::unique_ptr<CommentHandler> IWYUHandler = Index: clang-tools-extra/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/clangd/ClangdServer.h +++ clang-tools-extra/clangd/ClangdServer.h @@ -447,6 +447,7 @@ std::optional<std::string> WorkspaceRoot; std::optional<AsyncTaskRunner> IndexTasks; // for stdlib indexing. + std::optional<Semaphore> Barrier; std::optional<TUScheduler> WorkScheduler; // Invalidation policy used for actions that we assume are "transient". TUScheduler::ASTActionInvalidation Transient; Index: clang-tools-extra/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/clangd/ClangdServer.cpp +++ clang-tools-extra/clangd/ClangdServer.cpp @@ -61,21 +61,36 @@ struct UpdateIndexCallbacks : public ParsingCallbacks { UpdateIndexCallbacks(FileIndex *FIndex, ClangdServer::Callbacks *ServerCallbacks, - const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks) - : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS), - Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks) {} - - void onPreambleAST(PathRef Path, llvm::StringRef Version, - const CompilerInvocation &CI, ASTContext &Ctx, - Preprocessor &PP, - const CanonicalIncludes &CanonIncludes) override { - // If this preamble uses a standard library we haven't seen yet, index it. - if (FIndex) - if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo())) - indexStdlib(CI, std::move(*Loc)); + const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks, + std::optional<Semaphore> &Barrier) + : FIndex(FIndex), ServerCallbacks(ServerCallbacks), + TFS(TFS), Stdlib{std::make_shared<StdLibSet>()}, Tasks(Tasks), + Barrier(Barrier) {} + + void onPreambleAST( + PathRef Path, llvm::StringRef Version, CapturedASTCtx ASTCtx, + const std::shared_ptr<CanonicalIncludes> CanonIncludes) override { + + if (!FIndex) + return; + + auto &PP = ASTCtx.getPreprocessor(); + auto &CI = ASTCtx.getCompilerInvocation(); + if (auto Loc = Stdlib->add(*CI.getLangOpts(), PP.getHeaderSearchInfo())) + indexStdlib(CI, std::move(*Loc)); + + auto Task = [FIndex(FIndex), Path(Path), Version(Version), + ASTCtx(std::move(ASTCtx)), + CanonIncludes(CanonIncludes)]() mutable { + FIndex->updatePreamble(Path, Version, ASTCtx.getASTContext(), + ASTCtx.getPreprocessor(), *CanonIncludes); + }; - if (FIndex) - FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes); + if (Tasks) { + std::unique_lock<Semaphore> Lock(*Barrier, std::try_to_lock); + Tasks->runAsync("task:" + Path + Version, std::move(Task)); + } else + Task(); } void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) { @@ -95,10 +110,10 @@ if (Stdlib->isBest(LO)) FIndex->updatePreamble(std::move(IF)); }; - if (Tasks) - // This doesn't have a semaphore to enforce -j, but it's rare. + if (Tasks) { + std::unique_lock<Semaphore> Lock(*Barrier, std::try_to_lock); Tasks->runAsync("IndexStdlib", std::move(Task)); - else + } else Task(); } @@ -139,6 +154,7 @@ const ThreadsafeFS &TFS; std::shared_ptr<StdLibSet> Stdlib; AsyncTaskRunner *Tasks; + std::optional<Semaphore> &Barrier; }; class DraftStoreFS : public ThreadsafeFS { @@ -193,15 +209,17 @@ Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate : TUScheduler::NoInvalidation), DirtyFS(std::make_unique<DraftStoreFS>(TFS, DraftMgr)) { - if (Opts.AsyncThreadsCount != 0) + if (Opts.AsyncThreadsCount != 0) { IndexTasks.emplace(); + Barrier.emplace(1); + } // 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. WorkScheduler.emplace(CDB, TUScheduler::Options(Opts), std::make_unique<UpdateIndexCallbacks>( DynamicIdx.get(), Callbacks, TFS, - IndexTasks ? &*IndexTasks : nullptr)); + IndexTasks ? &*IndexTasks : nullptr, Barrier)); // Adds an index to the stack, at higher priority than existing indexes. auto AddIndex = [&](SymbolIndex *Idx) { if (this->Index != nullptr) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits