Initialize cpp_mangle_context_ in Mangler's constructor (#5095)

This is a followup of [a
comment](https://github.com/carbon-language/carbon-lang/pull/5062/files/89e56d51858bcc18d4242d4e5c9ee0e7496d887e#r1979993815)
in #5062.

Add a mutable AST pointer to `FileContext`.

This is necessary since we use [Clang with lack of const
correctness](llvm/llvm-project#130096 (comment)).

Alternatives in Clang:
* Change `ASTUnit::getASTContext() const` to return a non-const
`ASTContext`. [Tried and was rejected upstream due to weakening const
correctness](llvm/llvm-project#130096).
* Change `createMangleContext()` to be `const`. Tried that and it seems
like it relies heavily on non const API.
* Change `MangleContext::mangleName()` to `const`. Tried that but there
are several lazy initialization and id creations happening that modify
the context. See details in
llvm/llvm-project#130613.

Alternatives in Carbon:
* Use `const_cast` on `ASTContext` when calling `createMangleContext()`.
* Make `FileContext::sem_ir_` point to a mutable `SemIR::File`.
* Change `File::cpp_ast()` to be const while keeping it return a mutable
pointer.

Part of #4666.

Author: bricknerb

toolchain/driver/compile_subcommand.cpp

@@ -598,9 +598,10 @@ auto CompilationUnit::RunLower(
     // TODO: Consider disabling instruction naming by default if we're not
     // producing textual LLVM IR.
     SemIR::InstNamer inst_namer(&*sem_ir_);
-    module_ = Lower::LowerToLLVM(
-        *llvm_context_, tree_and_subtrees_getters_for_debug_info,
-        input_filename_, *sem_ir_, &inst_namer, vlog_stream_);
+    module_ = Lower::LowerToLLVM(*llvm_context_,
+                                 tree_and_subtrees_getters_for_debug_info,
+                                 input_filename_, *sem_ir_, sem_ir_->cpp_ast(),
+                                 &inst_namer, vlog_stream_);
   });
   if (vlog_stream_) {
     CARBON_VLOG("*** llvm::Module ***\n");

toolchain/lower/file_context.cpp

@@ -31,7 +31,8 @@ FileContext::FileContext(
     std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
         tree_and_subtrees_getters_for_debug_info,
     llvm::StringRef module_name, const SemIR::File& sem_ir,
-    const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream)
+    clang::ASTUnit* cpp_ast, const SemIR::InstNamer* inst_namer,
+    llvm::raw_ostream* vlog_stream)
     : llvm_context_(&llvm_context),
       llvm_module_(std::make_unique<llvm::Module>(module_name, llvm_context)),
       di_builder_(*llvm_module_),
@@ -42,6 +43,7 @@ FileContext::FileContext(
       tree_and_subtrees_getters_for_debug_info_(
           tree_and_subtrees_getters_for_debug_info),
       sem_ir_(&sem_ir),
+      cpp_ast_(cpp_ast),
       inst_namer_(inst_namer),
       vlog_stream_(vlog_stream) {
   CARBON_CHECK(!sem_ir.has_errors(),

toolchain/lower/file_context.h

@@ -33,7 +33,8 @@ class FileContext {
       std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
           tree_and_subtrees_getters_for_debug_info,
       llvm::StringRef module_name, const SemIR::File& sem_ir,
-      const SemIR::InstNamer* inst_namer, llvm::raw_ostream* vlog_stream);
+      clang::ASTUnit* cpp_ast, const SemIR::InstNamer* inst_namer,
+      llvm::raw_ostream* vlog_stream);
 
   // Lowers the SemIR::File to LLVM IR. Should only be called once, and handles
   // the main execution loop.
@@ -91,6 +92,7 @@ class FileContext {
   auto llvm_context() -> llvm::LLVMContext& { return *llvm_context_; }
   auto llvm_module() -> llvm::Module& { return *llvm_module_; }
   auto sem_ir() -> const SemIR::File& { return *sem_ir_; }
+  auto cpp_ast() -> clang::ASTUnit* { return cpp_ast_; }
   auto inst_namer() -> const SemIR::InstNamer* { return inst_namer_; }
   auto global_variables() -> const Map<SemIR::InstId, llvm::GlobalVariable*>& {
     return global_variables_;
@@ -162,6 +164,10 @@ class FileContext {
   // The input SemIR.
   const SemIR::File* const sem_ir_;
 
+  // A mutable Clang AST is necessary for lowering since using the AST in lower
+  // modifies it.
+  clang::ASTUnit* cpp_ast_;
+
   // The instruction namer, if given.
   const SemIR::InstNamer* const inst_namer_;
 

toolchain/lower/lower.cpp

@@ -12,11 +12,11 @@ auto LowerToLLVM(llvm::LLVMContext& llvm_context,
                  std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
                      tree_and_subtrees_getters_for_debug_info,
                  llvm::StringRef module_name, const SemIR::File& sem_ir,
-                 const SemIR::InstNamer* inst_namer,
+                 clang::ASTUnit* cpp_ast, const SemIR::InstNamer* inst_namer,
                  llvm::raw_ostream* vlog_stream)
     -> std::unique_ptr<llvm::Module> {
   FileContext context(llvm_context, tree_and_subtrees_getters_for_debug_info,
-                      module_name, sem_ir, inst_namer, vlog_stream);
+                      module_name, sem_ir, cpp_ast, inst_namer, vlog_stream);
   return context.Run();
 }
 

toolchain/lower/lower.h

@@ -19,7 +19,7 @@ auto LowerToLLVM(llvm::LLVMContext& llvm_context,
                  std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
                      tree_and_subtrees_getters_for_debug_info,
                  llvm::StringRef module_name, const SemIR::File& sem_ir,
-                 const SemIR::InstNamer* inst_namer,
+                 clang::ASTUnit* cpp_ast, const SemIR::InstNamer* inst_namer,
                  llvm::raw_ostream* vlog_stream)
     -> std::unique_ptr<llvm::Module>;
 

toolchain/lower/mangler.cpp

@@ -162,13 +162,9 @@ auto Mangler::Mangle(SemIR::FunctionId function_id,
 }
 
 auto Mangler::MangleCppClang(const clang::NamedDecl* decl) -> std::string {
-  if (!cpp_mangle_context_) {
-    // We assume all declarations are from the same AST Context.
-    // TODO: Consider initializing this in the constructor. This is related to:
-    // https://github.com/carbon-language/carbon-lang/issues/4666. See
-    // https://github.com/carbon-language/carbon-lang/pull/5062/files/89e56d51858bcc18d4242d4e5c9ee0e7496d887e#r1979993815
-    cpp_mangle_context_.reset(decl->getASTContext().createMangleContext());
-  }
+  CARBON_CHECK(
+      cpp_mangle_context_,
+      "Mangling of a C++ imported declaration without a Clang `MangleContext`");
 
   RawStringOstream cpp_mangled_name;
   cpp_mangle_context_->mangleName(decl, cpp_mangled_name);

toolchain/lower/mangler.h

@@ -22,7 +22,12 @@ class Mangler {
  public:
   // Initialize a new Mangler instance for mangling entities within the
   // specified `FileContext`.
-  explicit Mangler(FileContext& file_context) : file_context_(file_context) {}
+  explicit Mangler(FileContext& file_context)
+      : file_context_(file_context),
+        cpp_mangle_context_(
+            file_context.cpp_ast()
+                ? file_context.cpp_ast()->getASTContext().createMangleContext()
+                : nullptr) {}
 
   // Produce a deterministically unique mangled name for the function specified
   // by `function_id` and `specific_id`.

toolchain/sem_ir/file.h

@@ -189,7 +189,8 @@ class File : public Printable<File> {
   }
   auto cpp_ast() -> clang::ASTUnit* { return cpp_ast_; }
   // TODO: When the AST can be created before creating `File`, initialize the
-  // pointer in the constructor and remove this function.
+  // pointer in the constructor and remove this function. This is part of
+  // https://github.com/carbon-language/carbon-lang/issues/4666
   auto set_cpp_ast(clang::ASTUnit* cpp_ast) -> void { cpp_ast_ = cpp_ast; }
   auto names() const -> NameStoreWrapper {
     return NameStoreWrapper(&identifiers());