From 2d87bb02d914fcef874321752aa153e4f21ddc8e Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Fri, 5 Jun 2026 09:16:11 +1000 Subject: [PATCH] Use clang_decls as the source of truth for function interop mapping (#7303) Removing the clang_decl_id on SemIR::Function - using only the clang_decls map to create the association between SemIR::Function and clang::FunctionDecls. This adds an `is_external` flag to ClangDecl to indicate whether the entity originated from Carbon or was imported from another language. (I'm open to names - I guess for now we mostly use "is this from C++" to be more specific than "is this external" - eg: NameScope::is_cpp_scope) --- toolchain/check/cpp/constant.cpp | 11 ++++---- toolchain/check/cpp/generate_ast.cpp | 5 ++-- toolchain/check/cpp/import.cpp | 42 +++++++++++++++++----------- toolchain/check/cpp/operators.cpp | 4 +-- toolchain/check/cpp/thunk.cpp | 14 ++++++++-- toolchain/check/eval.cpp | 11 ++++++-- toolchain/check/import_ref.cpp | 2 +- toolchain/lower/file_context.cpp | 21 ++++++++++---- toolchain/lower/handle_call.cpp | 5 +++- toolchain/sem_ir/clang_decl.h | 4 +++ toolchain/sem_ir/function.h | 6 ---- toolchain/sem_ir/mangler.cpp | 9 ++++-- 12 files changed, 87 insertions(+), 47 deletions(-) diff --git a/toolchain/check/cpp/constant.cpp b/toolchain/check/cpp/constant.cpp index 626d24e1fa15..e560c26c8867 100644 --- a/toolchain/check/cpp/constant.cpp +++ b/toolchain/check/cpp/constant.cpp @@ -303,11 +303,12 @@ auto MaybeModifyCppThunkCallForConstEval(Context& context, SemIR::Call* call) .GetAs(thunk_callee_inst_id) .function_id); - function_decl = - cast(context.clang_decls() - .Get(thunk_callee_function.clang_decl_id) - .GetAsKey() - .decl); + function_decl = cast( + context.clang_decls() + .Get(context.clang_decls().Lookup( + thunk_callee_function.first_decl_id())) + .GetAsKey() + .decl); if (!(function_decl->isConstexpr() || function_decl->isConsteval())) { return; diff --git a/toolchain/check/cpp/generate_ast.cpp b/toolchain/check/cpp/generate_ast.cpp index 320c67e72c3e..2d885caa5ec6 100644 --- a/toolchain/check/cpp/generate_ast.cpp +++ b/toolchain/check/cpp/generate_ast.cpp @@ -448,9 +448,10 @@ auto CarbonExternalASTSource::GetOrExportFunctionToCpp( SemIR::InstId target_inst_id, SemIR::FunctionId function_id) -> clang::FunctionDecl* { const SemIR::Function& function = context_->functions().Get(function_id); - if (function.clang_decl_id.has_value()) { + auto clang_decl_id = context_->clang_decls().Lookup(function.first_decl_id()); + if (clang_decl_id.has_value()) { return cast( - context_->clang_decls().Get(function.clang_decl_id).key.decl); + context_->clang_decls().Get(clang_decl_id).key.decl); } return ExportFunctionToCpp(*context_, SemIR::LocId(target_inst_id), diff --git a/toolchain/check/cpp/import.cpp b/toolchain/check/cpp/import.cpp index aeb4b7390985..d52e3e78fd8b 100644 --- a/toolchain/check/cpp/import.cpp +++ b/toolchain/check/cpp/import.cpp @@ -149,7 +149,8 @@ auto ImportCpp(Context& context, context.clang_decls().Add( {.key = SemIR::ClangDeclKey( context.ast_context().getTranslationUnitDecl()), - .inst_id = name_scope.inst_id()}), + .inst_id = name_scope.inst_id(), + .is_imported = true}), /*is_cpp_scope=*/true); } else { name_scope.set_has_error(); @@ -524,7 +525,8 @@ static auto ImportNamespaceDecl(Context& context, context.name_scopes() .Get(result.name_scope_id) .set_clang_decl_context_id( - context.clang_decls().Add({.key = key, .inst_id = result.inst_id}), + context.clang_decls().Add( + {.key = key, .inst_id = result.inst_id, .is_imported = true}), /*is_cpp_scope=*/true); return result.inst_id; } @@ -587,8 +589,8 @@ static auto ImportTagDecl(Context& context, clang::TagDecl* clang_decl) // TODO: The caller does the same lookup. Avoid doing it twice. auto key = SemIR::ClangDeclKey(clang_decl); - auto clang_decl_id = - context.clang_decls().Add({.key = key, .inst_id = class_inst_id}); + auto clang_decl_id = context.clang_decls().Add( + {.key = key, .inst_id = class_inst_id, .is_imported = true}); // Name lookup into the Carbon class looks in the C++ class definition. auto& class_info = context.classes().Get(class_id); @@ -814,7 +816,8 @@ static auto ImportClassObjectRepr(Context& context, SemIR::ClassId class_id, // The imported SemIR::FieldDecl represents the original declaration `decl`, // which is either the field or the indirect field declaration. auto key = SemIR::ClangDeclKey::ForNonFunctionDecl(decl); - context.clang_decls().Add({.key = key, .inst_id = field_decl_id}); + context.clang_decls().Add( + {.key = key, .inst_id = field_decl_id, .is_imported = true}); // Compute the offset to the field that appears directly in the class. uint64_t offset = clang_layout.getFieldOffset( @@ -1063,13 +1066,15 @@ static auto ImportEnumConstantDecl(Context& context, context.sem_ir(), import_ir_inst_id, SemIR::IntValue{.type_id = type_id, .int_id = int_id})); context.imports().push_back(inst_id); - context.clang_decls().Add({.key = key, .inst_id = inst_id}); + context.clang_decls().Add( + {.key = key, .inst_id = inst_id, .is_imported = true}); return inst_id; } // Mark the given `key` as failed in `clang_decls`. static auto MarkFailedDecl(Context& context, SemIR::ClangDeclKey key) { - context.clang_decls().Add({.key = key, .inst_id = SemIR::ErrorInst::InstId}); + context.clang_decls().Add( + {.key = key, .inst_id = SemIR::ErrorInst::InstId, .is_imported = true}); } // Creates an integer type of the given size. @@ -1215,7 +1220,8 @@ static auto MapTagType(Context& context, const clang::TagType& type) if (auto* record_decl = dyn_cast(tag_decl)) { auto custom_type = LookupCustomRecordType(context, record_decl); if (custom_type.inst_id.has_value()) { - context.clang_decls().Add({.key = key, .inst_id = custom_type.inst_id}); + context.clang_decls().Add( + {.key = key, .inst_id = custom_type.inst_id, .is_imported = true}); return custom_type; } } @@ -1888,10 +1894,10 @@ static auto ImportFunction(Context& context, SemIR::LocId loc_id, }}); context.imports().push_back(decl_id); - context.functions().Get(function_id).clang_decl_id = - context.clang_decls().Add({.key = SemIR::ClangDeclKey::ForFunctionDecl( - clang_decl, signature_id), - .inst_id = decl_id}); + context.clang_decls().Add( + {.key = SemIR::ClangDeclKey::ForFunctionDecl(clang_decl, signature_id), + .inst_id = decl_id, + .is_imported = true}); return function_id; } @@ -2128,8 +2134,10 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id, // Register the variable so we don't create it again, and track the // corresponding declaration to use for mangling. - auto clang_decl_id = context.clang_decls().Add( - {.key = SemIR::ClangDeclKey(var_decl), .inst_id = var_storage_inst_id}); + auto clang_decl_id = + context.clang_decls().Add({.key = SemIR::ClangDeclKey(var_decl), + .inst_id = var_storage_inst_id, + .is_imported = true}); context.cpp_global_names().Add({.key = {.entity_name_id = entity_name_id}, .clang_decl_id = clang_decl_id}); @@ -2165,7 +2173,8 @@ static auto ImportTemplateDecl(Context& context, auto name_id = context.entity_names().Add( {.name_id = AddIdentifierName(context, template_decl->getName()), .parent_scope_id = GetParentNameScopeId(context, template_decl)}); - auto decl_id = context.clang_decls().Add({.key = key, .inst_id = inst_id}); + auto decl_id = context.clang_decls().Add( + {.key = key, .inst_id = inst_id, .is_imported = true}); value.type_id = GetCppTemplateNameType(context, name_id, decl_id); // Update the value with its type. @@ -2196,7 +2205,8 @@ static auto ImportDeclAfterDependencies(Context& context, SemIR::LocId loc_id, type.getAsString())); return SemIR::ErrorInst::InstId; } - context.clang_decls().Add({.key = key, .inst_id = type_inst_id}); + context.clang_decls().Add( + {.key = key, .inst_id = type_inst_id, .is_imported = true}); return type_inst_id; } if (isa(clang_decl)) { diff --git a/toolchain/check/cpp/operators.cpp b/toolchain/check/cpp/operators.cpp index eba3f59f1899..906ad696d5c9 100644 --- a/toolchain/check/cpp/operators.cpp +++ b/toolchain/check/cpp/operators.cpp @@ -732,8 +732,8 @@ static auto GetAsCppFunctionDecl(Context& context, SemIR::InstId inst_id) if (!function_type) { return nullptr; } - SemIR::ClangDeclId clang_decl_id = - context.functions().Get(function_type->function_id).clang_decl_id; + SemIR::ClangDeclId clang_decl_id = context.clang_decls().Lookup( + context.functions().Get(function_type->function_id).first_decl_id()); return clang_decl_id.has_value() ? dyn_cast( context.clang_decls().Get(clang_decl_id).key.decl) diff --git a/toolchain/check/cpp/thunk.cpp b/toolchain/check/cpp/thunk.cpp index 9cbf1419870a..aaded7295e78 100644 --- a/toolchain/check/cpp/thunk.cpp +++ b/toolchain/check/cpp/thunk.cpp @@ -245,11 +245,17 @@ struct CalleeFunctionInfo { auto IsCppThunkRequired(Context& context, const SemIR::Function& function) -> bool { - if (!function.clang_decl_id.has_value()) { + auto clang_decl_id = context.clang_decls().Lookup(function.first_decl_id()); + if (!clang_decl_id.has_value()) { + return false; + } + + const auto& decl_info = context.clang_decls().Get(clang_decl_id); + + if (!decl_info.is_imported) { return false; } - const auto& decl_info = context.clang_decls().Get(function.clang_decl_id); const auto& signature = context.clang_decl_signatures().Get(decl_info.key.signature_id); auto* decl = cast(decl_info.key.decl); @@ -646,7 +652,9 @@ static auto BuildThunkBody(CppContext& cpp_context, clang::Sema& sema, auto BuildCppThunk(Context& context, const SemIR::Function& callee_function) -> clang::FunctionDecl* { auto clang_decl_key = - context.clang_decls().Get(callee_function.clang_decl_id).key; + context.clang_decls() + .Get(context.clang_decls().Lookup(callee_function.first_decl_id())) + .key; clang::FunctionDecl* callee_function_decl = clang_decl_key.decl->getAsFunction(); CARBON_CHECK(callee_function_decl); diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index b96eaa8252e5..4cf698e80641 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -3452,9 +3452,14 @@ static auto TryEvalCall(EvalContext& outer_eval_context, SemIR::LocId loc_id, const SemIR::Function& function, SemIR::SpecificId specific_id, SemIR::InstBlockId args_id) -> SemIR::ConstantId { - if (function.clang_decl_id != SemIR::ClangDeclId::None) { - return EvalCppCall(outer_eval_context.context(), loc_id, - function.clang_decl_id, args_id); + auto clang_decl_id = outer_eval_context.sem_ir().clang_decls().Lookup( + function.first_decl_id()); + if (clang_decl_id.has_value() && outer_eval_context.sem_ir() + .clang_decls() + .Get(clang_decl_id) + .is_imported) { + return EvalCppCall(outer_eval_context.context(), loc_id, clang_decl_id, + args_id); } else if (function.body_block_ids.empty()) { // TODO: Diagnose this. return SemIR::ConstantId::NotConstant; diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 6994dec09179..1b35c6d4716a 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -3744,7 +3744,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, resolver.local_context(), SemIR::LocId(inst_id), resolver.import_ir(), name_scope.clang_decl_context_id())) { auto clang_decl_id = resolver.local_context().clang_decls().Add( - {.key = *key, .inst_id = inst_id}); + {.key = *key, .inst_id = inst_id, .is_imported = true}); local_scope.set_clang_decl_context_id(clang_decl_id, true); } } diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index 79e87d8d642f..e8dcb0d6daa7 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -347,12 +347,21 @@ auto FileContext::GetOrCreateLLVMFunction( const FunctionTypeInfo& function_type_info, SemIR::FunctionId function_id, SemIR::SpecificId specific_id) -> llvm::Function* { // If this is a C++ function, tell Clang that we referenced it. - if (auto clang_decl_id = sem_ir().functions().Get(function_id).clang_decl_id; - clang_decl_id.has_value()) { - CARBON_CHECK(!specific_id.has_value(), - "Specific functions cannot have C++ definitions"); - return HandleReferencedCppFunction( - sem_ir().clang_decls().Get(clang_decl_id).key.decl->getAsFunction()); + // The global_ctor function can't be a C++ function (but doesn't have any + // decl_id so it doesn't fall out naturally from the handling below) + if (function_id != sem_ir().global_ctor_id()) { + const auto& function = sem_ir().functions().Get(function_id); + auto clang_decl_id = + sem_ir().clang_decls().Lookup(function.first_decl_id()); + if (clang_decl_id.has_value()) { + const auto& clang_decl = sem_ir().clang_decls().Get(clang_decl_id); + if (clang_decl.is_imported) { + CARBON_CHECK(!specific_id.has_value(), + "Specific functions cannot have C++ definitions"); + return HandleReferencedCppFunction( + clang_decl.key.decl->getAsFunction()); + } + } } SemIR::Mangler m(sem_ir(), context().total_ir_count(), diff --git a/toolchain/lower/handle_call.cpp b/toolchain/lower/handle_call.cpp index e00178dac509..f4775fadf072 100644 --- a/toolchain/lower/handle_call.cpp +++ b/toolchain/lower/handle_call.cpp @@ -637,7 +637,10 @@ static auto HandleVirtualCall(FunctionContext& context, auto* pointer_type = llvm::PointerType::get(context.llvm_context(), /* address space */ 0); llvm::Value* virtual_fn; - if (function.clang_decl_id.has_value()) { + auto clang_decl_id = + context.sem_ir().clang_decls().Lookup(function.first_decl_id()); + if (clang_decl_id.has_value() && + context.sem_ir().clang_decls().Get(clang_decl_id).is_imported) { // Use absolute vtables for clang interop - the itanium vtable contains // function pointers. auto* virtual_function_pointer_address = context.builder().CreateGEP( diff --git a/toolchain/sem_ir/clang_decl.h b/toolchain/sem_ir/clang_decl.h index 07cf0c095a18..6fdc2796810a 100644 --- a/toolchain/sem_ir/clang_decl.h +++ b/toolchain/sem_ir/clang_decl.h @@ -180,6 +180,10 @@ struct ClangDecl : public Printable { // The instruction the Clang declaration is mapped to. InstId inst_id; + // True if this declaration originated from C++. False if this declaration was + // created by exporting some Carbon declaration to C++. + bool is_imported = false; + auto GetAsKey() const -> ClangDeclKey { return key; } }; diff --git a/toolchain/sem_ir/function.h b/toolchain/sem_ir/function.h index 27a8a6dcf5b0..dfd9039305c3 100644 --- a/toolchain/sem_ir/function.h +++ b/toolchain/sem_ir/function.h @@ -157,12 +157,6 @@ struct FunctionFields { // function, in lexical order. The first block is the entry block. This will // be empty for declarations that don't have a visible definition. llvm::SmallVector body_block_ids = {}; - - // If the function is imported from C++, the Clang function declaration. Used - // for mangling and inline function definition code generation. The AST is - // owned by `CompileSubcommand` so we expect it to be live from `Function` - // creation to mangling. - ClangDeclId clang_decl_id = ClangDeclId::None; }; inline constexpr FunctionFields::CallParamIndexRanges diff --git a/toolchain/sem_ir/mangler.cpp b/toolchain/sem_ir/mangler.cpp index c86df939bf87..2de04cf23422 100644 --- a/toolchain/sem_ir/mangler.cpp +++ b/toolchain/sem_ir/mangler.cpp @@ -192,8 +192,13 @@ auto Mangler::Mangle(SemIR::FunctionId function_id, } // Clang should emit C++ function declarations for us. - CARBON_CHECK(!function.clang_decl_id.has_value(), - "Shouldn't mangle C++ function"); + if (function_id != sem_ir().global_ctor_id()) { + auto clang_decl_id = + sem_ir().clang_decls().Lookup(function.first_decl_id()); + CARBON_CHECK(!clang_decl_id.has_value() || + !sem_ir().clang_decls().Get(clang_decl_id).is_imported, + "Shouldn't mangle C++ function"); + } RawStringOstream os; os << "_C";