llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> This PR extracts the creation of `CompilerInstance` for compiling an implicitly-discovered module out of `compileModuleImpl()` into its own separate function and passes it into `compileModuleImpl()` from the outside. This makes the instance creation logic reusable (useful for my experiments) and also simplifies the API, removing the `PreBuildStep` and `PostBuildStep` hooks from `compileModuleImpl()`. --- Full diff: https://github.com/llvm/llvm-project/pull/134887.diff 1 Files Affected: - (modified) clang/lib/Frontend/CompilerInstance.cpp (+155-127) ``````````diff diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 9cab17ae70eeb..62fff9e2397a1 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1138,41 +1138,16 @@ void CompilerInstance::LoadRequestedPlugins() { } } -/// Determine the appropriate source input kind based on language -/// options. -static Language getLanguageFromOptions(const LangOptions &LangOpts) { - if (LangOpts.OpenCL) - return Language::OpenCL; - if (LangOpts.CUDA) - return Language::CUDA; - if (LangOpts.ObjC) - return LangOpts.CPlusPlus ? Language::ObjCXX : Language::ObjC; - return LangOpts.CPlusPlus ? Language::CXX : Language::C; -} - -/// Compile a module file for the given module, using the options -/// provided by the importing compiler instance. Returns true if the module -/// was built without errors. -static bool -compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, - StringRef ModuleName, FrontendInputFile Input, - StringRef OriginalModuleMapFile, StringRef ModuleFileName, - llvm::function_ref<void(CompilerInstance &)> PreBuildStep = - [](CompilerInstance &) {}, - llvm::function_ref<void(CompilerInstance &)> PostBuildStep = - [](CompilerInstance &) {}) { - llvm::TimeTraceScope TimeScope("Module Compile", ModuleName); - - // Never compile a module that's already finalized - this would cause the - // existing module to be freed, causing crashes if it is later referenced - if (ImportingInstance.getModuleCache().getInMemoryModuleCache().isPCMFinal( - ModuleFileName)) { - ImportingInstance.getDiagnostics().Report( - ImportLoc, diag::err_module_rebuild_finalized) - << ModuleName; - return false; - } - +/// Creates a \c CompilerInstance for compiling a module. +/// +/// This expects a properly initialized \c FrontendInputFile. +static std::unique_ptr<CompilerInstance> +createCompilerInstanceForModuleCompileImpl(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, + StringRef ModuleName, + FrontendInputFile Input, + StringRef OriginalModuleMapFile, + StringRef ModuleFileName) { // Construct a compiler invocation for creating this module. auto Invocation = std::make_shared<CompilerInvocation>(ImportingInstance.getInvocation()); @@ -1226,8 +1201,11 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, // module. Since we're sharing an in-memory module cache, // CompilerInstance::CompilerInstance is responsible for finalizing the // buffers to prevent use-after-frees. - CompilerInstance Instance(ImportingInstance.getPCHContainerOperations(), - &ImportingInstance.getModuleCache()); + auto InstancePtr = std::make_unique<CompilerInstance>( + ImportingInstance.getPCHContainerOperations(), + &ImportingInstance.getModuleCache()); + auto &Instance = *InstancePtr; + auto &Inv = *Invocation; Instance.setInvocation(std::move(Invocation)); @@ -1267,45 +1245,19 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, Instance.setModuleDepCollector(ImportingInstance.getModuleDepCollector()); Inv.getDependencyOutputOpts() = DependencyOutputOptions(); - ImportingInstance.getDiagnostics().Report(ImportLoc, - diag::remark_module_build) - << ModuleName << ModuleFileName; - - PreBuildStep(Instance); - - // Execute the action to actually build the module in-place. Use a separate - // thread so that we get a stack large enough. - bool Crashed = !llvm::CrashRecoveryContext().RunSafelyOnThread( - [&]() { - GenerateModuleFromModuleMapAction Action; - Instance.ExecuteAction(Action); - }, - DesiredStackSize); - - PostBuildStep(Instance); - - ImportingInstance.getDiagnostics().Report(ImportLoc, - diag::remark_module_build_done) - << ModuleName; - - // Propagate the statistics to the parent FileManager. - if (!FrontendOpts.ModulesShareFileManager) - ImportingInstance.getFileManager().AddStats(Instance.getFileManager()); - - if (Crashed) { - // Clear the ASTConsumer if it hasn't been already, in case it owns streams - // that must be closed before clearing output files. - Instance.setSema(nullptr); - Instance.setASTConsumer(nullptr); - - // Delete any remaining temporary files related to Instance. - Instance.clearOutputFiles(/*EraseFiles=*/true); - } + return InstancePtr; +} - // If \p AllowPCMWithCompilerErrors is set return 'success' even if errors - // occurred. - return !Instance.getDiagnostics().hasErrorOccurred() || - Instance.getFrontendOpts().AllowPCMWithCompilerErrors; +/// Determine the appropriate source input kind based on language +/// options. +static Language getLanguageFromOptions(const LangOptions &LangOpts) { + if (LangOpts.OpenCL) + return Language::OpenCL; + if (LangOpts.CUDA) + return Language::CUDA; + if (LangOpts.ObjC) + return LangOpts.CPlusPlus ? Language::ObjCXX : Language::ObjC; + return LangOpts.CPlusPlus ? Language::CXX : Language::C; } static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File, @@ -1321,20 +1273,24 @@ static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File, return FileMgr.getOptionalFileRef(PublicFilename); } -/// Compile a module file for the given module in a separate compiler instance, -/// using the options provided by the importing compiler instance. Returns true -/// if the module was built without errors. -static bool compileModule(CompilerInstance &ImportingInstance, - SourceLocation ImportLoc, Module *Module, - StringRef ModuleFileName) { +/// Creates a \c CompilerInstance for compiling a module. +/// +/// This takes care of creating appropriate \c FrontendInputFile for +/// public/private frameworks, inferred modules and such. +static std::unique_ptr<CompilerInstance> +createCompilerInstanceForModuleCompile(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, Module *Module, + StringRef ModuleFileName) { + StringRef ModuleName = Module->getTopLevelModuleName(); + InputKind IK(getLanguageFromOptions(ImportingInstance.getLangOpts()), InputKind::ModuleMap); // Get or create the module map that we'll use to build this module. - ModuleMap &ModMap - = ImportingInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap(); + ModuleMap &ModMap = + ImportingInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap(); SourceManager &SourceMgr = ImportingInstance.getSourceManager(); - bool Result; + if (FileID ModuleMapFID = ModMap.getContainingModuleMapFileID(Module); ModuleMapFID.isValid()) { // We want to use the top-level module map. If we don't, the compiling @@ -1368,44 +1324,115 @@ static bool compileModule(CompilerInstance &ImportingInstance, bool IsSystem = isSystem(SLoc.getFile().getFileCharacteristic()); // Use the module map where this module resides. - Result = compileModuleImpl( - ImportingInstance, ImportLoc, Module->getTopLevelModuleName(), + return createCompilerInstanceForModuleCompileImpl( + ImportingInstance, ImportLoc, ModuleName, FrontendInputFile(ModuleMapFilePath, IK, IsSystem), ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName); - } else { - // FIXME: We only need to fake up an input file here as a way of - // transporting the module's directory to the module map parser. We should - // be able to do that more directly, and parse from a memory buffer without - // inventing this file. - SmallString<128> FakeModuleMapFile(Module->Directory->getName()); - llvm::sys::path::append(FakeModuleMapFile, "__inferred_module.map"); - - std::string InferredModuleMapContent; - llvm::raw_string_ostream OS(InferredModuleMapContent); - Module->print(OS); - - Result = compileModuleImpl( - ImportingInstance, ImportLoc, Module->getTopLevelModuleName(), - FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem), - ModMap.getModuleMapFileForUniquing(Module)->getName(), - ModuleFileName, - [&](CompilerInstance &Instance) { - std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer = - llvm::MemoryBuffer::getMemBuffer(InferredModuleMapContent); - FileEntryRef ModuleMapFile = Instance.getFileManager().getVirtualFileRef( - FakeModuleMapFile, InferredModuleMapContent.size(), 0); - Instance.getSourceManager().overrideFileContents( - ModuleMapFile, std::move(ModuleMapBuffer)); - }); } + // FIXME: We only need to fake up an input file here as a way of + // transporting the module's directory to the module map parser. We should + // be able to do that more directly, and parse from a memory buffer without + // inventing this file. + SmallString<128> FakeModuleMapFile(Module->Directory->getName()); + llvm::sys::path::append(FakeModuleMapFile, "__inferred_module.map"); + + std::string InferredModuleMapContent; + llvm::raw_string_ostream OS(InferredModuleMapContent); + Module->print(OS); + + auto Instance = createCompilerInstanceForModuleCompileImpl( + ImportingInstance, ImportLoc, ModuleName, + FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem), + ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName); + + std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer = + llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent); + FileEntryRef ModuleMapFile = Instance->getFileManager().getVirtualFileRef( + FakeModuleMapFile, InferredModuleMapContent.size(), 0); + Instance->getSourceManager().overrideFileContents(ModuleMapFile, + std::move(ModuleMapBuffer)); + + return Instance; +} + +/// Compile a module file for the given module, using the options +/// provided by the importing compiler instance. Returns true if the module +/// was built without errors. +static bool compileModuleImpl(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, StringRef ModuleName, + StringRef ModuleFileName, + CompilerInstance &Instance) { + llvm::TimeTraceScope TimeScope("Module Compile", ModuleName); + + ImportingInstance.getDiagnostics().Report(ImportLoc, + diag::remark_module_build) + << ModuleName << ModuleFileName; + + // Execute the action to actually build the module in-place. Use a separate + // thread so that we get a stack large enough. + bool Crashed = !llvm::CrashRecoveryContext().RunSafelyOnThread( + [&]() { + GenerateModuleFromModuleMapAction Action; + Instance.ExecuteAction(Action); + }, + DesiredStackSize); + + ImportingInstance.getDiagnostics().Report(ImportLoc, + diag::remark_module_build_done) + << ModuleName; + + // Propagate the statistics to the parent FileManager. + if (ImportingInstance.getFileManagerPtr() != Instance.getFileManagerPtr()) + ImportingInstance.getFileManager().AddStats(Instance.getFileManager()); + + if (Crashed) { + // Clear the ASTConsumer if it hasn't been already, in case it owns streams + // that must be closed before clearing output files. + Instance.setSema(nullptr); + Instance.setASTConsumer(nullptr); + + // Delete any remaining temporary files related to Instance. + Instance.clearOutputFiles(/*EraseFiles=*/true); + } + + // If \p AllowPCMWithCompilerErrors is set return 'success' even if errors + // occurred. + return !Instance.getDiagnostics().hasErrorOccurred() || + Instance.getFrontendOpts().AllowPCMWithCompilerErrors; +} + +/// Compile a module file for the given module in a separate compiler instance, +/// using the options provided by the importing compiler instance. Returns true +/// if the module was built without errors. +static bool compileModule(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, Module *Module, + StringRef ModuleFileName) { + StringRef ModuleName = Module->getTopLevelModuleName(); + + // Never compile a module that's already finalized - this would cause the + // existing module to be freed, causing crashes if it is later referenced + if (ImportingInstance.getModuleCache().getInMemoryModuleCache().isPCMFinal( + ModuleFileName)) { + ImportingInstance.getDiagnostics().Report( + ImportLoc, diag::err_module_rebuild_finalized) + << ModuleName; + return false; + } + + auto Instance = createCompilerInstanceForModuleCompile( + ImportingInstance, ImportLoc, Module, ModuleFileName); + + bool Success = compileModuleImpl(ImportingInstance, ImportLoc, ModuleName, + ModuleFileName, *Instance); + // We've rebuilt a module. If we're allowed to generate or update the global // module index, record that fact in the importing compiler instance. if (ImportingInstance.getFrontendOpts().GenerateGlobalModuleIndex) { ImportingInstance.setBuildGlobalModuleIndex(true); } - return Result; + return Success; } /// Read the AST right after compiling the module. @@ -2232,25 +2259,26 @@ void CompilerInstance::createModuleFromSource(SourceLocation ImportLoc, std::string NullTerminatedSource(Source.str()); - auto PreBuildStep = [&](CompilerInstance &Other) { - // Create a virtual file containing our desired source. - // FIXME: We shouldn't need to do this. - FileEntryRef ModuleMapFile = Other.getFileManager().getVirtualFileRef( - ModuleMapFileName, NullTerminatedSource.size(), 0); - Other.getSourceManager().overrideFileContents( - ModuleMapFile, llvm::MemoryBuffer::getMemBuffer(NullTerminatedSource)); + auto Other = createCompilerInstanceForModuleCompileImpl( + *this, ImportLoc, ModuleName, Input, StringRef(), ModuleFileName); - Other.BuiltModules = std::move(BuiltModules); - Other.DeleteBuiltModules = false; - }; + // Create a virtual file containing our desired source. + // FIXME: We shouldn't need to do this. + FileEntryRef ModuleMapFile = Other->getFileManager().getVirtualFileRef( + ModuleMapFileName, NullTerminatedSource.size(), 0); + Other->getSourceManager().overrideFileContents( + ModuleMapFile, llvm::MemoryBuffer::getMemBuffer(NullTerminatedSource)); - auto PostBuildStep = [this](CompilerInstance &Other) { - BuiltModules = std::move(Other.BuiltModules); - }; + Other->BuiltModules = std::move(BuiltModules); + Other->DeleteBuiltModules = false; // Build the module, inheriting any modules that we've built locally. - if (compileModuleImpl(*this, ImportLoc, ModuleName, Input, StringRef(), - ModuleFileName, PreBuildStep, PostBuildStep)) { + bool Success = + compileModuleImpl(*this, ImportLoc, ModuleName, ModuleFileName, *Other); + + BuiltModules = std::move(Other->BuiltModules); + + if (Success) { BuiltModules[std::string(ModuleName)] = std::string(ModuleFileName); llvm::sys::RemoveFileOnSignal(ModuleFileName); } `````````` </details> https://github.com/llvm/llvm-project/pull/134887 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
