Refactor function return type representation

toolchain/check/call.cpp

@@ -221,7 +221,7 @@ static auto CheckCalleeFunctionReturnType(Context& context, SemIR::LocId loc_id,
       &context.emitter(), [&](auto& builder) {
         CARBON_DIAGNOSTIC(IncompleteReturnTypeHere, Note,
                           "return type declared here");
-        builder.Note(function.return_slot_pattern_id, IncompleteReturnTypeHere);
+        builder.Note(function.return_type_inst_id, IncompleteReturnTypeHere);
       });
   return CheckFunctionReturnType(context, loc_id, function, callee_specific_id);
 }

toolchain/check/context.cpp

@@ -72,6 +72,7 @@ auto Context::VerifyOnFinish() const -> void {
   vtable_stack_.VerifyOnFinish();
   region_stack_.VerifyOnFinish();
   CARBON_CHECK(impl_lookup_stack_.empty());
+  CARBON_CHECK(return_type_inst_id_ == std::nullopt);
 
 #ifndef NDEBUG
   if (auto verify = sem_ir_->Verify(); !verify.ok()) {

toolchain/check/context.h

@@ -256,6 +256,25 @@ class Context {
     return rewrites_stack_;
   }
 
+  // Pushes inst_id onto the stack of return type declarations for in-progress
+  // function declarations.
+  //
+  // Note: the "stack" currently can only have one element, but that restriction
+  // can be relaxed if it becomes possible to have multiple pending return type
+  // declarations.
+  auto PushReturnTypeInstId(SemIR::TypeInstId inst_id) -> void {
+    CARBON_CHECK(return_type_inst_id_ == std::nullopt,
+                 "TODO: make return_type_inst_id_ a stack if necessary");
+    return_type_inst_id_ = inst_id;
+  }
+
+  // Pops a TypeInstId off the stack of return type declarations for in-progress
+  // function declarations.
+  auto PopReturnTypeInstId() -> SemIR::TypeInstId {
+    CARBON_CHECK(return_type_inst_id_ != std::nullopt);
+    return *std::exchange(return_type_inst_id_, std::nullopt);
+  }
+
   // --------------------------------------------------------------------------
   // Directly expose SemIR::File data accessors for brevity in calls.
   // --------------------------------------------------------------------------
@@ -492,6 +511,9 @@ class Context {
   // value on the RHS. Used during checking of a `where` expression to allow
   // constraints to access values from earlier constraints.
   llvm::SmallVector<Map<SemIR::ConstantId, SemIR::InstId>> rewrites_stack_;
+
+  // Declared return type for the in-progress function declaration, if any.
+  std::optional<SemIR::TypeInstId> return_type_inst_id_;
 };
 
 }  // namespace Carbon::Check

toolchain/check/convert.cpp

@@ -1811,7 +1811,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
     -> SemIR::InstBlockId {
   auto param_patterns =
       context.inst_blocks().GetOrEmpty(callee.param_patterns_id);
-  auto return_slot_pattern_id = callee.return_slot_pattern_id;
+  auto return_patterns_id = callee.return_patterns_id;
 
   // The caller should have ensured this callee has the right arity.
   CARBON_CHECK(arg_refs.size() == param_patterns.size());
@@ -1828,7 +1828,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
   }
 
   return CallerPatternMatch(context, callee_specific_id, callee.self_param_id,
-                            callee.param_patterns_id, return_slot_pattern_id,
+                            callee.param_patterns_id, return_patterns_id,
                             self_id, arg_refs, return_slot_arg_id);
 }
 

toolchain/check/cpp/import.cpp

@@ -1220,6 +1220,9 @@ static auto MapTagType(Context& context, const clang::TagType& type)
   SemIR::TypeInstId record_type_inst_id =
       context.types().GetAsTypeInstId(tag_inst_id);
   return {
+      // TODO: inst_id's location should be the location of the usage, not
+      // the location of the type definition. Possibly we should synthesize a
+      // NameRef inst, to match how this would work in Carbon code.
       .inst_id = record_type_inst_id,
       .type_id = context.types().GetTypeIdForTypeInstId(record_type_inst_id)};
 }
@@ -1559,19 +1562,28 @@ static auto GetReturnTypeExpr(Context& context, SemIR::LocId loc_id,
       .type_id = context.types().GetTypeIdForTypeInstId(record_type_inst_id)};
 }
 
-// Returns the return pattern of the given function declaration. In case of an
-// unsupported return type, it produces a diagnostic and returns
-// `SemIR::ErrorInst::InstId`. Constructors are treated as returning a class
-// instance.
-static auto GetReturnPattern(Context& context, SemIR::LocId loc_id,
-                             clang::FunctionDecl* clang_decl) -> SemIR::InstId {
+// Information about a function's declared return type, corresponding to the
+// fields of SemIR::Function with the same names.
+struct ReturnInfo {
+  SemIR::TypeInstId return_type_inst_id;
+  SemIR::InstBlockId return_patterns_id;
+};
+
+// Returns information about the declared return type of the given function
+// declaration. In case of an unsupported return type, it produces a diagnostic,
+// and the returned return_type_inst_id will be `SemIR::ErrorInst::InstId`.
+// Constructors are treated as returning a class instance.
+static auto GetReturnInfo(Context& context, SemIR::LocId loc_id,
+                          clang::FunctionDecl* clang_decl) -> ReturnInfo {
   auto [type_inst_id, type_id] = GetReturnTypeExpr(context, loc_id, clang_decl);
   if (!type_inst_id.has_value()) {
     // void.
-    return SemIR::InstId::None;
+    return {.return_type_inst_id = type_inst_id,
+            .return_patterns_id = SemIR::InstBlockId::None};
   }
   if (type_inst_id == SemIR::ErrorInst::TypeInstId) {
-    return SemIR::ErrorInst::InstId;
+    return {.return_type_inst_id = type_inst_id,
+            .return_patterns_id = SemIR::InstBlockId::None};
   }
   auto pattern_type_id = GetPatternType(context, type_id);
   clang::SourceLocation return_type_loc =
@@ -1597,33 +1609,34 @@ static auto GetReturnPattern(Context& context, SemIR::LocId loc_id,
           SemIR::OutParamPattern({.type_id = pattern_type_id,
                                   .subpattern_id = return_slot_pattern_id,
                                   .index = SemIR::CallParamIndex::None})));
-  return param_pattern_id;
+  auto return_patterns_id = context.inst_blocks().Add({param_pattern_id});
+  return {.return_type_inst_id = type_inst_id,
+          .return_patterns_id = return_patterns_id};
 }
 
 namespace {
-// Represents the parameter patterns block id, the return slot pattern id and
-// the call parameters block id for a function declaration.
-struct FunctionParamsInsts {
+// Represents the insts and inst blocks associated with the parameters and
+// returns of a function declaration, corresponding to the fields of
+// SemIR::Function with the same names.
+struct FunctionSignatureInsts {
   SemIR::InstBlockId implicit_param_patterns_id;
   SemIR::InstBlockId param_patterns_id;
-  SemIR::InstId return_slot_pattern_id;
+  SemIR::TypeInstId return_type_inst_id;
+  SemIR::InstBlockId return_patterns_id;
   SemIR::InstBlockId call_params_id;
 };
 }  // namespace
 
-// Creates a block containing the parameter pattern instructions for the
-// explicit parameters, a parameter pattern instruction for the return type and
-// a block containing the call parameters of the function. Emits a callee
-// pattern-match for the explicit parameter patterns and the return slot pattern
-// to create the Call parameters instructions block. Currently the implicit
-// parameter patterns are not taken into account. Returns the parameter patterns
-// block id, the return slot pattern id, and the call parameters block id.
-// Produces a diagnostic and returns `std::nullopt` if the function declaration
-// has an unsupported parameter type.
-static auto CreateFunctionParamsInsts(Context& context, SemIR::LocId loc_id,
-                                      clang::FunctionDecl* clang_decl,
-                                      int num_params)
-    -> std::optional<FunctionParamsInsts> {
+// Creates the insts and inst blocks that represent the parameters and returns
+// of the given C++ function's Carbon counterpart, including emitting a callee
+// pattern match to create the `Call` parameters, and returns a
+// FunctionSignatureInsts containing the results. Produces a diagnostic and
+// returns `std::nullopt` if the function declaration has an unsupported
+// parameter type.
+static auto CreateFunctionSignatureInsts(Context& context, SemIR::LocId loc_id,
+                                         clang::FunctionDecl* clang_decl,
+                                         int num_params)
+    -> std::optional<FunctionSignatureInsts> {
   if (isa<clang::CXXDestructorDecl>(clang_decl)) {
     context.TODO(loc_id, "Unsupported: Destructor");
     return std::nullopt;
@@ -1639,18 +1652,20 @@ static auto CreateFunctionParamsInsts(Context& context, SemIR::LocId loc_id,
   if (!param_patterns_id.has_value()) {
     return std::nullopt;
   }
-  auto return_slot_pattern_id = GetReturnPattern(context, loc_id, clang_decl);
-  if (SemIR::ErrorInst::InstId == return_slot_pattern_id) {
+  auto [return_type_inst_id, return_patterns_id] =
+      GetReturnInfo(context, loc_id, clang_decl);
+  if (return_type_inst_id == SemIR::ErrorInst::TypeInstId) {
     return std::nullopt;
   }
 
   auto call_params_id =
       CalleePatternMatch(context, implicit_param_patterns_id, param_patterns_id,
-                         return_slot_pattern_id);
+                         return_patterns_id);
 
   return {{.implicit_param_patterns_id = implicit_param_patterns_id,
            .param_patterns_id = param_patterns_id,
-           .return_slot_pattern_id = return_slot_pattern_id,
+           .return_type_inst_id = return_type_inst_id,
+           .return_patterns_id = return_patterns_id,
            .call_params_id = call_params_id}};
 }
 
@@ -1690,7 +1705,7 @@ static auto ImportFunction(Context& context, SemIR::LocId loc_id,
   context.pattern_block_stack().Push();
 
   auto function_params_insts =
-      CreateFunctionParamsInsts(context, loc_id, clang_decl, num_params);
+      CreateFunctionSignatureInsts(context, loc_id, clang_decl, num_params);
 
   auto pattern_block_id = context.pattern_block_stack().Pop();
   auto decl_block_id = context.inst_block_stack().Pop();
@@ -1736,7 +1751,8 @@ static auto ImportFunction(Context& context, SemIR::LocId loc_id,
        .first_owning_decl_id = decl_id,
        .definition_id = SemIR::InstId::None},
       {.call_params_id = function_params_insts->call_params_id,
-       .return_slot_pattern_id = function_params_insts->return_slot_pattern_id,
+       .return_type_inst_id = function_params_insts->return_type_inst_id,
+       .return_patterns_id = function_params_insts->return_patterns_id,
        .virtual_modifier = virtual_modifier,
        .virtual_index = virtual_index,
        .self_param_id = FindSelfPattern(

toolchain/check/cpp/thunk.cpp

@@ -556,19 +556,26 @@ auto PerformCppThunkCall(Context& context, SemIR::LocId loc_id,
   auto& callee_function = context.functions().Get(callee_function_id);
   auto callee_function_params =
       context.inst_blocks().Get(callee_function.call_params_id);
+  auto callee_return_patterns =
+      context.inst_blocks().GetOrEmpty(callee_function.return_patterns_id);
 
   auto thunk_callee = GetCalleeAsFunction(context.sem_ir(), thunk_callee_id);
   auto& thunk_function = context.functions().Get(thunk_callee.function_id);
   auto thunk_function_params =
       context.inst_blocks().Get(thunk_function.call_params_id);
+  auto thunk_return_patterns =
+      context.inst_blocks().GetOrEmpty(thunk_function.return_patterns_id);
+
+  CARBON_CHECK(
+      callee_return_patterns.size() <= 1 && thunk_return_patterns.size() <= 1,
+      "TODO: generalize this logic to support multiple return patterns.");
 
   // Whether we need to pass a return address to the thunk as a final argument.
   bool thunk_takes_return_address =
-      callee_function.return_slot_pattern_id.has_value() &&
-      !thunk_function.return_slot_pattern_id.has_value();
+      !callee_return_patterns.empty() && thunk_return_patterns.empty();
 
   // The number of arguments we should be acquiring in order to call the thunk.
-  // This includes the return address parameter, if any.
+  // This includes the return address parameters, if any.
   unsigned num_thunk_args =
       context.inst_blocks().Get(thunk_function.param_patterns_id).size();
 
@@ -582,15 +589,13 @@ auto PerformCppThunkCall(Context& context, SemIR::LocId loc_id,
     return_slot_id = callee_arg_ids.consume_back();
   }
 
-  // If there's a return slot pattern, drop the corresponding parameter.
+  // If there are return slot patterns, drop the corresponding parameters.
   // TODO: The parameter should probably only be created if the return pattern
   // actually needs a return address to be passed in.
-  if (thunk_function.return_slot_pattern_id.has_value()) {
-    thunk_function_params.consume_back();
-  }
-  if (callee_function.return_slot_pattern_id.has_value()) {
-    callee_function_params.consume_back();
-  }
+  thunk_function_params =
+      thunk_function_params.drop_back(thunk_return_patterns.size());
+  callee_function_params =
+      callee_function_params.drop_back(callee_return_patterns.size());
 
   // We assume that the call parameters exactly match the parameter patterns for
   // both the thunk and the callee. This is currently guaranteed because we only

toolchain/check/function.cpp

@@ -135,9 +135,8 @@ auto CheckFunctionDefinitionSignature(Context& context,
       context.inst_blocks().GetOrEmpty(function.call_params_id);
 
   // Check the return type is complete.
-  if (function.return_slot_pattern_id.has_value()) {
-    CheckFunctionReturnType(context,
-                            SemIR::LocId(function.return_slot_pattern_id),
+  if (function.return_type_inst_id.has_value()) {
+    CheckFunctionReturnType(context, SemIR::LocId(function.return_type_inst_id),
                             function, SemIR::SpecificId::None);
     // Don't re-check the return type below.
     params_to_complete.consume_back();

toolchain/check/global_init.cpp

@@ -49,7 +49,8 @@ auto GlobalInit::Finalize() -> void {
         .non_owning_decl_id = SemIR::InstId::None,
         .first_owning_decl_id = SemIR::InstId::None},
        {.call_params_id = SemIR::InstBlockId::Empty,
-        .return_slot_pattern_id = SemIR::InstId::None,
+        .return_type_inst_id = SemIR::TypeInstId::None,
+        .return_patterns_id = SemIR::InstBlockId::None,
         .body_block_ids = {SemIR::InstBlockId::GlobalInit}}}));
 }
 

toolchain/check/handle_function.cpp

@@ -56,6 +56,7 @@ auto HandleParseNode(Context& context, Parse::ReturnTypeId node_id) -> bool {
   // Propagate the type expression.
   auto [type_node_id, type_inst_id] = context.node_stack().PopExprWithNodeId();
   auto as_type = ExprAsType(context, type_node_id, type_inst_id);
+  context.PushReturnTypeInstId(as_type.inst_id);
 
   // If the previous node was `IdentifierNameBeforeParams`, then it would have
   // caused these entries to be pushed to the pattern stacks. But it's possible
@@ -167,7 +168,8 @@ static auto MergeFunctionRedecl(Context& context,
     // match IDs in the signature.
     prev_function.MergeDefinition(new_function);
     prev_function.call_params_id = new_function.call_params_id;
-    prev_function.return_slot_pattern_id = new_function.return_slot_pattern_id;
+    prev_function.return_type_inst_id = new_function.return_type_inst_id;
+    prev_function.return_patterns_id = new_function.return_patterns_id;
     prev_function.self_param_id = new_function.self_param_id;
   }
   if (prev_import_ir_id.has_value()) {
@@ -383,14 +385,18 @@ static auto BuildFunctionDecl(Context& context,
                               Parse::AnyFunctionDeclId node_id,
                               bool is_definition)
     -> std::pair<SemIR::FunctionId, SemIR::InstId> {
-  auto return_slot_pattern_id = SemIR::InstId::None;
+  llvm::SmallVector<SemIR::InstId> return_patterns;
+  auto return_type_inst_id = SemIR::TypeInstId::None;
   if (auto [return_node, maybe_return_slot_pattern_id] =
           context.node_stack().PopWithNodeIdIf<Parse::NodeKind::ReturnType>();
       maybe_return_slot_pattern_id) {
-    return_slot_pattern_id = *maybe_return_slot_pattern_id;
+    return_patterns.push_back(*maybe_return_slot_pattern_id);
+    return_type_inst_id = context.PopReturnTypeInstId();
+    CARBON_CHECK(return_type_inst_id.has_value());
   }
 
-  auto name = PopNameComponent(context, return_slot_pattern_id);
+  auto return_patterns_id = context.inst_blocks().Add(return_patterns);
+  auto name = PopNameComponent(context, return_patterns_id);
   auto name_context = context.decl_name_stack().FinishName(name);
 
   context.node_stack()
@@ -421,7 +427,8 @@ static auto BuildFunctionDecl(Context& context,
       SemIR::Function{name_context.MakeEntityWithParamsBase(
                           name, decl_id, is_extern, introducer.extern_library),
                       {.call_params_id = name.call_params_id,
-                       .return_slot_pattern_id = name.return_slot_pattern_id,
+                       .return_type_inst_id = return_type_inst_id,
+                       .return_patterns_id = return_patterns_id,
                        .virtual_modifier = virtual_modifier,
                        .self_param_id = self_param_id}};
   if (is_definition) {
@@ -542,9 +549,7 @@ auto HandleParseNode(Context& context, Parse::FunctionDefinitionId node_id)
   // If the `}` of the function is reachable, reject if we need a return value
   // and otherwise add an implicit `return;`.
   if (IsCurrentPositionReachable(context)) {
-    if (context.functions()
-            .Get(function_id)
-            .return_slot_pattern_id.has_value()) {
+    if (context.functions().Get(function_id).return_type_inst_id.has_value()) {
       CARBON_DIAGNOSTIC(
           MissingReturnStatement, Error,
           "missing `return` at end of function with declared return type");
@@ -610,11 +615,12 @@ static auto IsValidBuiltinDeclaration(Context& context,
     return false;
   }
 
-  // Find the list of call parameters other than the implicit return slot.
-  auto call_params = context.inst_blocks().Get(function.call_params_id);
-  if (function.return_slot_pattern_id.has_value()) {
-    call_params.consume_back();
-  }
+  // Find the list of call parameters other than the implicit return slots.
+  auto call_params = context.inst_blocks()
+                         .Get(function.call_params_id)
+                         .drop_back(context.inst_blocks()
+                                        .GetOrEmpty(function.return_patterns_id)
+                                        .size());
 
   // Get the return type. This is `()` if none was specified.
   auto return_type_id = function.GetDeclaredReturnType(context.sem_ir());

toolchain/check/handle_impl.cpp

@@ -113,7 +113,7 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
     // because `impl`s are never actually called at runtime.
     auto call_params_id =
         CalleePatternMatch(context, *implicit_param_patterns_id,
-                           SemIR::InstBlockId::None, SemIR::InstId::None);
+                           SemIR::InstBlockId::None, SemIR::InstBlockId::None);
     CARBON_CHECK(call_params_id == SemIR::InstBlockId::Empty ||
                  llvm::all_of(context.inst_blocks().Get(call_params_id),
                               [](SemIR::InstId inst_id) {
@@ -143,7 +143,6 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
           .params_loc_id = Parse::NodeId::None,
           .param_patterns_id = SemIR::InstBlockId::None,
           .call_params_id = SemIR::InstBlockId::None,
-          .return_slot_pattern_id = SemIR::InstId::None,
           .pattern_block_id = pattern_block_id};
 }