Convert StructTypeField to a specific type. (#4492)

This converts `StructTypeField` from an instruction to a dedicated type,
with its own store. This had originated from discussing how
`.GetAs<SemIR::StructTypeField>` was more prevalent than for other
instructions, but is probably more interesting for the storage savings
(16 bytes StructTypeField + 4 byte LocId + 4 byte InstId -> 8 byte
StructTypeField).

Due to the different structure, these now have their own stack during
construction, reducing (but not eliminating) `args_type_info_stack_`
use-cases.

The test changes of different InstIds is expected because structs and
classes generate fewer instructions now. Other than that, results should
remain the same.

I'm generally trying to avoid unrelated cleanup here due to the PR size,
though I did scrutinize the `VerifyOnFinish` calls, adding one and
commenting others (putting them in member order because that's how I was
checking what was verified and what wasn't).

toolchain/check/handle_struct.cpp

@@ -10,23 +10,23 @@
 
 namespace Carbon::Check {
 
-auto HandleParseNode(Context& context, Parse::StructTypeLiteralStartId node_id)
-    -> bool {
-  context.scope_stack().Push();
-  context.node_stack().Push(node_id);
-  context.param_and_arg_refs_stack().Push();
-  return true;
-}
-
 auto HandleParseNode(Context& context, Parse::StructLiteralStartId node_id)
     -> bool {
   context.scope_stack().Push();
   context.node_stack().Push(node_id);
-  context.args_type_info_stack().Push();
+  context.struct_type_fields_stack().PushArray();
   context.param_and_arg_refs_stack().Push();
   return true;
 }
 
+auto HandleParseNode(Context& context, Parse::StructTypeLiteralStartId node_id)
+    -> bool {
+  context.scope_stack().Push();
+  context.node_stack().Push(node_id);
+  context.struct_type_fields_stack().PushArray();
+  return true;
+}
+
 auto HandleParseNode(Context& context,
                      Parse::StructFieldDesignatorId /*node_id*/) -> bool {
   // This leaves the designated name on top because the `.` isn't interesting.
@@ -40,52 +40,47 @@ auto HandleParseNode(Context& context, Parse::StructLiteralCommaId /*node_id*/)
   return true;
 }
 
-auto HandleParseNode(Context& context,
+auto HandleParseNode(Context& /*context*/,
                      Parse::StructTypeLiteralCommaId /*node_id*/) -> bool {
-  context.param_and_arg_refs_stack().ApplyComma();
   return true;
 }
 
 auto HandleParseNode(Context& context, Parse::StructLiteralFieldId node_id)
     -> bool {
   auto value_inst_id = context.node_stack().PopExpr();
-  auto [name_node, name_id] = context.node_stack().PopNameWithNodeId();
+  // Get the name while leaving it on the stack.
+  auto name_id = context.node_stack().Peek<Parse::NodeCategory::MemberName>();
 
   // Store the name for the type.
-  context.args_type_info_stack().AddInstId(
-      context.AddInstInNoBlock<SemIR::StructTypeField>(
-          name_node,
-          {.name_id = name_id,
-           .field_type_id = context.insts().Get(value_inst_id).type_id()}));
+  auto value_type_id = context.insts().Get(value_inst_id).type_id();
+  context.struct_type_fields_stack().AppendToTop(
+      {.name_id = name_id, .type_id = value_type_id});
 
   // Push the value back on the stack as an argument.
   context.node_stack().Push(node_id, value_inst_id);
   return true;
 }
 
-auto HandleParseNode(Context& context, Parse::StructTypeLiteralFieldId node_id)
-    -> bool {
+auto HandleParseNode(Context& context,
+                     Parse::StructTypeLiteralFieldId /*node_id*/) -> bool {
   auto [type_node, type_id] = context.node_stack().PopExprWithNodeId();
   SemIR::TypeId cast_type_id = ExprAsType(context, type_node, type_id).type_id;
+  // Get the name while leaving it on the stack.
+  auto name_id = context.node_stack().Peek<Parse::NodeCategory::MemberName>();
 
-  auto [name_node, name_id] = context.node_stack().PopNameWithNodeId();
-
-  auto inst_id = context.AddInst<SemIR::StructTypeField>(
-      name_node, {.name_id = name_id, .field_type_id = cast_type_id});
-  context.node_stack().Push(node_id, inst_id);
+  context.struct_type_fields_stack().AppendToTop(
+      {.name_id = name_id, .type_id = cast_type_id});
   return true;
 }
 
-static auto DiagnoseDuplicateNames(Context& context,
-                                   SemIR::InstBlockId type_block_id,
-                                   bool is_struct_type_literal) -> bool {
-  auto& sem_ir = context.sem_ir();
-  auto fields = sem_ir.inst_blocks().Get(type_block_id);
-  Map<SemIR::NameId, SemIR::InstId> names;
-  auto& insts = sem_ir.insts();
-  for (SemIR::InstId field_inst_id : fields) {
-    auto field_inst = insts.GetAs<SemIR::StructTypeField>(field_inst_id);
-    auto result = names.Insert(field_inst.name_id, field_inst_id);
+// Diagnoses and returns true if there's a duplicate name.
+static auto DiagnoseDuplicateNames(
+    Context& context, llvm::ArrayRef<Parse::NodeId> field_name_nodes,
+    llvm::ArrayRef<SemIR::StructTypeField> fields, bool is_struct_type_literal)
+    -> bool {
+  Map<SemIR::NameId, Parse::NodeId> names;
+  for (auto [field_name_node, field] : llvm::zip(field_name_nodes, fields)) {
+    auto result = names.Insert(field.name_id, field_name_node);
     if (!result.is_inserted()) {
       CARBON_DIAGNOSTIC(StructNameDuplicate, Error,
                         "duplicated field name `{1}` in "
@@ -94,9 +89,9 @@ static auto DiagnoseDuplicateNames(Context& context,
       CARBON_DIAGNOSTIC(StructNamePrevious, Note,
                         "field with the same name here");
       context.emitter()
-          .Build(field_inst_id, StructNameDuplicate, is_struct_type_literal,
-                 field_inst.name_id)
-          .Note(result.value(), StructNamePrevious)
+          .Build(result.value(), StructNameDuplicate, is_struct_type_literal,
+                 field.name_id)
+          .Note(field_name_node, StructNamePrevious)
           .Emit();
       return true;
     }
@@ -104,47 +99,80 @@ static auto DiagnoseDuplicateNames(Context& context,
   return false;
 }
 
+// Pops the names of each field from the stack. These will have been left while
+// handling struct fields.
+static auto PopFieldNameNodes(Context& context, size_t field_count)
+    -> llvm::SmallVector<Parse::NodeId> {
+  llvm::SmallVector<Parse::NodeId> nodes;
+  nodes.reserve(field_count);
+  while (true) {
+    auto [name_node, _] =
+        context.node_stack().PopWithNodeIdIf<Parse::NodeCategory::MemberName>();
+    if (name_node.is_valid()) {
+      nodes.push_back(name_node);
+    } else {
+      break;
+    }
+  }
+  CARBON_CHECK(nodes.size() == field_count, "Found {0} names, expected {1}",
+               nodes.size(), field_count);
+  return nodes;
+}
+
 auto HandleParseNode(Context& context, Parse::StructLiteralId node_id) -> bool {
-  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
+  if (!context.node_stack().PeekIs(Parse::NodeCategory::MemberName)) {
+    // Remove the last parameter from the node stack before collecting names.
+    context.param_and_arg_refs_stack().EndNoPop(
+        Parse::NodeKind::StructLiteralStart);
+  }
+
+  auto fields = context.struct_type_fields_stack().PeekArray();
+  llvm::SmallVector<Parse::NodeId> field_name_nodes =
+      PopFieldNameNodes(context, fields.size());
+
+  auto elements_id = context.param_and_arg_refs_stack().EndAndPop(
       Parse::NodeKind::StructLiteralStart);
 
   context.scope_stack().Pop();
   context.node_stack()
       .PopAndDiscardSoloNodeId<Parse::NodeKind::StructLiteralStart>();
-  auto type_block_id = context.args_type_info_stack().Pop();
-  if (DiagnoseDuplicateNames(context, type_block_id,
+
+  if (DiagnoseDuplicateNames(context, field_name_nodes, fields,
                              /*is_struct_type_literal=*/false)) {
     context.node_stack().Push(node_id, SemIR::InstId::BuiltinError);
-    return true;
+  } else {
+    auto type_id = context.GetStructType(
+        context.struct_type_fields().AddCanonical(fields));
+
+    auto value_id = context.AddInst<SemIR::StructLiteral>(
+        node_id, {.type_id = type_id, .elements_id = elements_id});
+    context.node_stack().Push(node_id, value_id);
   }
 
-  auto type_id = context.GetStructType(type_block_id);
-
-  auto value_id = context.AddInst<SemIR::StructLiteral>(
-      node_id, {.type_id = type_id, .elements_id = refs_id});
-  context.node_stack().Push(node_id, value_id);
+  context.struct_type_fields_stack().PopArray();
   return true;
 }
 
 auto HandleParseNode(Context& context, Parse::StructTypeLiteralId node_id)
     -> bool {
-  auto refs_id = context.param_and_arg_refs_stack().EndAndPop(
-      Parse::NodeKind::StructTypeLiteralStart);
+  auto fields = context.struct_type_fields_stack().PeekArray();
+  llvm::SmallVector<Parse::NodeId> field_name_nodes =
+      PopFieldNameNodes(context, fields.size());
 
   context.scope_stack().Pop();
   context.node_stack()
       .PopAndDiscardSoloNodeId<Parse::NodeKind::StructTypeLiteralStart>();
 
-  CARBON_CHECK(refs_id != SemIR::InstBlockId::Empty,
-               "{{}} is handled by StructLiteral.");
-
-  if (DiagnoseDuplicateNames(context, refs_id,
+  if (DiagnoseDuplicateNames(context, field_name_nodes, fields,
                              /*is_struct_type_literal=*/true)) {
     context.node_stack().Push(node_id, SemIR::InstId::BuiltinError);
-    return true;
+  } else {
+    auto fields_id = context.struct_type_fields().AddCanonical(fields);
+    context.AddInstAndPush<SemIR::StructType>(
+        node_id, {.type_id = SemIR::TypeId::TypeType, .fields_id = fields_id});
   }
-  context.AddInstAndPush<SemIR::StructType>(
-      node_id, {.type_id = SemIR::TypeId::TypeType, .fields_id = refs_id});
+
+  context.struct_type_fields_stack().PopArray();
   return true;
 }