Share more function logic between custom/thunk/C++ functions. (#6690)

I need to do more work on the custom witness functions. This is trying
to make it easier to see the differences between the approaches before I
resume work there (e.g. this helps flag a possible reason I was having
trouble switching definitions when it came to generics, I think those
are mishandled right now).

This changes the thunk test because it was doing
`CheckFunctionDefinitionSignature` in a different order from
`handle_function.cpp`, and I think `handle_function.cpp` is more
canonical here (changing that affects tests with defined functions).

Assisted-by: Google Antigravity with Gemini 3 Flash

toolchain/check/cpp/import.cpp

@@ -1443,27 +1443,17 @@ static auto ImportFunction(Context& context, SemIR::LocId loc_id,
                            clang::FunctionDecl* clang_decl,
                            SemIR::ClangDeclKey::Signature signature)
     -> std::optional<SemIR::FunctionId> {
-  context.scope_stack().PushForDeclName();
-  context.inst_block_stack().Push();
-  context.pattern_block_stack().Push();
+  StartFunctionSignature(context);
 
   auto function_params_insts =
       CreateFunctionSignatureInsts(context, loc_id, clang_decl, signature);
 
-  auto pattern_block_id = context.pattern_block_stack().Pop();
-  auto decl_block_id = context.inst_block_stack().Pop();
-  context.scope_stack().Pop();
+  auto [pattern_block_id, decl_block_id] = FinishFunctionSignature(context);
 
   if (!function_params_insts.has_value()) {
     return std::nullopt;
   }
 
-  auto function_decl = SemIR::FunctionDecl{
-      SemIR::TypeId::None, SemIR::FunctionId::None, decl_block_id};
-  auto decl_id = AddPlaceholderImportedInstInNoBlock(
-      context,
-      MakeImportedLocIdAndInst(context, import_ir_inst_id, function_decl));
-
   auto virtual_modifier = SemIR::Function::VirtualModifier::None;
   int32_t virtual_index = -1;
   if (auto* method_decl = dyn_cast<clang::CXXMethodDecl>(clang_decl)) {
@@ -1479,39 +1469,47 @@ static auto ImportFunction(Context& context, SemIR::LocId loc_id,
                           ->getMethodVTableIndex(method_decl);
     }
   }
-  auto function_info = SemIR::Function{
-      {.name_id = GetFunctionName(context, clang_decl),
-       .parent_scope_id = GetParentNameScopeId(context, clang_decl),
-       .generic_id = SemIR::GenericId::None,
-       .first_param_node_id = Parse::NodeId::None,
-       .last_param_node_id = Parse::NodeId::None,
-       .pattern_block_id = pattern_block_id,
-       .implicit_param_patterns_id =
-           function_params_insts->implicit_param_patterns_id,
-       .param_patterns_id = function_params_insts->param_patterns_id,
-       .is_extern = false,
-       .extern_library_id = SemIR::LibraryNameId::None,
-       .non_owning_decl_id = SemIR::InstId::None,
-       .first_owning_decl_id = decl_id,
-       .definition_id = SemIR::InstId::None},
-      {.call_param_patterns_id = function_params_insts->call_param_patterns_id,
-       .call_params_id = function_params_insts->call_params_id,
-       .return_type_inst_id = function_params_insts->return_type_inst_id,
-       .return_form_inst_id = function_params_insts->return_form_inst_id,
-       .return_patterns_id = function_params_insts->return_patterns_id,
-       .virtual_modifier = virtual_modifier,
-       .virtual_index = virtual_index,
-       .self_param_id = FindSelfPattern(
-           context, function_params_insts->implicit_param_patterns_id),
-       .clang_decl_id = context.clang_decls().Add(
-           {.key = SemIR::ClangDeclKey::ForFunctionDecl(clang_decl, signature),
-            .inst_id = decl_id})}};
-
-  function_decl.function_id = context.functions().Add(function_info);
-  function_decl.type_id = GetFunctionType(context, function_decl.function_id,
-                                          SemIR::SpecificId::None);
-  ReplaceInstBeforeConstantUse(context, decl_id, function_decl);
-  return function_decl.function_id;
+
+  auto [decl_id, function_id] = MakeFunctionDecl(
+      context, import_ir_inst_id, decl_block_id, /*build_generic=*/false,
+      /*is_definition=*/false,
+      SemIR::Function{
+          {
+              .name_id = GetFunctionName(context, clang_decl),
+              .parent_scope_id = GetParentNameScopeId(context, clang_decl),
+              .generic_id = SemIR::GenericId::None,
+              .first_param_node_id = Parse::NodeId::None,
+              .last_param_node_id = Parse::NodeId::None,
+              .pattern_block_id = pattern_block_id,
+              .implicit_param_patterns_id =
+                  function_params_insts->implicit_param_patterns_id,
+              .param_patterns_id = function_params_insts->param_patterns_id,
+              .is_extern = false,
+              .extern_library_id = SemIR::LibraryNameId::None,
+              .non_owning_decl_id = SemIR::InstId::None,
+              // Set by `MakeFunctionDecl`.
+              .first_owning_decl_id = SemIR::InstId::None,
+          },
+          {
+              .call_param_patterns_id =
+                  function_params_insts->call_param_patterns_id,
+              .call_params_id = function_params_insts->call_params_id,
+              .return_type_inst_id = function_params_insts->return_type_inst_id,
+              .return_form_inst_id = function_params_insts->return_form_inst_id,
+              .return_patterns_id = function_params_insts->return_patterns_id,
+              .virtual_modifier = virtual_modifier,
+              .virtual_index = virtual_index,
+              .self_param_id = FindSelfPattern(
+                  context, function_params_insts->implicit_param_patterns_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),
+           .inst_id = decl_id});
+
+  return function_id;
 }
 
 // Imports a C++ function, returning a corresponding Carbon function.

toolchain/check/function.cpp

@@ -7,10 +7,12 @@
 #include "common/find.h"
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/convert.h"
+#include "toolchain/check/generic.h"
 #include "toolchain/check/inst.h"
 #include "toolchain/check/merge.h"
 #include "toolchain/check/pattern.h"
 #include "toolchain/check/pattern_match.h"
+#include "toolchain/check/scope_stack.h"
 #include "toolchain/check/type.h"
 #include "toolchain/check/type_completion.h"
 #include "toolchain/sem_ir/builtin_function_kind.h"
@@ -90,10 +92,7 @@ auto MakeBuiltinFunction(Context& context, SemIR::LocId loc_id,
                          SemIR::NameScopeId name_scope_id,
                          SemIR::NameId name_id,
                          BuiltinFunctionSignature signature) -> SemIR::InstId {
-  // TODO: Refactor with function construction in thunk.cpp and cpp/import.cpp.
-  context.scope_stack().PushForDeclName();
-  context.inst_block_stack().Push();
-  context.pattern_block_stack().Push();
+  StartFunctionSignature(context);
 
   // Build and add a `[ref self: Self]` parameter if needed.
   auto implicit_param_patterns_id = SemIR::InstBlockId::None;
@@ -150,53 +149,47 @@ auto MakeBuiltinFunction(Context& context, SemIR::LocId loc_id,
                          return_patterns_id);
 
   context.full_pattern_stack().PopFullPattern();
-  auto pattern_block_id = context.pattern_block_stack().Pop();
-  auto decl_block_id = context.inst_block_stack().Pop();
-  context.scope_stack().Pop();
+  auto [pattern_block_id, decl_block_id] = FinishFunctionSignature(context);
 
   // Add the function declaration.
-  SemIR::FunctionDecl function_decl = {.type_id = SemIR::TypeId::None,
-                                       .function_id = SemIR::FunctionId::None,
-                                       .decl_block_id = decl_block_id};
-  auto decl_id = AddPlaceholderInstInNoBlock(
-      context, SemIR::LocIdAndInst::UncheckedLoc(loc_id, function_decl));
-
-  // Build the function entity.
-  auto function = SemIR::Function{
-      {
-          .name_id = name_id,
-          .parent_scope_id = name_scope_id,
-          .generic_id = SemIR::GenericId::None,
-          .first_param_node_id = Parse::NodeId::None,
-          .last_param_node_id = Parse::NodeId::None,
-          .pattern_block_id = pattern_block_id,
-          .implicit_param_patterns_id = implicit_param_patterns_id,
-          .param_patterns_id = param_patterns_id,
-          .is_extern = false,
-          .extern_library_id = SemIR::LibraryNameId::None,
-          .non_owning_decl_id = SemIR::InstId::None,
-          .first_owning_decl_id = decl_id,
-          .definition_id = decl_id,
-      },
-      {
-          .call_param_patterns_id = call_param_patterns_id,
-          .call_params_id = call_params_id,
-          .return_type_inst_id = return_form.type_component_id,
-          .return_form_inst_id = return_form.form_inst_id,
-          .return_patterns_id = return_patterns_id,
-          .self_param_id = self_param_id,
-      }};
-  CARBON_CHECK(IsValidBuiltinDeclaration(context, function, builtin_kind));
-  function.SetBuiltinFunction(builtin_kind);
-  function_decl.function_id = context.functions().Add(function);
-  function_decl.type_id = GetFunctionType(context, function_decl.function_id,
-                                          SemIR::SpecificId::None);
-  ReplaceInstBeforeConstantUse(context, decl_id, function_decl);
+  // TODO: This should probably handle generics.
+  auto [decl_id, function_id] = MakeFunctionDecl(
+      context, loc_id, decl_block_id, /*build_generic=*/false,
+      /*is_definition=*/true,
+      SemIR::Function{
+          {
+              .name_id = name_id,
+              .parent_scope_id = name_scope_id,
+              .generic_id = SemIR::GenericId::None,
+              .first_param_node_id = Parse::NodeId::None,
+              .last_param_node_id = Parse::NodeId::None,
+              .pattern_block_id = pattern_block_id,
+              .implicit_param_patterns_id = implicit_param_patterns_id,
+              .param_patterns_id = param_patterns_id,
+              .is_extern = false,
+              .extern_library_id = SemIR::LibraryNameId::None,
+              .non_owning_decl_id = SemIR::InstId::None,
+              // Set by `MakeFunctionDecl`.
+              .first_owning_decl_id = SemIR::InstId::None,
+          },
+          {
+              .call_param_patterns_id = call_param_patterns_id,
+              .call_params_id = call_params_id,
+              .return_type_inst_id = return_form.type_component_id,
+              .return_form_inst_id = return_form.form_inst_id,
+              .return_patterns_id = return_patterns_id,
+              .self_param_id = self_param_id,
+          }});
   // Add the builtin to the imports block so that it appears in the formatted
   // IR.
   // TODO: Find a better way to handle this. Ideally we should stop using this
   // function entirely and declare builtins in the prelude.
   context.imports().push_back(decl_id);
+
+  auto& function = context.functions().Get(function_id);
+  CARBON_CHECK(IsValidBuiltinDeclaration(context, function, builtin_kind));
+  function.SetBuiltinFunction(builtin_kind);
+
   return decl_id;
 }
 
@@ -356,4 +349,71 @@ auto CheckFunctionDefinitionSignature(Context& context,
   }
 }
 
+auto StartFunctionSignature(Context& context) -> void {
+  context.scope_stack().PushForDeclName();
+  context.inst_block_stack().Push();
+  context.pattern_block_stack().Push();
+}
+
+auto FinishFunctionSignature(Context& context)
+    -> FinishFunctionSignatureResult {
+  auto pattern_block_id = context.pattern_block_stack().Pop();
+  auto decl_block_id = context.inst_block_stack().Pop();
+  context.scope_stack().Pop();
+  return {.pattern_block_id = pattern_block_id, .decl_block_id = decl_block_id};
+}
+
+auto MakeFunctionDecl(Context& context, SemIR::LocId loc_id,
+                      SemIR::InstBlockId decl_block_id, bool build_generic,
+                      bool is_definition, SemIR::Function function)
+    -> std::pair<SemIR::InstId, SemIR::FunctionId> {
+  CARBON_CHECK(!function.first_owning_decl_id.has_value());
+
+  SemIR::FunctionDecl function_decl = {SemIR::TypeId::None,
+                                       SemIR::FunctionId::None, decl_block_id};
+  auto decl_id = AddPlaceholderInstInNoBlock(
+      context, SemIR::LocIdAndInst::UncheckedLoc(loc_id, function_decl));
+  function.first_owning_decl_id = decl_id;
+  if (is_definition) {
+    function.definition_id = decl_id;
+  }
+
+  if (build_generic) {
+    function.generic_id = BuildGenericDecl(context, decl_id);
+  }
+
+  // Create the `Function` object.
+  function_decl.function_id = context.functions().Add(std::move(function));
+  function_decl.type_id =
+      GetFunctionType(context, function_decl.function_id,
+                      build_generic ? context.scope_stack().PeekSpecificId()
+                                    : SemIR::SpecificId::None);
+  ReplaceInstBeforeConstantUse(context, decl_id, function_decl);
+  return {decl_id, function_decl.function_id};
+}
+
+auto StartFunctionDefinition(Context& context, SemIR::InstId decl_id,
+                             SemIR::FunctionId function_id) -> void {
+  // Create the function scope and the entry block.
+  context.scope_stack().PushForFunctionBody(decl_id);
+  context.inst_block_stack().Push();
+  context.region_stack().PushRegion(context.inst_block_stack().PeekOrAdd());
+  StartGenericDefinition(context,
+                         context.functions().Get(function_id).generic_id);
+
+  CheckFunctionDefinitionSignature(context, function_id);
+}
+
+auto FinishFunctionDefinition(Context& context, SemIR::FunctionId function_id)
+    -> void {
+  context.inst_block_stack().Pop();
+  context.scope_stack().Pop();
+
+  auto& function = context.functions().Get(function_id);
+  function.body_block_ids = context.region_stack().PopRegion();
+
+  // If this is a generic function, collect information about the definition.
+  FinishGenericDefinition(context, function.generic_id);
+}
+
 }  // namespace Carbon::Check

toolchain/check/function.h

@@ -101,6 +101,37 @@ auto CheckFunctionReturnPatternType(Context& context, SemIR::LocId loc_id,
 auto CheckFunctionDefinitionSignature(Context& context,
                                       SemIR::FunctionId function_id) -> void;
 
+// Prepares for a function signature. Handles necessary stack setup. This is
+// used for generated functions/thunks, not user-declared functions.
+auto StartFunctionSignature(Context& context) -> void;
+
+// Results for `FinishFunctionSignature`.
+struct FinishFunctionSignatureResult {
+  SemIR::InstBlockId pattern_block_id;
+  SemIR::InstBlockId decl_block_id;
+};
+
+// Finishes signatures started by `StartFunctionSignature`.
+auto FinishFunctionSignature(Context& context) -> FinishFunctionSignatureResult;
+
+// Creates a function object for the given function declaration. The caller must
+// add the returned `decl_id` to a block (typically the current block or
+// imports).
+auto MakeFunctionDecl(Context& context, SemIR::LocId loc_id,
+                      SemIR::InstBlockId decl_block_id, bool build_generic,
+                      bool is_definition, SemIR::Function function)
+    -> std::pair<SemIR::InstId, SemIR::FunctionId>;
+
+// Starts a function definition. Handles necessary stack setup, creating the
+// function scope and entry block, and definition validation. This is used for
+// both generated functions/thunks and user-declared functions.
+auto StartFunctionDefinition(Context& context, SemIR::InstId decl_id,
+                             SemIR::FunctionId function_id) -> void;
+
+// Finishes definitions started by `StartFunctionDefinition`.
+auto FinishFunctionDefinition(Context& context, SemIR::FunctionId function_id)
+    -> void;
+
 }  // namespace Carbon::Check
 
 #endif  // CARBON_TOOLCHAIN_CHECK_FUNCTION_H_

toolchain/check/handle_function.cpp

@@ -506,15 +506,7 @@ auto HandleParseNode(Context& context, Parse::FunctionDeclId node_id) -> bool {
 static auto HandleFunctionDefinitionAfterSignature(
     Context& context, Parse::FunctionDefinitionStartId node_id,
     SemIR::FunctionId function_id, SemIR::InstId decl_id) -> void {
-  // Create the function scope and the entry block.
-  context.scope_stack().PushForFunctionBody(decl_id);
-  context.inst_block_stack().Push();
-  context.region_stack().PushRegion(context.inst_block_stack().PeekOrAdd());
-  StartGenericDefinition(context,
-                         context.functions().Get(function_id).generic_id);
-
-  CheckFunctionDefinitionSignature(context, function_id);
-
+  StartFunctionDefinition(context, decl_id, function_id);
   context.node_stack().Push(node_id, function_id);
 }
 
@@ -566,16 +558,9 @@ auto HandleParseNode(Context& context, Parse::FunctionDefinitionId node_id)
     }
   }
 
-  context.inst_block_stack().Pop();
-  context.scope_stack().Pop();
+  FinishFunctionDefinition(context, function_id);
   context.decl_name_stack().PopScope();
 
-  auto& function = context.functions().Get(function_id);
-  function.body_block_ids = context.region_stack().PopRegion();
-
-  // If this is a generic function, collect information about the definition.
-  FinishGenericDefinition(context, function.generic_id);
-
   return true;
 }
 

toolchain/check/testdata/impl/impl_thunk.carbon

@@ -935,11 +935,11 @@ impl () as I({}) {
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:   %.loc10_34.3: require_specific_def_type = require_specific_def @ptr.as.Copy.impl(%T.loc5_8.1) [symbolic = %.loc10_34.3 (constants.%.2f2)]
 // CHECK:STDOUT:   %Copy.lookup_impl_witness: <witness> = lookup_impl_witness %ptr, @Copy [symbolic = %Copy.lookup_impl_witness (constants.%Copy.lookup_impl_witness.2e6)]
 // CHECK:STDOUT:   %Copy.facet.loc10_34.3: %Copy.type = facet_value %ptr, (%Copy.lookup_impl_witness) [symbolic = %Copy.facet.loc10_34.3 (constants.%Copy.facet)]
 // CHECK:STDOUT:   %empty_tuple.type.as.I.impl.F.specific_fn.loc10_34.2: <specific function> = specific_function constants.%empty_tuple.type.as.I.impl.F.8be29b.1, @empty_tuple.type.as.I.impl.F.loc10_34.1(%Copy.facet.loc10_34.3) [symbolic = %empty_tuple.type.as.I.impl.F.specific_fn.loc10_34.2 (constants.%empty_tuple.type.as.I.impl.F.specific_fn)]
-// CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:
 // CHECK:STDOUT:   fn(%x.param: @empty_tuple.type.as.I.impl.F.loc10_34.2.%ptr (%ptr.e8f)) -> out %return.param: @empty_tuple.type.as.I.impl.F.loc10_34.2.%ptr (%ptr.e8f) [thunk @empty_tuple.type.as.I.impl.%empty_tuple.type.as.I.impl.F.decl.loc10_34.1] {
 // CHECK:STDOUT:   !entry:
@@ -1107,11 +1107,11 @@ impl () as I({}) {
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:
 // CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:   %.loc10_44.3: require_specific_def_type = require_specific_def @ptr.as.Copy.impl(%T.loc5_20.1) [symbolic = %.loc10_44.3 (constants.%.2f2)]
 // CHECK:STDOUT:   %Copy.lookup_impl_witness: <witness> = lookup_impl_witness %ptr, @Copy [symbolic = %Copy.lookup_impl_witness (constants.%Copy.lookup_impl_witness.2e6)]
 // CHECK:STDOUT:   %Copy.facet.loc10_44.3: %Copy.type = facet_value %ptr, (%Copy.lookup_impl_witness) [symbolic = %Copy.facet.loc10_44.3 (constants.%Copy.facet)]
 // CHECK:STDOUT:   %empty_tuple.type.as.I.impl.F.specific_fn.loc10_44.2: <specific function> = specific_function constants.%empty_tuple.type.as.I.impl.F.8be29b.1, @empty_tuple.type.as.I.impl.F.loc10_44.1(%Copy.facet.loc10_44.3) [symbolic = %empty_tuple.type.as.I.impl.F.specific_fn.loc10_44.2 (constants.%empty_tuple.type.as.I.impl.F.specific_fn)]
-// CHECK:STDOUT:   <elided>
 // CHECK:STDOUT:
 // CHECK:STDOUT:   fn(%self.param: %empty_tuple.type, %x.param: @empty_tuple.type.as.I.impl.F.loc10_44.2.%ptr (%ptr.e8f)) -> out %return.param: @empty_tuple.type.as.I.impl.F.loc10_44.2.%ptr (%ptr.e8f) [thunk @empty_tuple.type.as.I.impl.%empty_tuple.type.as.I.impl.F.decl.loc10_44.1] {
 // CHECK:STDOUT:   !entry:

toolchain/check/thunk.cpp

@@ -200,41 +200,39 @@ static auto CloneFunctionDecl(Context& context, SemIR::LocId loc_id,
   auto decl_block_id = context.inst_block_stack().Pop();
 
   // Create the `FunctionDecl` instruction.
-  SemIR::FunctionDecl function_decl = {SemIR::TypeId::None,
-                                       SemIR::FunctionId::None, decl_block_id};
-  auto decl_id = AddPlaceholderInst(
-      context, SemIR::LocIdAndInst::UncheckedLoc(loc_id, function_decl));
-  auto generic_id = BuildGenericDecl(context, decl_id);
-
-  // Create the `Function` object.
   auto& callee = context.functions().Get(callee_id);
-  function_decl.function_id = context.functions().Add(
-      SemIR::Function{{.name_id = signature.name_id,
-                       .parent_scope_id = callee.parent_scope_id,
-                       .generic_id = generic_id,
-                       .first_param_node_id = signature.first_param_node_id,
-                       .last_param_node_id = signature.last_param_node_id,
-                       .pattern_block_id = pattern_block_id,
-                       .implicit_param_patterns_id = implicit_param_patterns_id,
-                       .param_patterns_id = param_patterns_id,
-                       .is_extern = false,
-                       .extern_library_id = SemIR::LibraryNameId::None,
-                       .non_owning_decl_id = SemIR::InstId::None,
-                       .first_owning_decl_id = decl_id,
-                       .definition_id = decl_id},
-                      {.call_param_patterns_id = call_param_patterns_id,
-                       .call_params_id = call_params_id,
-                       .return_type_inst_id = return_type_inst_id,
-                       .return_form_inst_id = return_form_inst_id,
-                       .return_patterns_id = return_patterns_id,
-                       .virtual_modifier = callee.virtual_modifier,
-                       .virtual_index = callee.virtual_index,
-                       .self_param_id = self_param_id}});
-  function_decl.type_id =
-      GetFunctionType(context, function_decl.function_id,
-                      context.scope_stack().PeekSpecificId());
-  ReplaceInstBeforeConstantUse(context, decl_id, function_decl);
-  return {function_decl.function_id, decl_id};
+  auto [decl_id, function_id] = MakeFunctionDecl(
+      context, loc_id, decl_block_id, /*build_generic=*/true,
+      /*is_definition=*/true,
+      SemIR::Function{
+          {
+              .name_id = signature.name_id,
+              .parent_scope_id = callee.parent_scope_id,
+              // Set by `MakeFunctionDecl`.
+              .generic_id = SemIR::GenericId::None,
+              .first_param_node_id = signature.first_param_node_id,
+              .last_param_node_id = signature.last_param_node_id,
+              .pattern_block_id = pattern_block_id,
+              .implicit_param_patterns_id = implicit_param_patterns_id,
+              .param_patterns_id = param_patterns_id,
+              .is_extern = false,
+              .extern_library_id = SemIR::LibraryNameId::None,
+              .non_owning_decl_id = SemIR::InstId::None,
+              // Set by `MakeFunctionDecl`.
+              .first_owning_decl_id = SemIR::InstId::None,
+          },
+          {
+              .call_param_patterns_id = call_param_patterns_id,
+              .call_params_id = call_params_id,
+              .return_type_inst_id = return_type_inst_id,
+              .return_form_inst_id = return_form_inst_id,
+              .return_patterns_id = return_patterns_id,
+              .virtual_modifier = callee.virtual_modifier,
+              .virtual_index = callee.virtual_index,
+              .self_param_id = self_param_id,
+          }});
+  context.inst_block_stack().AddInstId(decl_id);
+  return {function_id, decl_id};
 }
 
 static auto HasDeclaredReturnType(Context& context,
@@ -351,17 +349,9 @@ static auto BuildThunkDefinition(Context& context,
           builder.Note(callee_id, ThunkCallee);
         });
 
-    CheckFunctionDefinitionSignature(context, function_id);
+    StartFunctionDefinition(context, thunk_id, function_id);
   }
 
-  // TODO: This duplicates much of the handling for FunctionDefinitionStart and
-  // FunctionDefinition parse nodes. Consider refactoring.
-  context.scope_stack().PushForFunctionBody(thunk_id);
-  context.inst_block_stack().Push();
-  context.region_stack().PushRegion(context.inst_block_stack().PeekOrAdd());
-  StartGenericDefinition(context,
-                         context.functions().Get(function_id).generic_id);
-
   // The checks below produce diagnostics pointing at the callee, so also note
   // the signature.
   Diagnostics::AnnotationScope annot_scope(
@@ -381,12 +371,7 @@ static auto BuildThunkDefinition(Context& context,
     BuildReturnWithNoExpr(context, SemIR::LocId(callee_id));
   }
 
-  context.inst_block_stack().Pop();
-  context.scope_stack().Pop();
-
-  auto& function = context.functions().Get(function_id);
-  function.body_block_ids = context.region_stack().PopRegion();
-  FinishGenericDefinition(context, function.generic_id);
+  FinishFunctionDefinition(context, function_id);
 }
 
 auto BuildThunkDefinition(Context& context,