Remove return_slot_id (#4577)

Co-authored-by: Richard Smith <[email protected]>

toolchain/check/call.cpp

@@ -177,7 +177,8 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
         &context.emitter(), [&](auto& builder) {
           CARBON_DIAGNOSTIC(IncompleteReturnTypeHere, Note,
                             "return type declared here");
-          builder.Note(function.return_slot_id, IncompleteReturnTypeHere);
+          builder.Note(function.return_slot_pattern_id,
+                       IncompleteReturnTypeHere);
         });
     return CheckFunctionReturnType(context, callee_id, function,
                                    *callee_specific_id);

toolchain/check/convert.cpp

@@ -24,11 +24,11 @@
 
 namespace Carbon::Check {
 
-// Given an initializing expression, find its return slot. Returns `Invalid` if
-// there is no return slot, because the initialization is not performed in
-// place.
-static auto FindReturnSlotForInitializer(SemIR::File& sem_ir,
-                                         SemIR::InstId init_id)
+// Given an initializing expression, find its return slot argument. Returns
+// `Invalid` if there is no return slot, because the initialization is not
+// performed in place.
+static auto FindReturnSlotArgForInitializer(SemIR::File& sem_ir,
+                                            SemIR::InstId init_id)
     -> SemIR::InstId {
   while (true) {
     SemIR::Inst init_untyped = sem_ir.insts().Get(init_id);
@@ -77,15 +77,15 @@ static auto FindReturnSlotForInitializer(SemIR::File& sem_ir,
 static auto MarkInitializerFor(SemIR::File& sem_ir, SemIR::InstId init_id,
                                SemIR::InstId target_id,
                                PendingBlock& target_block) -> void {
-  auto return_slot_id = FindReturnSlotForInitializer(sem_ir, init_id);
-  if (return_slot_id.is_valid()) {
+  auto return_slot_arg_id = FindReturnSlotArgForInitializer(sem_ir, init_id);
+  if (return_slot_arg_id.is_valid()) {
     // Replace the temporary in the return slot with a reference to our target.
-    CARBON_CHECK(sem_ir.insts().Get(return_slot_id).kind() ==
+    CARBON_CHECK(sem_ir.insts().Get(return_slot_arg_id).kind() ==
                      SemIR::TemporaryStorage::Kind,
                  "Return slot for initializer does not contain a temporary; "
                  "initialized multiple times? Have {0}",
-                 sem_ir.insts().Get(return_slot_id));
-    target_block.MergeReplacing(return_slot_id, target_id);
+                 sem_ir.insts().Get(return_slot_arg_id));
+    target_block.MergeReplacing(return_slot_arg_id, target_id);
   }
 }
 
@@ -97,18 +97,18 @@ static auto MarkInitializerFor(SemIR::File& sem_ir, SemIR::InstId init_id,
 static auto FinalizeTemporary(Context& context, SemIR::InstId init_id,
                               bool discarded) -> SemIR::InstId {
   auto& sem_ir = context.sem_ir();
-  auto return_slot_id = FindReturnSlotForInitializer(sem_ir, init_id);
-  if (return_slot_id.is_valid()) {
+  auto return_slot_arg_id = FindReturnSlotArgForInitializer(sem_ir, init_id);
+  if (return_slot_arg_id.is_valid()) {
     // The return slot should already have a materialized temporary in it.
-    CARBON_CHECK(sem_ir.insts().Get(return_slot_id).kind() ==
+    CARBON_CHECK(sem_ir.insts().Get(return_slot_arg_id).kind() ==
                      SemIR::TemporaryStorage::Kind,
                  "Return slot for initializer does not contain a temporary; "
                  "initialized multiple times? Have {0}",
-                 sem_ir.insts().Get(return_slot_id));
+                 sem_ir.insts().Get(return_slot_arg_id));
     auto init = sem_ir.insts().Get(init_id);
     return context.AddInst<SemIR::Temporary>(sem_ir.insts().GetLocId(init_id),
                                              {.type_id = init.type_id(),
-                                              .storage_id = return_slot_id,
+                                              .storage_id = return_slot_arg_id,
                                               .init_id = init_id});
   }
 
@@ -258,9 +258,9 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
 
   // Arrays are always initialized in-place. Allocate a temporary as the
   // destination for the array initialization if we weren't given one.
-  SemIR::InstId return_slot_id = target.init_id;
+  SemIR::InstId return_slot_arg_id = target.init_id;
   if (!target.init_id.is_valid()) {
-    return_slot_id = target_block->AddInst<SemIR::TemporaryStorage>(
+    return_slot_arg_id = target_block->AddInst<SemIR::TemporaryStorage>(
         value_loc_id, {.type_id = target.type_id});
   }
 
@@ -276,7 +276,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
     auto init_id =
         ConvertAggregateElement<SemIR::TupleAccess, SemIR::ArrayIndex>(
             context, value_loc_id, value_id, src_type_id, literal_elems,
-            ConversionTarget::FullInitializer, return_slot_id,
+            ConversionTarget::FullInitializer, return_slot_arg_id,
             array_type.element_type_id, target_block, i, i);
     if (init_id == SemIR::InstId::BuiltinErrorInst) {
       return SemIR::InstId::BuiltinErrorInst;
@@ -290,7 +290,7 @@ static auto ConvertTupleToArray(Context& context, SemIR::TupleType tuple_type,
   return context.AddInst<SemIR::ArrayInit>(
       value_loc_id, {.type_id = target.type_id,
                      .inits_id = sem_ir.inst_blocks().Add(inits),
-                     .dest_id = return_slot_id});
+                     .dest_id = return_slot_arg_id});
 }
 
 // Performs a conversion from a tuple to a tuple type. This function only

toolchain/check/global_init.cpp

@@ -49,7 +49,6 @@ auto GlobalInit::Finalize() -> void {
         .non_owning_decl_id = SemIR::InstId::Invalid,
         .first_owning_decl_id = SemIR::InstId::Invalid},
        {.return_slot_pattern_id = SemIR::InstId::Invalid,
-        .return_slot_id = SemIR::InstId::Invalid,
         .body_block_ids = {SemIR::InstBlockId::GlobalInit}}}));
 }
 

toolchain/check/handle_function.cpp

@@ -45,11 +45,12 @@ 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 type_id = ExprAsType(context, type_node_id, type_inst_id).type_id;
-  auto return_slot_id = context.AddPatternInst<SemIR::ReturnSlotPattern>(
-      node_id, {.type_id = type_id, .type_inst_id = type_inst_id});
+  auto return_slot_pattern_id =
+      context.AddPatternInst<SemIR::ReturnSlotPattern>(
+          node_id, {.type_id = type_id, .type_inst_id = type_inst_id});
   auto param_pattern_id = context.AddPatternInst<SemIR::OutParamPattern>(
       node_id, {.type_id = type_id,
-                .subpattern_id = return_slot_id,
+                .subpattern_id = return_slot_pattern_id,
                 .runtime_index = SemIR::RuntimeParamIndex::Unknown});
   context.node_stack().Push(node_id, param_pattern_id);
   return true;
@@ -102,7 +103,7 @@ static auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc,
     // Track the signature from the definition, so that IDs in the body
     // match IDs in the signature.
     prev_function.MergeDefinition(new_function);
-    prev_function.return_slot_id = new_function.return_slot_id;
+    prev_function.return_slot_pattern_id = new_function.return_slot_pattern_id;
   }
   if ((prev_import_ir_id.is_valid() && !new_is_import)) {
     ReplacePrevInstForMerge(context, new_function.parent_scope_id,
@@ -243,7 +244,6 @@ static auto BuildFunctionDecl(Context& context,
       SemIR::Function{{name_context.MakeEntityWithParamsBase(
                           name, decl_id, is_extern, introducer.extern_library)},
                       {.return_slot_pattern_id = name.return_slot_pattern_id,
-                       .return_slot_id = name.return_slot_id,
                        .virtual_modifier = virtual_modifier}};
   if (is_definition) {
     function_info.definition_id = decl_id;
@@ -345,12 +345,12 @@ static auto HandleFunctionDefinitionAfterSignature(
   context.AddCurrentCodeBlockToFunction();
 
   // Check the return type is complete.
-  CheckFunctionReturnType(context, function.return_slot_id, function,
+  CheckFunctionReturnType(context, function.return_slot_pattern_id, function,
                           SemIR::SpecificId::Invalid);
 
   auto params_to_complete =
       context.inst_blocks().GetOrEmpty(function.call_params_id);
-  if (function.return_slot_id.is_valid()) {
+  if (function.return_slot_pattern_id.is_valid()) {
     // Exclude the return slot because it's diagnosed above.
     params_to_complete = params_to_complete.drop_back();
   }
@@ -412,7 +412,9 @@ 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 (context.is_current_position_reachable()) {
-    if (context.functions().Get(function_id).return_slot_id.is_valid()) {
+    if (context.functions()
+            .Get(function_id)
+            .return_slot_pattern_id.is_valid()) {
       CARBON_DIAGNOSTIC(
           MissingReturnStatement, Error,
           "missing `return` at end of function with declared return type");

toolchain/check/handle_impl.cpp

@@ -201,11 +201,10 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
   if (implicit_param_patterns_id) {
     // Emit the `forall` match. This shouldn't produce any `Call` params,
     // because `impl`s are never actually called at runtime.
-    auto parameter_blocks =
+    auto call_params_id =
         CalleePatternMatch(context, *implicit_param_patterns_id,
                            SemIR::InstBlockId::Invalid, SemIR::InstId::Invalid);
-    CARBON_CHECK(parameter_blocks.call_params_id == SemIR::InstBlockId::Empty);
-    CARBON_CHECK(parameter_blocks.return_slot_id == SemIR::InstId::Invalid);
+    CARBON_CHECK(call_params_id == SemIR::InstBlockId::Empty);
   }
 
   Parse::NodeId first_param_node_id =
@@ -224,7 +223,6 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
       .param_patterns_id = SemIR::InstBlockId::Invalid,
       .call_params_id = SemIR::InstBlockId::Invalid,
       .return_slot_pattern_id = SemIR::InstId::Invalid,
-      .return_slot_id = SemIR::InstId::Invalid,
       .pattern_block_id = context.pattern_block_stack().Pop(),
   };
 }

toolchain/check/import_ref.cpp

@@ -1668,7 +1668,6 @@ static auto MakeFunctionDecl(ImportContext& context,
   function_decl.function_id = context.local_functions().Add(
       {GetIncompleteLocalEntityBase(context, function_decl_id, import_function),
        {.return_slot_pattern_id = SemIR::InstId::Invalid,
-        .return_slot_id = SemIR::InstId::Invalid,
         .builtin_function_kind = import_function.builtin_function_kind}});
 
   function_decl.type_id = context.local_context().GetFunctionType(
@@ -1716,10 +1715,11 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
   }
 
   auto return_type_const_id = SemIR::ConstantId::Invalid;
-  if (import_function.return_slot_id.is_valid()) {
+  if (import_function.return_slot_pattern_id.is_valid()) {
     return_type_const_id = GetLocalConstantId(
-        resolver,
-        resolver.import_insts().Get(import_function.return_slot_id).type_id());
+        resolver, resolver.import_insts()
+                      .Get(import_function.return_slot_pattern_id)
+                      .type_id());
   }
   auto parent_scope_id =
       GetLocalNameScopeId(resolver, import_function.parent_scope_id);
@@ -1744,18 +1744,6 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
   SetGenericData(resolver, import_function.generic_id, new_function.generic_id,
                  generic_data);
 
-  if (import_function.return_slot_id.is_valid()) {
-    // Recreate the return slot from scratch.
-    // TODO: Once we import function definitions, we'll need to make sure we
-    // use the same return storage variable in the declaration and definition.
-    new_function.return_slot_id =
-        resolver.local_context().AddInstInNoBlock<SemIR::VarStorage>(
-            AddImportIRInst(resolver, import_function.return_slot_id),
-            {.type_id = resolver.local_context().GetTypeIdForTypeConstant(
-                 return_type_const_id),
-             .name_id = SemIR::NameId::ReturnSlot});
-  }
-
   if (import_function.definition_id.is_valid()) {
     new_function.definition_id = new_function.first_owning_decl_id;
   }

toolchain/check/name_component.cpp

@@ -43,7 +43,7 @@ auto PopNameComponent(Context& context, SemIR::InstId return_slot_pattern_id)
     implicit_param_patterns_id = SemIR::InstBlockId::Invalid;
   }
 
-  auto [call_params_id, return_slot_id] =
+  auto call_params_id =
       CalleePatternMatch(context, *implicit_param_patterns_id,
                          *param_patterns_id, return_slot_pattern_id);
 
@@ -59,7 +59,6 @@ auto PopNameComponent(Context& context, SemIR::InstId return_slot_pattern_id)
       .param_patterns_id = *param_patterns_id,
       .call_params_id = call_params_id,
       .return_slot_pattern_id = return_slot_pattern_id,
-      .return_slot_id = return_slot_id,
       .pattern_block_id = context.pattern_block_stack().Pop(),
   };
 }

toolchain/check/name_component.h

@@ -41,10 +41,9 @@ struct NameComponent {
   SemIR::InstBlockId call_params_id;
 
   // The return slot.
-  // TODO: These are only used for function declarations. Should they go
+  // TODO: This is only used for function declarations. Should it go
   // somewhere else?
   SemIR::InstId return_slot_pattern_id;
-  SemIR::InstId return_slot_id;
 
   // The pattern block.
   SemIR::InstBlockId pattern_block_id;

toolchain/check/pattern_match.cpp

@@ -63,10 +63,7 @@ class MatchContext {
   // specific.
   explicit MatchContext(MatchKind kind, SemIR::SpecificId callee_specific_id =
                                             SemIR::SpecificId::Invalid)
-      : next_index_(0),
-        kind_(kind),
-        callee_specific_id_(callee_specific_id),
-        return_slot_id_(SemIR::InstId::Invalid) {}
+      : next_index_(0), kind_(kind), callee_specific_id_(callee_specific_id) {}
 
   // Adds a work item to the stack.
   auto AddWork(WorkItem work_item) -> void { stack_.push_back(work_item); }
@@ -77,8 +74,6 @@ class MatchContext {
   // returns an inst block with references to all the emitted BindName insts.
   auto DoWork(Context& context) -> SemIR::InstBlockId;
 
-  auto return_slot_id() const -> SemIR::InstId { return return_slot_id_; }
-
  private:
   // Allocates the next unallocated RuntimeParamIndex, starting from 0.
   auto NextRuntimeIndex() -> SemIR::RuntimeParamIndex {
@@ -113,10 +108,6 @@ class MatchContext {
 
   // The SpecificId of the function being called (if any).
   SemIR::SpecificId callee_specific_id_;
-
-  // The return slot inst emitted by `DoWork`, if any.
-  // TODO: Can this be added to the block returned by `DoWork`, instead?
-  SemIR::InstId return_slot_id_;
 };
 
 }  // namespace
@@ -280,10 +271,15 @@ auto MatchContext::EmitPatternMatch(Context& context,
     }
     case CARBON_KIND(SemIR::ReturnSlotPattern return_slot_pattern): {
       CARBON_CHECK(kind_ == MatchKind::Callee);
-      return_slot_id_ = context.AddInst<SemIR::ReturnSlot>(
+      auto return_slot_id = context.AddInst<SemIR::ReturnSlot>(
           pattern.loc_id, {.type_id = return_slot_pattern.type_id,
                            .type_inst_id = return_slot_pattern.type_inst_id,
                            .storage_id = entry.scrutinee_id});
+      bool already_in_lookup =
+          context.scope_stack()
+              .LookupOrAddName(SemIR::NameId::ReturnSlot, return_slot_id)
+              .is_valid();
+      CARBON_CHECK(!already_in_lookup);
       results_.push_back(entry.scrutinee_id);
       break;
     }
@@ -297,11 +293,10 @@ auto CalleePatternMatch(Context& context,
                         SemIR::InstBlockId implicit_param_patterns_id,
                         SemIR::InstBlockId param_patterns_id,
                         SemIR::InstId return_slot_pattern_id)
-    -> ParameterBlocks {
+    -> SemIR::InstBlockId {
   if (!return_slot_pattern_id.is_valid() && !param_patterns_id.is_valid() &&
       !implicit_param_patterns_id.is_valid()) {
-    return {.call_params_id = SemIR::InstBlockId::Invalid,
-            .return_slot_id = SemIR::InstId::Invalid};
+    return SemIR::InstBlockId::Invalid;
   }
 
   MatchContext match(MatchKind::Callee);
@@ -329,8 +324,7 @@ auto CalleePatternMatch(Context& context,
     }
   }
 
-  return {.call_params_id = match.DoWork(context),
-          .return_slot_id = match.return_slot_id()};
+  return match.DoWork(context);
 }
 
 auto CallerPatternMatch(Context& context, SemIR::SpecificId specific_id,

toolchain/check/pattern_match.h

@@ -10,17 +10,6 @@
 
 namespace Carbon::Check {
 
-// The outputs of CalleePatternMatch.
-// TODO: Rename or remove this struct.
-struct ParameterBlocks {
-  // The `Call` parameters of the function.
-  SemIR::InstBlockId call_params_id;
-
-  // The return slot.
-  // TODO: Drop this and just use the last element of above?
-  SemIR::InstId return_slot_id;
-};
-
 // TODO: Find a better place for this overview, once it has stabilized.
 //
 // The signature pattern of a function call is matched partially by the caller
@@ -35,11 +24,13 @@ struct ParameterBlocks {
 // callee side of pattern matching, starting at the `ParamPattern` insts, and
 // matching them against the corresponding `Call` parameters (see
 // entity_with_params_base.h for the definition of that term).
+// Returns the ID of an inst block consisting of references to the `Call`
+// parameters of the function.
 auto CalleePatternMatch(Context& context,
                         SemIR::InstBlockId implicit_param_patterns_id,
                         SemIR::InstBlockId param_patterns_id,
                         SemIR::InstId return_slot_pattern_id)
-    -> ParameterBlocks;
+    -> SemIR::InstBlockId;
 
 // Emits the pattern-match IR for matching the given arguments with the given
 // parameter patterns, and returns an inst block of the arguments that should