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 <cjdb.ns@gmail.com>

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",

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<llvm::vfs::FileSystem> fs,
     const CheckParseTreesOptions& options,
     std::shared_ptr<clang::CompilerInvocation> 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<std::unique_ptr<CppDomain>> cpp_domains;
+
   // UnitAndImports is big due to its SmallVectors, so we default to 0 on the
   // stack.
   llvm::SmallVector<UnitAndImports, 0> 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<std::shared_ptr<CppDomain>> 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();
       }
     }
   }

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.

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(

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<Parse::Tree::PackagingNames> cpp_imports;
 
   // The C++ domain for this unit.
-  std::shared_ptr<CppDomain> cpp_domain;
+  CppDomain* cpp_domain = nullptr;
 
   // The remaining number of imports which must be checked before this unit can
   // be processed.

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<clang::Parser> parser,
+CppContext::CppContext(CppDomain& domain,
                        std::unique_ptr<CppDiagnosticListener> 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());

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<clang::Parser> parser,
+  explicit CppContext(CppDomain& domain,
                       std::unique_ptr<CppDiagnosticListener> 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<clang::Parser> { 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<clang::Parser> 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.

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::CompilerInstance> clang_instance,
+                     std::unique_ptr<clang::Parser> 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

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 <memory>
+
+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::CompilerInstance> clang_instance,
+                     std::unique_ptr<clang::Parser> 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<clang::CompilerInstance> {
+    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::CompilerInstance> clang_instance_;
+  std::unique_ptr<clang::Parser> parser_;
+  clang::CodeGenerator* code_generator_ = nullptr;
+  llvm::LLVMContext* llvm_context_ = nullptr;
+};
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_CPP_DOMAIN_H_

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<llvm::vfs::FileSystem> fs,
     llvm::LLVMContext* llvm_context,
     std::shared_ptr<clang::CompilerInvocation> base_invocation)
-    -> std::shared_ptr<CppDomain> {
+    -> std::unique_ptr<CppDomain> {
   std::shared_ptr<clang::CompilerInstance> clang_instance;
   llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine> diags;
 
@@ -810,11 +811,9 @@ auto InitializeCppDomain(
   auto parser = action.TakeParser();
   CARBON_CHECK(parser);
 
-  return std::make_shared<CppDomain>(
-      CppDomain{.clang_instance = std::move(clang_instance),
-                .parser = std::move(parser),
-                .code_generator = action.code_generator(),
-                .llvm_context = llvm_context});
+  return std::make_unique<CppDomain>(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<SemIR::CppFile>(clang_instance, domain.llvm_context);
-  if (domain.code_generator) {
-    cpp_file->SetCodeGenerator(domain.code_generator);
+      std::make_unique<SemIR::CppFile>(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<CppContext>(
-      *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());
   }
 }
 

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::CompilerInstance> clang_instance;
-  std::shared_ptr<clang::Parser> 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<llvm::vfs::FileSystem> fs,
     llvm::LLVMContext* llvm_context,
     std::shared_ptr<clang::CompilerInvocation> base_invocation)
-    -> std::shared_ptr<CppDomain>;
+    -> std::unique_ptr<CppDomain>;
 
 // 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

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"

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