Skip to content

Commit 31598ae

Browse files
committed
Initialize cpp_mangle_context_ in Mangler's constructor
This is a followup of [a comment](https://github.com/carbon-language/carbon-lang/pull/5062/files/89e56d51858bcc18d4242d4e5c9ee0e7496d887e#r1979993815) in carbon-language#5062. Change `File::cpp_ast()` to be `const` while keeping it returning a non-const pointer to `clang::ASTUnit`. This makes the fact we use [Clang with lack of const correctness](llvm/llvm-project#130096 (comment)) explicit. Alternatives: * Change `ASTUnit::getASTContext() const` to return a non-const `ASTContext`. [Rejected 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 Clang `MangleContext::mangleName()` to `const`. * Use `const_cast` on `ASTContext` when calling `createMangleContext()`.
1 parent b3be298 commit 31598ae

File tree

3 files changed

+12
-9
lines changed

3 files changed

+12
-9
lines changed

toolchain/lower/mangler.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,9 @@ auto Mangler::Mangle(SemIR::FunctionId function_id,
162162
}
163163

164164
auto Mangler::MangleCppClang(const clang::NamedDecl* decl) -> std::string {
165-
if (!cpp_mangle_context_) {
166-
// We assume all declarations are from the same AST Context.
167-
// TODO: Consider initializing this in the constructor. This is related to:
168-
// https://github.com/carbon-language/carbon-lang/issues/4666. See
169-
// https://github.com/carbon-language/carbon-lang/pull/5062/files/89e56d51858bcc18d4242d4e5c9ee0e7496d887e#r1979993815
170-
cpp_mangle_context_.reset(decl->getASTContext().createMangleContext());
171-
}
165+
CARBON_CHECK(
166+
cpp_mangle_context_,
167+
"Mangling of a C++ imported declaration without a Clang `MangleContext`");
172168

173169
RawStringOstream cpp_mangled_name;
174170
cpp_mangle_context_->mangleName(decl, cpp_mangled_name);

toolchain/lower/mangler.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,14 @@ class Mangler {
2222
public:
2323
// Initialize a new Mangler instance for mangling entities within the
2424
// specified `FileContext`.
25-
explicit Mangler(FileContext& file_context) : file_context_(file_context) {}
25+
explicit Mangler(FileContext& file_context)
26+
: file_context_(file_context),
27+
cpp_mangle_context_(file_context.sem_ir().cpp_ast()
28+
? file_context.sem_ir()
29+
.cpp_ast()
30+
->getASTContext()
31+
.createMangleContext()
32+
: nullptr) {}
2633

2734
// Produce a deterministically unique mangled name for the function specified
2835
// by `function_id` and `specific_id`.

toolchain/sem_ir/file.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ class File : public Printable<File> {
187187
auto import_cpps() const -> const ValueStore<ImportCppId>& {
188188
return import_cpps_;
189189
}
190-
auto cpp_ast() -> clang::ASTUnit* { return cpp_ast_; }
190+
auto cpp_ast() const -> clang::ASTUnit* { return cpp_ast_; }
191191
// TODO: When the AST can be created before creating `File`, initialize the
192192
// pointer in the constructor and remove this function.
193193
auto set_cpp_ast(clang::ASTUnit* cpp_ast) -> void { cpp_ast_ = cpp_ast; }

0 commit comments

Comments
 (0)