llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (Sirraide) <details> <summary>Changes</summary> Essentially, figuring out how to use `CodeGenerator` was very confusing to me and I figured the API could be improved a bit, so: - the `CodeGenerator` ctor is now protected since an instance of `CodeGenerator` that is not a `CodeGeneratorImpl` is a bit useless (and deriving from it and implementing it yourself honestly just defeats the point of using this to begin with); - `ReleaseModule()` releases ownership of the module, so it should return a `unique_ptr`; - `CreateLLVMCodeGen()` also returns a `unique_ptr` now; - added a `CreateLLVMCodeGen()` overload that takes a `CompilerInstance&` and uses some of its state instead of requiring the user to pass everything in manually; this is consistent w/ other parts of our API, and most uses of this function in the codebase can be refactored to use that overload instead (and a code search I did also showed that a lot of people that use this API also just use the state from a `CompilerInstance`). I should have liked to replace `CreateLLVMCodeGen` w/ `CodeGenerator::Create`, but there are a lot of uses of `CreateLLVMCodeGen()` in the wild, so the only thing we could do is keep `CreateLLVMCodeGen()` and deprecate it, and at that point I don’t think it’s really worth it; I added a comment to the `CodeGenerator()` constructor declaration that points to it though. clang-format is definitely going to complain because the indentation in `ModuleBuilder.cpp` is all wrong, but I don’t think we should just reformat that file. A release note for this might be nice, but I’m not sure what section it should go in (if we want one at all) Fixes #<!-- -->172169. --- Full diff: https://github.com/llvm/llvm-project/pull/175239.diff 6 Files Affected: - (modified) clang/include/clang/CodeGen/ModuleBuilder.h (+19-13) - (modified) clang/lib/CodeGen/CodeGenAction.cpp (+1-3) - (modified) clang/lib/CodeGen/ModuleBuilder.cpp (+21-12) - (modified) clang/tools/clang-import-test/clang-import-test.cpp (+1-4) - (modified) clang/unittests/CodeGen/TestCompiler.h (+1-4) - (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+2-5) ``````````diff diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index 4298ba06c472e..5173fd05d75d6 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -40,6 +40,7 @@ namespace clang { class HeaderSearchOptions; class LangOptions; class PreprocessorOptions; + class CompilerInstance; namespace CodeGen { class CodeGenModule; @@ -47,12 +48,13 @@ namespace CodeGen { } /// The primary public interface to the Clang code generator. -/// -/// This is not really an abstract interface. class CodeGenerator : public ASTConsumer { virtual void anchor(); protected: + /// Use CreateLLVMCodeGen() below to create an instance of this class. + CodeGenerator() = default; + /// True if we've finished generating IR. This prevents us from generating /// additional LLVM IR after emitting output in HandleTranslationUnit. This /// can happen when Clang plugins trigger additional AST deserialization. @@ -78,7 +80,7 @@ class CodeGenerator : public ASTConsumer { /// /// It is illegal to call methods other than GetModule on the /// CodeGenerator after releasing its module. - llvm::Module *ReleaseModule(); + std::unique_ptr<llvm::Module> ReleaseModule(); /// Return debug info code generator. CodeGen::CGDebugInfo *getCGDebugInfo(); @@ -109,16 +111,20 @@ class CodeGenerator : public ASTConsumer { }; /// CreateLLVMCodeGen - Create a CodeGenerator instance. -/// It is the responsibility of the caller to call delete on -/// the allocated CodeGenerator instance. -CodeGenerator *CreateLLVMCodeGen(DiagnosticsEngine &Diags, - llvm::StringRef ModuleName, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, - const HeaderSearchOptions &HeaderSearchOpts, - const PreprocessorOptions &PreprocessorOpts, - const CodeGenOptions &CGO, - llvm::LLVMContext &C, - CoverageSourceInfo *CoverageInfo = nullptr); +/// +/// Remember to call Initialize() if you plan to use this directly. +std::unique_ptr<CodeGenerator> +CreateLLVMCodeGen(const CompilerInstance &CI, llvm::StringRef ModuleName, + llvm::LLVMContext &C, + CoverageSourceInfo *CoverageInfo = nullptr); + +std::unique_ptr<CodeGenerator> +CreateLLVMCodeGen(DiagnosticsEngine &Diags, llvm::StringRef ModuleName, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, + const HeaderSearchOptions &HeaderSearchOpts, + const PreprocessorOptions &PreprocessorOpts, + const CodeGenOptions &CGO, llvm::LLVMContext &C, + CoverageSourceInfo *CoverageInfo = nullptr); namespace CodeGen { /// Demangle the artificial function name (\param FuncName) used to encode trap diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 60d6b7fa009e7..a5ef4ac9d361d 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -118,9 +118,7 @@ BackendConsumer::BackendConsumer(CompilerInstance &CI, BackendAction Action, : CI(CI), Diags(CI.getDiagnostics()), CodeGenOpts(CI.getCodeGenOpts()), TargetOpts(CI.getTargetOpts()), LangOpts(CI.getLangOpts()), AsmOutStream(std::move(OS)), FS(VFS), Action(Action), - Gen(CreateLLVMCodeGen(Diags, InFile, std::move(VFS), - CI.getHeaderSearchOpts(), CI.getPreprocessorOpts(), - CI.getCodeGenOpts(), C, CoverageInfo)), + Gen(CreateLLVMCodeGen(CI, InFile, C, CoverageInfo)), LinkModules(std::move(LinkModules)), CurLinkModule(CurLinkModule) { TimerIsEnabled = CodeGenOpts.TimePasses; llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses; diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp index 8ec8aef311656..b4885d572c294 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -19,6 +19,7 @@ #include "clang/Basic/CodeGenOptions.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/TargetInfo.h" +#include "clang/Frontend/CompilerInstance.h" #include "llvm/ADT/StringRef.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/LLVMContext.h" @@ -31,7 +32,7 @@ using namespace clang; using namespace CodeGen; namespace { - class CodeGeneratorImpl : public CodeGenerator { + class CodeGeneratorImpl final : public CodeGenerator { DiagnosticsEngine &Diags; ASTContext *Ctx; IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS; // Only used for debug info. @@ -60,12 +61,8 @@ namespace { }; CoverageSourceInfo *CoverageInfo; - - protected: std::unique_ptr<llvm::Module> M; std::unique_ptr<CodeGen::CodeGenModule> Builder; - - private: SmallVector<FunctionDecl *, 8> DeferredInlineMemberFuncDefs; static llvm::StringRef ExpandModuleName(llvm::StringRef ModuleName, @@ -107,8 +104,8 @@ namespace { return Builder->getModuleDebugInfo(); } - llvm::Module *ReleaseModule() { - return M.release(); + std::unique_ptr<llvm::Module> ReleaseModule() { + return std::exchange(M, nullptr); } const Decl *GetDeclForMangledName(StringRef MangledName) { @@ -143,6 +140,7 @@ namespace { std::unique_ptr<CodeGenModule> OldBuilder = std::move(Builder); + assert(Ctx && "must call Initialize() before calling StartModule()"); Initialize(*Ctx); if (OldBuilder) @@ -251,6 +249,7 @@ namespace { // For MSVC compatibility, treat declarations of static data members with // inline initializers as definitions. + assert(Ctx && "Initialize() not called"); if (Ctx->getTargetInfo().getCXXABI().isMicrosoft()) { for (Decl *Member : D->decls()) { if (VarDecl *VD = dyn_cast<VarDecl>(Member)) { @@ -341,7 +340,7 @@ llvm::Module *CodeGenerator::GetModule() { return static_cast<CodeGeneratorImpl*>(this)->GetModule(); } -llvm::Module *CodeGenerator::ReleaseModule() { +std::unique_ptr<llvm::Module> CodeGenerator::ReleaseModule() { return static_cast<CodeGeneratorImpl*>(this)->ReleaseModule(); } @@ -368,16 +367,26 @@ llvm::Module *CodeGenerator::StartModule(llvm::StringRef ModuleName, return static_cast<CodeGeneratorImpl*>(this)->StartModule(ModuleName, C); } -CodeGenerator * +std::unique_ptr<CodeGenerator> clang::CreateLLVMCodeGen(DiagnosticsEngine &Diags, llvm::StringRef ModuleName, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, const HeaderSearchOptions &HeaderSearchOpts, const PreprocessorOptions &PreprocessorOpts, const CodeGenOptions &CGO, llvm::LLVMContext &C, CoverageSourceInfo *CoverageInfo) { - return new CodeGeneratorImpl(Diags, ModuleName, std::move(FS), - HeaderSearchOpts, PreprocessorOpts, CGO, C, - CoverageInfo); + return std::make_unique<CodeGeneratorImpl>(Diags, ModuleName, std::move(FS), + HeaderSearchOpts, PreprocessorOpts, + CGO, C, CoverageInfo); +} + +std::unique_ptr<CodeGenerator> +clang::CreateLLVMCodeGen(const CompilerInstance &CI, StringRef ModuleName, + llvm::LLVMContext &C, + CoverageSourceInfo *CoverageInfo) { + return CreateLLVMCodeGen(CI.getDiagnostics(), ModuleName, + CI.getVirtualFileSystemPtr(), + CI.getHeaderSearchOpts(), CI.getPreprocessorOpts(), + CI.getCodeGenOpts(), C, CoverageInfo); } namespace clang { diff --git a/clang/tools/clang-import-test/clang-import-test.cpp b/clang/tools/clang-import-test/clang-import-test.cpp index 977cec1d53157..8e83687d3e96a 100644 --- a/clang/tools/clang-import-test/clang-import-test.cpp +++ b/clang/tools/clang-import-test/clang-import-test.cpp @@ -235,10 +235,7 @@ BuildASTContext(CompilerInstance &CI, SelectorTable &ST, Builtin::Context &BC) { std::unique_ptr<CodeGenerator> BuildCodeGen(CompilerInstance &CI, llvm::LLVMContext &LLVMCtx) { StringRef ModuleName("$__module"); - return std::unique_ptr<CodeGenerator>(CreateLLVMCodeGen( - CI.getDiagnostics(), ModuleName, CI.getVirtualFileSystemPtr(), - CI.getHeaderSearchOpts(), CI.getPreprocessorOpts(), CI.getCodeGenOpts(), - LLVMCtx)); + return CreateLLVMCodeGen(CI, ModuleName, LLVMCtx); } } // namespace init_convenience diff --git a/clang/unittests/CodeGen/TestCompiler.h b/clang/unittests/CodeGen/TestCompiler.h index 9bd90609fcd29..18947584bd0b3 100644 --- a/clang/unittests/CodeGen/TestCompiler.h +++ b/clang/unittests/CodeGen/TestCompiler.h @@ -57,10 +57,7 @@ struct TestCompiler { compiler.createASTContext(); - CG.reset(CreateLLVMCodeGen( - compiler.getDiagnostics(), "main-module", - compiler.getVirtualFileSystemPtr(), compiler.getHeaderSearchOpts(), - compiler.getPreprocessorOpts(), compiler.getCodeGenOpts(), Context)); + CG = CreateLLVMCodeGen(compiler, "main-module", Context); } void init(const char *TestProgram, diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index bae3c44e333b6..25259cd4bdcf5 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -874,11 +874,8 @@ ClangExpressionParser::ClangExpressionParser( std::string module_name("$__lldb_module"); m_llvm_context = std::make_unique<LLVMContext>(); - m_code_generator.reset(CreateLLVMCodeGen( - m_compiler->getDiagnostics(), module_name, - m_compiler->getVirtualFileSystemPtr(), m_compiler->getHeaderSearchOpts(), - m_compiler->getPreprocessorOpts(), m_compiler->getCodeGenOpts(), - *m_llvm_context)); + m_code_generator = + CreateLLVMCodeGen(*m_compiler, module_name, *m_llvm_context); } ClangExpressionParser::~ClangExpressionParser() = default; `````````` </details> https://github.com/llvm/llvm-project/pull/175239 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
