Handle signature mismatch when a Carbon function overrides a C++ virtual function. (#7499)

When a Carbon virtual function overrides a C++ virtual function, we need to export it with the C++ signature in order for it to work as an override. Instead of mapping the C++ signature into Carbon and then back again, use the original C++ signature from the base class as the signature exported to C++. Also add documentation explaining how we use thunks in C++ interop, including in this new virtual function handling logic.
2026-09-24 22:02:23 +01:00 · 2026-07-15 18:42:03 +00:00
parent 14e8bffe24
commit 9ae73d2847
14 changed files with 635 additions and 174 deletions
@@ -99,6 +99,8 @@ auto ExportNameScopeToCpp(Context& context, SemIR::LocId loc_id,
      decl_context = namespace_decl;
    } else if (inst.Is<SemIR::ClassDecl>()) {
      // TODO: Provide a source location.
+      // TODO: This duplicates work done by ExportClassToCpp. Factor out the
+      // shared code!
      auto* record_decl = clang::CXXRecordDecl::Create(
          context.ast_context(), clang::TagTypeKind::Class, decl_context,
          clang::SourceLocation(), clang::SourceLocation(), identifier_info);
@@ -164,6 +166,7 @@ auto ExportClassToCpp(Context& context, SemIR::LocId loc_id,
    record_decl->setAccess(clang::AS_public);
  }

+  decl_context->addHiddenDecl(record_decl);
  record_decl->setHasExternalLexicalStorage();
  record_decl->setHasExternalVisibleStorage();

@@ -384,6 +387,23 @@ static auto MapToCppThunkParamType(Context& context, SemIR::TypeId type_id)
      context.ast_context().getConstType(cpp_type));
 }

+// Build FunctionInfo for an export of the given Carbon function. Exports the
+// name scope if necessary.
+static auto BuildFunctionInfo(Context& context, SemIR::LocId loc_id,
+                              SemIR::FunctionId callee_function_id)
+    -> std::optional<FunctionInfo> {
+  const SemIR::Function& callee = context.functions().Get(callee_function_id);
+
+  // Map the parent scope into the C++ AST.
+  auto* decl_context =
+      ExportNameScopeToCpp(context, loc_id, callee.parent_scope_id);
+  if (!decl_context) {
+    return std::nullopt;
+  }
+
+  return FunctionInfo(context, callee_function_id, callee, decl_context);
+}
+
 // Create a `clang::FunctionDecl` for the given Carbon function. This
 // can be used to call the Carbon function from C++. The Carbon
 // function's ABI must be compatible with C++.
@@ -459,14 +479,55 @@ static auto BuildCppFunctionDeclForCarbonFn(Context& context,
  return function_decl;
 }

-// Create the declaration of the C++ thunk.
-static auto BuildCppToCarbonThunkDecl(
-    Context& context, SemIR::LocId loc_id, const FunctionInfo& target,
-    clang::DeclarationName thunk_name,
-    llvm::ArrayRef<clang::QualType> thunk_param_types) -> clang::FunctionDecl* {
-  clang::ASTContext& ast_context = context.ast_context();
+// Returns whether the given Carbon parameter should be passed as a C++ const
+// reference.
+static auto PassAsConstRef(Context& /*context*/,
+                           const FunctionInfo::Param& param,
+                           clang::QualType cpp_type) -> bool {
+  // Use pass-by-const-ref for value parameters of array type.
+  // TODO: Should we do this for value parameters of any type that uses a
+  // pointer value representation?
+  return param.kind == ParamPatternKind::Value && cpp_type->isArrayType();
+}

-  auto clang_loc = GetCppLocation(context, loc_id);
+// Converts a Carbon parameter type to the parameter type that should be exposed
+// to C++ callers.
+static auto MapToCppParamType(Context& context, SemIR::LocId loc_id,
+                              const FunctionInfo::Param& param)
+    -> clang::QualType {
+  auto cpp_type = MapToCppType(context, param.type_id);
+  if (cpp_type.isNull()) {
+    return clang::QualType();
+  }
+  if (param.kind == Check::ParamPatternKind::Ref) {
+    cpp_type = context.ast_context().getLValueReferenceType(cpp_type);
+  } else if (PassAsConstRef(context, param, cpp_type)) {
+    cpp_type = context.ast_context().getLValueReferenceType(
+        context.ast_context().getConstType(cpp_type));
+  } else if (cpp_type->isArrayType()) {
+    // C++ doesn't support passing arrays by value.
+    context.TODO(loc_id, "by-var array parameter");
+    return clang::QualType();
+  }
+  return cpp_type;
+}
+
+// Returns the C++ function type (`clang::FunctionProtoType`) to use for a C++
+// thunk calling a Carbon function.
+static auto BuildCppToCarbonThunkFunctionType(Context& context,
+                                              SemIR::LocId loc_id,
+                                              const FunctionInfo& target)
+    -> const clang::FunctionProtoType* {
+  llvm::SmallVector<clang::QualType> thunk_param_types;
+  thunk_param_types.reserve(target.explicit_params.size());
+  for (auto param : target.explicit_params) {
+    auto cpp_type = MapToCppParamType(context, loc_id, param);
+    if (cpp_type.isNull()) {
+      context.TODO(loc_id, "failed to map C++ type to Carbon");
+      return nullptr;
+    }
+    thunk_param_types.push_back(cpp_type);
+  }

  // Get the C++ return type (this corresponds to the return type of the
  // target Carbon function).
@@ -485,17 +546,50 @@ static auto BuildCppToCarbonThunkDecl(
    }
  }

-  clang::DeclarationNameInfo name_info(thunk_name, clang_loc);
-
  auto ext_proto_info = clang::FunctionProtoType::ExtProtoInfo();
  if (target.self_param && target.self_param->kind == ParamPatternKind::Ref) {
    ext_proto_info.RefQualifier = clang::RQ_LValue;
  }
-  clang::QualType thunk_function_type = ast_context.getFunctionType(
-      cpp_return_type, thunk_param_types, ext_proto_info);
+  return context.ast_context()
+      .getFunctionType(cpp_return_type, thunk_param_types, ext_proto_info)
+      ->getAs<clang::FunctionProtoType>();
+}

-  auto* tinfo =
-      ast_context.getTrivialTypeSourceInfo(thunk_function_type, clang_loc);
+// Create the declaration of the C++ thunk.
+static auto BuildCppToCarbonThunkDecl(Context& context, SemIR::LocId loc_id,
+                                      const FunctionInfo& target,
+                                      clang::DeclarationName thunk_name)
+    -> clang::FunctionDecl* {
+  clang::ASTContext& ast_context = context.ast_context();
+
+  auto clang_loc = GetCppLocation(context, loc_id);
+
+  // If the signature was imported from C++, use that declaration to form the
+  // parameter types rather than (lossily) re-exporting the Carbon signature
+  // back to C++.
+  const clang::FunctionProtoType* thunk_function_type = nullptr;
+  if (auto thunk_id = target.function.thunk_id(); thunk_id.has_value()) {
+    const auto& thunk = context.thunks().Get(thunk_id);
+    const auto& thunk_signature = context.functions().Get(thunk.signature_id);
+    if (const auto* clang_decl =
+            context.clang_decls().Lookup(thunk_signature.first_decl_id())) {
+      thunk_function_type = cast<clang::FunctionDecl>(clang_decl->decl())
+                                ->getType()
+                                ->getAs<clang::FunctionProtoType>();
+    }
+  }
+  if (!thunk_function_type) {
+    thunk_function_type =
+        BuildCppToCarbonThunkFunctionType(context, loc_id, target);
+    if (!thunk_function_type) {
+      return nullptr;
+    }
+  }
+
+  clang::DeclarationNameInfo name_info(thunk_name, clang_loc);
+
+  auto* tinfo = ast_context.getTrivialTypeSourceInfo(
+      clang::QualType(thunk_function_type, 0), clang_loc);

  bool uses_fp_intrin = false;
  bool inline_specified = true;
@@ -506,25 +600,31 @@ static auto BuildCppToCarbonThunkDecl(
  if (auto* parent_class =
          dyn_cast<clang::CXXRecordDecl>(target.decl_context)) {
    thunk_function_decl = clang::CXXMethodDecl::Create(
-        ast_context, parent_class, clang_loc, name_info, thunk_function_type,
-        tinfo, target.GetStorageClass(), uses_fp_intrin, inline_specified,
+        ast_context, parent_class, clang_loc, name_info,
+        clang::QualType(thunk_function_type, 0), tinfo,
+        target.GetStorageClass(), uses_fp_intrin, inline_specified,
        constexpr_kind, clang_loc, trailing_requires_clause);
    // TODO: Map Carbon access to C++ access.
    thunk_function_decl->setAccess(clang::AS_public);
+    // Carbon overriders are non-virtual in C++; only the corresponding thunk is
+    // virtual.
    thunk_function_decl->setVirtualAsWritten(
        target.function.virtual_modifier !=
-        SemIR::Function::VirtualModifier::None);
+            SemIR::Function::VirtualModifier::None &&
+        target.function.virtual_modifier !=
+            SemIR::Function::VirtualModifier::Override);
+    // TODO: Call setIsPureVirtual if VirtualModifier::Abstract is present.
  } else {
    thunk_function_decl = clang::FunctionDecl::Create(
        ast_context, target.decl_context, clang_loc, name_info,
-        thunk_function_type, tinfo, clang::SC_None, uses_fp_intrin,
-        inline_specified,
+        clang::QualType(thunk_function_type, 0), tinfo, clang::SC_None,
+        uses_fp_intrin, inline_specified,
        /*hasWrittenPrototype=*/true, constexpr_kind, trailing_requires_clause);
  }
  target.decl_context->addHiddenDecl(thunk_function_decl);

  llvm::SmallVector<clang::ParmVarDecl*> param_var_decls;
-  for (auto [i, type] : llvm::enumerate(thunk_param_types)) {
+  for (auto [i, type] : llvm::enumerate(thunk_function_type->param_types())) {
    clang::ParmVarDecl* thunk_param = clang::ParmVarDecl::Create(
        ast_context, thunk_function_decl, /*StartLoc=*/clang_loc,
        /*IdLoc=*/clang_loc, /*Id=*/nullptr, type,
@@ -632,84 +732,6 @@ static auto BuildCppToCarbonThunkBody(clang::Sema& sema,
                                     clang_loc);
 }

-// Returns whether the given Carbon parameter should be passed as a C++ const
-// reference.
-static auto PassAsConstRef(Context& /*context*/,
-                           const FunctionInfo::Param& param,
-                           clang::QualType cpp_type) -> bool {
-  // Use pass-by-const-ref for value parameters of array type.
-  // TODO: Should we do this for value parameters of any type that uses a
-  // pointer value representation?
-  return param.kind == ParamPatternKind::Value && cpp_type->isArrayType();
-}
-
-// Converts a Carbon parameter type to the parameter type that should be exposed
-// to C++ callers.
-static auto MapToCppParamType(Context& context, SemIR::LocId loc_id,
-                              const FunctionInfo::Param& param)
-    -> clang::QualType {
-  auto cpp_type = MapToCppType(context, param.type_id);
-  if (cpp_type.isNull()) {
-    return clang::QualType();
-  }
-  if (param.kind == Check::ParamPatternKind::Ref) {
-    cpp_type = context.ast_context().getLValueReferenceType(cpp_type);
-  } else if (PassAsConstRef(context, param, cpp_type)) {
-    cpp_type = context.ast_context().getLValueReferenceType(
-        context.ast_context().getConstType(cpp_type));
-  } else if (cpp_type->isArrayType()) {
-    // C++ doesn't support passing arrays by value.
-    context.TODO(loc_id, "by-var array parameter");
-    return clang::QualType();
-  }
-  return cpp_type;
-}
-
-// Create a C++ thunk that calls the Carbon thunk. The C++ thunk's
-// parameter types are mapped from the parameters of the target function
-// with `MapToCppType`. (Note that the target function here is the
-// callee of the Carbon thunk.)
-static auto BuildCppToCarbonThunk(Context& context, SemIR::LocId loc_id,
-                                  const FunctionInfo& target,
-                                  llvm::StringRef thunk_name,
-                                  clang::FunctionDecl* carbon_function_decl)
-    -> clang::FunctionDecl* {
-  auto& thunk_ident = context.ast_context().Idents.get(thunk_name);
-
-  llvm::SmallVector<clang::QualType> param_types;
-  for (auto param : target.explicit_params) {
-    auto cpp_type = MapToCppParamType(context, loc_id, param);
-    if (cpp_type.isNull()) {
-      context.TODO(loc_id, "failed to map C++ type to Carbon");
-      return nullptr;
-    }
-    param_types.push_back(cpp_type);
-  }
-
-  auto* thunk_function_decl = BuildCppToCarbonThunkDecl(
-      context, loc_id, target, &thunk_ident, param_types);
-  if (!thunk_function_decl) {
-    return nullptr;
-  }
-
-  // Build the thunk function body.
-  clang::Sema& sema = context.clang_sema();
-  clang::Sema::ContextRAII context_raii(sema, thunk_function_decl);
-  // Ensure that the evaluation context is not `Unevaluated`, as that
-  // would cause code generation to fail.
-  clang::EnterExpressionEvaluationContext evaluated(
-      sema, clang::Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
-  sema.ActOnStartOfFunctionDef(nullptr, thunk_function_decl);
-  clang::StmtResult body = BuildCppToCarbonThunkBody(
-      sema, target, thunk_function_decl, carbon_function_decl);
-  sema.ActOnFinishFunctionBody(thunk_function_decl, body.get());
-  CARBON_CHECK(!body.isInvalid());
-
-  context.clang_sema().getASTConsumer().HandleTopLevelDecl(
-      clang::DeclGroupRef(thunk_function_decl));
-  return thunk_function_decl;
-}
-
 // Create a Carbon thunk that calls `callee`. The thunk's parameters are
 // all references to the callee parameter type.
 //
@@ -759,15 +781,30 @@ static auto BuildCarbonToCarbonThunk(Context& context, SemIR::LocId loc_id,
  return carbon_thunk_function_id;
 }

-// Creates a `clang::FunctionDecl` that calls the Carbon function in
-// `target`. The `extra_name` string is appended to the Carbon thunk's
-// name.
-//
-// Returns nullptr if an error occurs.
-auto ExportNonGenericFunctionToCpp(Context& context, SemIR::LocId loc_id,
-                                   const FunctionInfo& target,
-                                   std::string_view extra_name = "")
+static auto ExportNonGenericFunctionDeclToCpp(Context& context,
+                                              SemIR::LocId loc_id,
+                                              const FunctionInfo& target)
    -> clang::FunctionDecl* {
+  auto& thunk_ident = context.ast_context().Idents.get(
+      context.names().GetFormatted(target.function.name_id));
+
+  return BuildCppToCarbonThunkDecl(context, loc_id, target, &thunk_ident);
+}
+
+auto ExportVirtualFunctionDeclToCpp(Context& context, SemIR::LocId loc_id,
+                                    clang::CXXRecordDecl* parent,
+                                    SemIR::FunctionId function_id)
+    -> clang::CXXMethodDecl* {
+  FunctionInfo target(context, function_id,
+                      context.functions().Get(function_id), parent);
+  return cast_or_null<clang::CXXMethodDecl>(
+      ExportNonGenericFunctionDeclToCpp(context, loc_id, target));
+}
+
+static auto BuildCppToCarbonThunk(Context& context, SemIR::LocId loc_id,
+                                  const FunctionInfo& target,
+                                  clang::FunctionDecl* thunk_function_decl,
+                                  std::string_view extra_name) -> void {
  // Create a Carbon thunk that calls the callee. The thunk's parameters
  // are all references so that the ABI is compatible with C++ callers.
  auto carbon_thunk_function_id =
@@ -777,14 +814,53 @@ auto ExportNonGenericFunctionToCpp(Context& context, SemIR::LocId loc_id,
  auto* carbon_function_decl = BuildCppFunctionDeclForCarbonFn(
      context, loc_id, carbon_thunk_function_id);
  if (!carbon_function_decl) {
+    return;
+  }
+
+  // Build the thunk function body.
+  clang::Sema& sema = context.clang_sema();
+  clang::Sema::ContextRAII context_raii(sema, thunk_function_decl);
+  // Ensure that the evaluation context is not `Unevaluated`, as that
+  // would cause code generation to fail.
+  clang::EnterExpressionEvaluationContext evaluated(
+      sema, clang::Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
+  sema.ActOnStartOfFunctionDef(nullptr, thunk_function_decl);
+  clang::StmtResult body = BuildCppToCarbonThunkBody(
+      sema, target, thunk_function_decl, carbon_function_decl);
+  sema.ActOnFinishFunctionBody(thunk_function_decl, body.get());
+  CARBON_CHECK(!body.isInvalid());
+
+  context.clang_sema().getASTConsumer().HandleTopLevelDecl(
+      clang::DeclGroupRef(thunk_function_decl));
+}
+
+auto DefineExportedVirtualFunction(Context& context, SemIR::LocId loc_id,
+                                   SemIR::FunctionId callee_function_id,
+                                   clang::CXXMethodDecl* method_decl) -> void {
+  const SemIR::Function& callee = context.functions().Get(callee_function_id);
+  FunctionInfo target_function_info(context, callee_function_id, callee,
+                                    method_decl->getDeclContext());
+  BuildCppToCarbonThunk(context, loc_id, target_function_info, method_decl, "");
+}
+
+// Creates a `clang::FunctionDecl` that calls the Carbon function in
+// `target`. The `extra_name` string is appended to the Carbon thunk's
+// name.
+//
+// Returns nullptr if an error occurs.
+auto ExportNonGenericFunctionToCpp(Context& context, SemIR::LocId loc_id,
+                                   const FunctionInfo& target,
+                                   std::string_view extra_name = "")
+    -> clang::FunctionDecl* {
+  auto* thunk_function_decl =
+      ExportNonGenericFunctionDeclToCpp(context, loc_id, target);
+  if (!thunk_function_decl) {
    return nullptr;
  }

-  // Create a C++ thunk that calls the Carbon thunk.
-  return BuildCppToCarbonThunk(
-      context, loc_id, target,
-      context.names().GetFormatted(target.function.name_id),
-      carbon_function_decl);
+  BuildCppToCarbonThunk(context, loc_id, target, thunk_function_decl,
+                        extra_name);
+  return thunk_function_decl;
 }

 auto ExportFunctionSpecializationToCpp(
@@ -946,23 +1022,16 @@ static auto ExportGenericFunctionToCpp(Context& context, SemIR::LocId loc_id,
 auto ExportFunctionToCpp(Context& context, SemIR::LocId loc_id,
                         SemIR::FunctionId callee_function_id)
    -> clang::NamedDecl* {
-  const SemIR::Function& callee = context.functions().Get(callee_function_id);
-
-  // Map the parent scope into the C++ AST.
-  auto* decl_context =
-      ExportNameScopeToCpp(context, loc_id, callee.parent_scope_id);
-  if (!decl_context) {
+  auto target = BuildFunctionInfo(context, loc_id, callee_function_id);
+  if (!target) {
    return nullptr;
  }

-  FunctionInfo target_function_info(context, callee_function_id, callee,
-                                    decl_context);
-
-  if (callee.generic_id.has_value()) {
-    return ExportGenericFunctionToCpp(context, loc_id, target_function_info);
+  if (target->function.generic_id.has_value()) {
+    return ExportGenericFunctionToCpp(context, loc_id, *target);
  }

-  return ExportNonGenericFunctionToCpp(context, loc_id, target_function_info);
+  return ExportNonGenericFunctionToCpp(context, loc_id, *target);
 }

 // Returns whether the given class has any abstract methods.