From 9bcee64b328fd2a458d64e7137f3750d6770d35b Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 31 Jul 2026 11:08:13 -0700 Subject: [PATCH] Add some encapsulation to `CppDomain` (#7579) Start tracking the domain within `CppContext`s instead of having them duplicate its fields. This allows us to remove the shared ownership of the clang parser. --------- Co-authored-by: Christopher Di Bella --- toolchain/check/BUILD | 2 ++ toolchain/check/check.cpp | 34 ++++++++++-------- toolchain/check/check.h | 2 +- toolchain/check/check_unit.cpp | 2 +- toolchain/check/check_unit.h | 4 +-- toolchain/check/cpp/context.cpp | 18 ++++++---- toolchain/check/cpp/context.h | 25 +++++++------ toolchain/check/cpp/domain.cpp | 22 ++++++++++++ toolchain/check/cpp/domain.h | 53 ++++++++++++++++++++++++++++ toolchain/check/cpp/generate_ast.cpp | 31 ++++++++-------- toolchain/check/cpp/generate_ast.h | 11 ++---- toolchain/check/cpp/import.cpp | 1 + toolchain/check/cpp/import.h | 2 +- 13 files changed, 141 insertions(+), 66 deletions(-) create mode 100644 toolchain/check/cpp/domain.cpp create mode 100644 toolchain/check/cpp/domain.h diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 8c8c88af7070..160e16cf7ac7 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -27,6 +27,7 @@ cc_library( "cpp/context.cpp", "cpp/custom_type_mapping.cpp", "cpp/diagnostic_consumer.cpp", + "cpp/domain.cpp", "cpp/export.cpp", "cpp/generate_ast.cpp", "cpp/impl_lookup.cpp", @@ -93,6 +94,7 @@ cc_library( "cpp/custom_type_mapping.h", "cpp/diagnostic_consumer.h", "cpp/diagnostic_listener.h", + "cpp/domain.h", "cpp/export.h", "cpp/generate_ast.h", "cpp/impl_lookup.h", diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 6204e764b9b1..b62ceb5ee2e4 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -16,6 +16,7 @@ #include "common/pretty_stack_trace_function.h" #include "toolchain/check/check_unit.h" #include "toolchain/check/context.h" +#include "toolchain/check/cpp/domain.h" #include "toolchain/check/cpp/generate_ast.h" #include "toolchain/check/cpp/import.h" #include "toolchain/check/diagnostic_emitter.h" @@ -416,6 +417,12 @@ auto CheckParseTrees( llvm::IntrusiveRefCntPtr fs, const CheckParseTreesOptions& options, std::shared_ptr clang_invocation) -> void { + // C++ domains used across files. When compiling with a single ASTContext + // (`options.share_cpp_ast`), there is only a single shared domain. This + // variable is created early so the domains outlive the `UnitAndImports` that + // reference them. + llvm::SmallVector> cpp_domains; + // UnitAndImports is big due to its SmallVectors, so we default to 0 on the // stack. llvm::SmallVector unit_infos( @@ -498,19 +505,16 @@ auto CheckParseTrees( } } - // C++ domains used across files. When compiling with a single ASTContext - // (`options.share_cpp_ast`), there is only a single shared domain. - llvm::SmallVector> cpp_domains; + // Create C++ domains for Cpp imports. if (options.share_cpp_ast) { // TODO: Remove dependence on properties of the first unit here. - auto shared_cpp_domain = InitializeCppDomain( - unit_infos.front().err_tracker, - unit_infos.front().unit->sem_ir->filename(), fs, - unit_infos.front().unit->llvm_context, clang_invocation); - if (shared_cpp_domain) { - cpp_domains.push_back(shared_cpp_domain); + if (auto cpp_domain = InitializeCppDomain( + unit_infos.front().err_tracker, + unit_infos.front().unit->sem_ir->filename(), fs, + unit_infos.front().unit->llvm_context, clang_invocation)) { + cpp_domains.push_back(std::move(cpp_domain)); for (auto& target_info : unit_infos) { - target_info.cpp_domain = shared_cpp_domain; + target_info.cpp_domain = cpp_domains.back().get(); } } } else { @@ -518,11 +522,11 @@ auto CheckParseTrees( if (unit_info.cpp_imports.empty()) { continue; } - unit_info.cpp_domain = InitializeCppDomain( - unit_info.err_tracker, unit_info.unit->sem_ir->filename(), fs, - unit_info.unit->llvm_context, clang_invocation); - if (unit_info.cpp_domain) { - cpp_domains.push_back(unit_info.cpp_domain); + if (auto cpp_domain = InitializeCppDomain( + unit_info.err_tracker, unit_info.unit->sem_ir->filename(), fs, + unit_info.unit->llvm_context, clang_invocation)) { + cpp_domains.push_back(std::move(cpp_domain)); + unit_info.cpp_domain = cpp_domains.back().get(); } } } diff --git a/toolchain/check/check.h b/toolchain/check/check.h index 04f3ced60631..927c57c831b4 100644 --- a/toolchain/check/check.h +++ b/toolchain/check/check.h @@ -32,7 +32,7 @@ struct Unit { int total_ir_count; }; -struct CppDomain; +class CppDomain; struct CheckParseTreesOptions { // Options must be set individually, not through initialization. diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index 81a07c29f37c..be8ab124ecd0 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -160,7 +160,7 @@ auto CheckUnit::InitPackageScopeAndImports() -> void { ImportOtherPackages(namespace_type_id); ImportCpp(context_, unit_and_imports_->cpp_imports, - unit_and_imports_->cpp_domain.get()); + unit_and_imports_->cpp_domain); } auto CheckUnit::CollectDirectImports( diff --git a/toolchain/check/check_unit.h b/toolchain/check/check_unit.h index 9815167fb1e3..47deae313b9c 100644 --- a/toolchain/check/check_unit.h +++ b/toolchain/check/check_unit.h @@ -20,7 +20,7 @@ class CompilerInvocation; namespace Carbon::Check { struct UnitAndImports; -struct CppDomain; +class CppDomain; // A file's imports corresponding to a single package, for // `UnitAndImports::package_imports`. @@ -103,7 +103,7 @@ struct UnitAndImports { llvm::SmallVector cpp_imports; // The C++ domain for this unit. - std::shared_ptr cpp_domain; + CppDomain* cpp_domain = nullptr; // The remaining number of imports which must be checked before this unit can // be processed. diff --git a/toolchain/check/cpp/context.cpp b/toolchain/check/cpp/context.cpp index 1f81b7b689e5..ae0bf2acadd5 100644 --- a/toolchain/check/cpp/context.cpp +++ b/toolchain/check/cpp/context.cpp @@ -6,20 +6,24 @@ #include "clang/AST/Mangle.h" #include "clang/Frontend/CompilerInstance.h" -#include "clang/Parse/Parser.h" +#include "toolchain/check/cpp/domain.h" namespace Carbon::Check { -CppContext::CppContext(clang::CompilerInstance& instance, - std::shared_ptr parser, +CppContext::CppContext(CppDomain& domain, std::unique_ptr listener) - : ast_context_(&instance.getASTContext()), - sema_(&instance.getSema()), - parser_(std::move(parser)), - diagnostic_listener_(std::move(listener)) {} + : domain_(&domain), diagnostic_listener_(std::move(listener)) {} CppContext::~CppContext() = default; +auto CppContext::ast_context() -> clang::ASTContext& { + return domain_->clang_instance().getASTContext(); +} + +auto CppContext::sema() -> clang::Sema& { + return domain_->clang_instance().getSema(); +} + auto CppContext::clang_mangle_context() -> clang::MangleContext& { if (!clang_mangle_context_) { clang_mangle_context_.reset(ast_context().createMangleContext()); diff --git a/toolchain/check/cpp/context.h b/toolchain/check/cpp/context.h index cffcd5eb14b1..94518cc4dfd3 100644 --- a/toolchain/check/cpp/context.h +++ b/toolchain/check/cpp/context.h @@ -11,6 +11,7 @@ #include "common/check.h" #include "llvm/ADT/SmallVector.h" #include "toolchain/check/cpp/diagnostic_listener.h" +#include "toolchain/check/cpp/domain.h" namespace clang { class ASTContext; @@ -30,15 +31,16 @@ namespace Carbon::Check { // declarations, and similar values. class CppContext { public: - explicit CppContext(clang::CompilerInstance& instance, - std::shared_ptr parser, + explicit CppContext(CppDomain& domain, std::unique_ptr listener); ~CppContext(); - auto ast_context() -> clang::ASTContext& { return *ast_context_; } - auto sema() -> clang::Sema& { return *sema_; } - auto parser() -> clang::Parser& { return *parser_; } - auto parser_ptr() const -> std::shared_ptr { return parser_; } + auto ast_context() -> clang::ASTContext&; + auto sema() -> clang::Sema&; + auto parser() -> clang::Parser& { return domain_->parser(); } + + auto domain() -> CppDomain& { return *domain_; } + auto domain() const -> const CppDomain& { return *domain_; } auto clang_mangle_context() -> clang::MangleContext&; @@ -54,14 +56,11 @@ class CppContext { } private: - // The Clang AST context. - clang::ASTContext* ast_context_; + // The C++ compilation domain. + CppDomain* domain_; - // The Clang semantic analysis engine. - clang::Sema* sema_; - - // The Clang parser. - std::shared_ptr parser_; + // TODO: All of the below state that is not specific to a particular + // Check::Context or SemIR::File should be moved into CppDomain. // Per-Carbon-file start locations for corresponding Clang source buffers. // Owned and managed by code in location.cpp. diff --git a/toolchain/check/cpp/domain.cpp b/toolchain/check/cpp/domain.cpp new file mode 100644 index 000000000000..68a0f939fda3 --- /dev/null +++ b/toolchain/check/cpp/domain.cpp @@ -0,0 +1,22 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/check/cpp/domain.h" + +#include "clang/Parse/Parser.h" + +namespace Carbon::Check { + +CppDomain::CppDomain(std::shared_ptr clang_instance, + std::unique_ptr parser, + clang::CodeGenerator* code_generator, + llvm::LLVMContext* llvm_context) + : clang_instance_(std::move(clang_instance)), + parser_(std::move(parser)), + code_generator_(code_generator), + llvm_context_(llvm_context) {} + +CppDomain::~CppDomain() = default; + +} // namespace Carbon::Check diff --git a/toolchain/check/cpp/domain.h b/toolchain/check/cpp/domain.h new file mode 100644 index 000000000000..8f08be69125e --- /dev/null +++ b/toolchain/check/cpp/domain.h @@ -0,0 +1,53 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CARBON_TOOLCHAIN_CHECK_CPP_DOMAIN_H_ +#define CARBON_TOOLCHAIN_CHECK_CPP_DOMAIN_H_ + +#include + +namespace clang { +class CodeGenerator; +class CompilerInstance; +class Parser; +} // namespace clang + +namespace llvm { +class LLVMContext; +} // namespace llvm + +namespace Carbon::Check { + +// A C++ compilation domain, including a live Clang instance that can be used to +// parse more code into that domain. May be shared across multiple Carbon files. +class CppDomain { + public: + explicit CppDomain(std::shared_ptr clang_instance, + std::unique_ptr parser, + clang::CodeGenerator* code_generator, + llvm::LLVMContext* llvm_context); + ~CppDomain(); + + auto clang_instance() const -> clang::CompilerInstance& { + return *clang_instance_; + } + auto clang_instance_ptr() const -> std::shared_ptr { + return clang_instance_; + } + auto parser() const -> clang::Parser& { return *parser_; } + auto code_generator() const -> clang::CodeGenerator* { + return code_generator_; + } + auto llvm_context() const -> llvm::LLVMContext* { return llvm_context_; } + + private: + std::shared_ptr clang_instance_; + std::unique_ptr parser_; + clang::CodeGenerator* code_generator_ = nullptr; + llvm::LLVMContext* llvm_context_ = nullptr; +}; + +} // namespace Carbon::Check + +#endif // CARBON_TOOLCHAIN_CHECK_CPP_DOMAIN_H_ diff --git a/toolchain/check/cpp/generate_ast.cpp b/toolchain/check/cpp/generate_ast.cpp index cc3f65e23039..31b0353fa5e0 100644 --- a/toolchain/check/cpp/generate_ast.cpp +++ b/toolchain/check/cpp/generate_ast.cpp @@ -33,6 +33,7 @@ #include "toolchain/check/cpp/access.h" #include "toolchain/check/cpp/diagnostic_consumer.h" #include "toolchain/check/cpp/diagnostic_listener.h" +#include "toolchain/check/cpp/domain.h" #include "toolchain/check/cpp/export.h" #include "toolchain/check/cpp/import.h" #include "toolchain/check/cpp/location.h" @@ -727,7 +728,7 @@ auto InitializeCppDomain( llvm::IntrusiveRefCntPtr fs, llvm::LLVMContext* llvm_context, std::shared_ptr base_invocation) - -> std::shared_ptr { + -> std::unique_ptr { std::shared_ptr clang_instance; llvm::IntrusiveRefCntPtr diags; @@ -810,11 +811,9 @@ auto InitializeCppDomain( auto parser = action.TakeParser(); CARBON_CHECK(parser); - return std::make_shared( - CppDomain{.clang_instance = std::move(clang_instance), - .parser = std::move(parser), - .code_generator = action.code_generator(), - .llvm_context = llvm_context}); + return std::make_unique(std::move(clang_instance), + std::move(parser), action.code_generator(), + llvm_context); } auto GenerateAst(Context& context, @@ -830,22 +829,20 @@ auto GenerateAst(Context& context, Diagnostics::AnnotationScope annotate_diagnostics(&context.emitter(), [](auto& /*builder*/) {}); - auto clang_instance = domain.clang_instance; - auto parser = domain.parser; + auto clang_instance = domain.clang_instance_ptr(); // Set up CppFile for the current SemIR::File. auto cpp_file = - std::make_unique(clang_instance, domain.llvm_context); - if (domain.code_generator) { - cpp_file->SetCodeGenerator(domain.code_generator); + std::make_unique(clang_instance, domain.llvm_context()); + if (domain.code_generator()) { + cpp_file->SetCodeGenerator(domain.code_generator()); } context.sem_ir().set_cpp_file(std::move(cpp_file)); // Set up CppContext for the current Context. context.set_cpp_context(std::make_unique( - *clang_instance, parser, - MakeContextDiagnosticListener( - *clang_instance->getDiagnostics().getClient(), context))); + domain, MakeContextDiagnosticListener( + *clang_instance->getDiagnostics().getClient(), context))); // The AST context is now available, so the mangle context (used to compute // stable identities for imported C++ types) can be created. @@ -917,10 +914,10 @@ auto FinishAst(Context& context) -> void { } auto FinalizeCppDomain(CppDomain& domain) -> void { - if (domain.clang_instance) { - domain.clang_instance->getSema().ActOnEndOfTranslationUnit(); + if (domain.clang_instance_ptr()) { + domain.clang_instance().getSema().ActOnEndOfTranslationUnit(); FlushDiagnosticConsumer( - *domain.clang_instance->getDiagnostics().getClient()); + *domain.clang_instance().getDiagnostics().getClient()); } } diff --git a/toolchain/check/cpp/generate_ast.h b/toolchain/check/cpp/generate_ast.h index d861d850418f..0a6794527be6 100644 --- a/toolchain/check/cpp/generate_ast.h +++ b/toolchain/check/cpp/generate_ast.h @@ -29,14 +29,7 @@ class Consumer; namespace Carbon::Check { -// A C++ compilation domain, including a live Clang instance that can be used to -// parse more code into that domain. May be shared across multiple Carbon files. -struct CppDomain { - std::shared_ptr clang_instance; - std::shared_ptr parser; - clang::CodeGenerator* code_generator = nullptr; - llvm::LLVMContext* llvm_context = nullptr; -}; +class CppDomain; // Initializes a Clang compilation instance, which can be used to parse C++ code // within one or more Carbon files. Returns the initialized state, or null on @@ -46,7 +39,7 @@ auto InitializeCppDomain( llvm::IntrusiveRefCntPtr fs, llvm::LLVMContext* llvm_context, std::shared_ptr base_invocation) - -> std::shared_ptr; + -> std::unique_ptr; // Generates a Clang AST for the given C++ imports and sets it as the context's // `cpp_context` and the SemIR's `cpp_file`. Returns a bool that represents diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index cceed53aa963..90fcc9041efe 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -39,6 +39,7 @@ #include "toolchain/check/core_identifier.h" #include "toolchain/check/cpp/access.h" #include "toolchain/check/cpp/custom_type_mapping.h" +#include "toolchain/check/cpp/domain.h" #include "toolchain/check/cpp/generate_ast.h" #include "toolchain/check/cpp/location.h" #include "toolchain/check/cpp/macros.h" diff --git a/toolchain/check/cpp/import.h b/toolchain/check/cpp/import.h index b209307fd8ff..1d6663f025af 100644 --- a/toolchain/check/cpp/import.h +++ b/toolchain/check/cpp/import.h @@ -25,7 +25,7 @@ class VarDecl; namespace Carbon::Check { -struct CppDomain; +class CppDomain; // Returns whether the given function is an object member function. This is true // if it's a non-static member function and not a constructor. Object member