lazy field index

toolchain/check/context.cpp

@@ -199,6 +199,15 @@ auto Context::ReplaceInstBeforeConstantUse(SemIR::InstId inst_id,
   FinishInst(inst_id, inst);
 }
 
+auto Context::ReplaceInstPreservingConstantValue(SemIR::InstId inst_id,
+                                                 SemIR::Inst inst) -> void {
+  auto old_const_id = sem_ir().constant_values().Get(inst_id);
+  sem_ir().insts().Set(inst_id, inst);
+  CARBON_VLOG("ReplaceInst: {0} -> {1}\n", inst_id, inst);
+  auto new_const_id = TryEvalInst(*this, inst_id, inst);
+  CARBON_CHECK(old_const_id == new_const_id);
+}
+
 auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def)
     -> void {
   CARBON_DIAGNOSTIC(NameDeclDuplicate, Error,

toolchain/check/context.h

@@ -194,6 +194,11 @@ class Context {
   auto ReplaceInstBeforeConstantUse(SemIR::InstId inst_id, SemIR::Inst inst)
       -> void;
 
+  // Replaces the instruction `inst_id` with `inst`, not affecting location.
+  // The instruction is required to not change its constant value.
+  auto ReplaceInstPreservingConstantValue(SemIR::InstId inst_id,
+                                          SemIR::Inst inst) -> void;
+
   // Sets only the parse node of an instruction. This is only used when setting
   // the parse node of an imported namespace. Versus
   // ReplaceInstBeforeConstantUse, it is safe to use after the namespace is used
@@ -492,6 +497,10 @@ class Context {
     return struct_type_fields_stack_;
   }
 
+  auto field_decls_stack() -> ArrayStack<SemIR::InstId>& {
+    return field_decls_stack_;
+  }
+
   auto decl_name_stack() -> DeclNameStack& { return decl_name_stack_; }
 
   auto decl_introducer_state_stack() -> DeclIntroducerStateStack& {
@@ -648,10 +657,12 @@ class Context {
   // arguments.
   InstBlockStack args_type_info_stack_;
 
-  // The stack of StructTypeFields for in-progress StructTypeLiterals and Class
-  // object representations.
+  // The stack of StructTypeFields for in-progress StructTypeLiterals.
   ArrayStack<SemIR::StructTypeField> struct_type_fields_stack_;
 
+  // The stack of FieldDecls for in-progress Class definitions.
+  ArrayStack<SemIR::InstId> field_decls_stack_;
+
   // The stack used for qualified declaration name construction.
   DeclNameStack decl_name_stack_;
 

toolchain/check/handle_binding_pattern.cpp

@@ -128,15 +128,11 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
         auto field_type_id = context.GetUnboundElementType(
             class_info.self_type_id, cast_type_id);
         auto field_id = context.AddInst<SemIR::FieldDecl>(
-            binding_id,
-            {.type_id = field_type_id,
-             .name_id = name_id,
-             .index = SemIR::ElementIndex(
-                 context.struct_type_fields_stack().PeekArray().size())});
+            binding_id, {.type_id = field_type_id,
+                         .name_id = name_id,
+                         .index = SemIR::ElementIndex::Invalid});
+        context.field_decls_stack().AppendToTop(field_id);
 
-        // Add a corresponding field to the object representation of the class.
-        context.struct_type_fields_stack().AppendToTop(
-            {.name_id = name_id, .type_id = cast_type_id});
         context.node_stack().Push(node_id, field_id);
         break;
       }

toolchain/check/handle_class.cpp

@@ -295,7 +295,7 @@ auto HandleParseNode(Context& context, Parse::ClassDefinitionStartId node_id)
 
   context.inst_block_stack().Push();
   context.node_stack().Push(node_id, class_id);
-  context.struct_type_fields_stack().PushArray();
+  context.field_decls_stack().PushArray();
 
   // TODO: Handle the case where there's control flow in the class body. For
   // example:
@@ -512,7 +512,7 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool {
     return true;
   }
 
-  if (!context.struct_type_fields_stack().PeekArray().empty()) {
+  if (!context.field_decls_stack().PeekArray().empty()) {
     // TODO: Add note that includes the first field location as an example.
     CARBON_DIAGNOSTIC(
         BaseDeclAfterFieldDecl, Error,
@@ -523,28 +523,23 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool {
 
   auto base_info = CheckBaseType(context, base_type_node_id, base_type_expr_id);
 
+  // TODO: Should we diagnose if there are already any fields?
+
   // The `base` value in the class scope has an unbound element type. Instance
   // binding will be performed when it's found by name lookup into an instance.
   auto field_type_id =
       context.GetUnboundElementType(class_info.self_type_id, base_info.type_id);
   class_info.base_id = context.AddInst<SemIR::BaseDecl>(
       node_id, {.type_id = field_type_id,
                 .base_type_id = base_info.type_id,
-                .index = SemIR::ElementIndex(
-                    context.struct_type_fields_stack().PeekArray().size())});
+                .index = SemIR::ElementIndex::Invalid});
 
   if (base_info.type_id != SemIR::TypeId::Error) {
     auto base_class_info = context.classes().Get(
         context.types().GetAs<SemIR::ClassType>(base_info.type_id).class_id);
     class_info.is_dynamic |= base_class_info.is_dynamic;
   }
 
-  // Add a corresponding field to the object representation of the class.
-  // TODO: Consider whether we want to use `partial T` here.
-  // TODO: Should we diagnose if there are already any fields?
-  context.struct_type_fields_stack().AppendToTop(
-      {.name_id = SemIR::NameId::Base, .type_id = base_info.type_id});
-
   // Bind the name `base` in the class to the base field.
   context.decl_name_stack().AddNameOrDiagnoseDuplicate(
       context.decl_name_stack().MakeUnqualifiedName(node_id,
@@ -567,8 +562,7 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool {
 // returns a corresponding complete type witness instruction.
 static auto CheckCompleteAdapterClassType(Context& context,
                                           Parse::NodeId node_id,
-                                          SemIR::ClassId class_id,
-                                          SemIR::StructTypeFieldsId fields_id)
+                                          SemIR::ClassId class_id)
     -> SemIR::InstId {
   const auto& class_info = context.classes().Get(class_id);
   if (class_info.base_id.is_valid()) {
@@ -581,17 +575,14 @@ static auto CheckCompleteAdapterClassType(Context& context,
     return SemIR::InstId::BuiltinErrorInst;
   }
 
-  if (auto fields = context.struct_type_fields().Get(fields_id);
-      !fields.empty()) {
-    auto [first_field_inst_id, _] = context.LookupNameInExactScope(
-        node_id, fields.front().name_id, class_info.scope_id,
-        context.name_scopes().Get(class_info.scope_id));
+  auto field_decls = context.field_decls_stack().PeekArray();
+  if (!field_decls.empty()) {
     CARBON_DIAGNOSTIC(AdaptWithFields, Error, "adapter with fields");
     CARBON_DIAGNOSTIC(AdaptWithFieldHere, Note,
                       "first field declaration is here");
     context.emitter()
         .Build(class_info.adapt_id, AdaptWithFields)
-        .Note(first_field_inst_id, AdaptWithFieldHere)
+        .Note(field_decls.front(), AdaptWithFieldHere)
         .Emit();
     return SemIR::InstId::BuiltinErrorInst;
   }
@@ -637,46 +628,79 @@ static auto CheckCompleteAdapterClassType(Context& context,
       {.type_id = context.GetBuiltinType(SemIR::BuiltinInstKind::WitnessType),
        .object_repr_id = adapted_type_id});
 }
+static auto AddStructTypeFields(
+    Context& context,
+    llvm::SmallVector<SemIR::StructTypeField>& struct_type_fields)
+    -> SemIR::StructTypeFieldsId {
+  for (auto field_decl_id : context.field_decls_stack().PeekArray()) {
+    auto field_decl = context.insts().GetAs<SemIR::FieldDecl>(field_decl_id);
+    field_decl.index =
+        SemIR::ElementIndex{static_cast<int>(struct_type_fields.size())};
+    context.ReplaceInstPreservingConstantValue(field_decl_id, field_decl);
+    if (field_decl.type_id == SemIR::TypeId::Error) {
+      struct_type_fields.push_back(
+          {.name_id = field_decl.name_id, .type_id = SemIR::TypeId::Error});
+      continue;
+    }
+    auto unbound_element_type =
+        context.sem_ir().types().GetAs<SemIR::UnboundElementType>(
+            field_decl.type_id);
+    struct_type_fields.push_back(
+        {.name_id = field_decl.name_id,
+         .type_id = unbound_element_type.element_type_id});
+  }
+  auto fields_id =
+      context.struct_type_fields().AddCanonical(struct_type_fields);
+  return fields_id;
+}
 
 // Checks that the specified finished class definition is valid and builds and
 // returns a corresponding complete type witness instruction.
 static auto CheckCompleteClassType(Context& context, Parse::NodeId node_id,
                                    SemIR::ClassId class_id) -> SemIR::InstId {
   auto& class_info = context.classes().Get(class_id);
   if (class_info.adapt_id.is_valid()) {
-    auto fields_id = context.struct_type_fields().AddCanonical(
-        context.struct_type_fields_stack().PeekArray());
-    context.struct_type_fields_stack().PopArray();
-
-    return CheckCompleteAdapterClassType(context, node_id, class_id, fields_id);
+    return CheckCompleteAdapterClassType(context, node_id, class_id);
   }
 
-  bool defining_vtable_ptr = class_info.is_dynamic;
+  bool defining_vptr = class_info.is_dynamic;
   if (class_info.base_id.is_valid()) {
     auto base_info = context.insts().GetAs<SemIR::BaseDecl>(class_info.base_id);
     // TODO: If the base class is template dependent, we will need to decide
     // whether to add a vptr as part of instantiation.
     if (auto* base_class_info = TryGetAsClass(context, base_info.base_type_id);
         base_class_info && base_class_info->is_dynamic) {
-      defining_vtable_ptr = false;
+      defining_vptr = false;
     }
   }
 
-  if (defining_vtable_ptr) {
-    context.struct_type_fields_stack().PrependToTop(
+  auto field_decls = context.field_decls_stack().PeekArray();
+  llvm::SmallVector<SemIR::StructTypeField> struct_type_fields;
+  struct_type_fields.reserve(defining_vptr + class_info.base_id.is_valid() +
+                             field_decls.size());
+  if (defining_vptr) {
+    struct_type_fields.push_back(
         {.name_id = SemIR::NameId::Vptr,
          .type_id = context.GetPointerType(
              context.GetBuiltinType(SemIR::BuiltinInstKind::VtableType))});
   }
-
-  auto fields_id = context.struct_type_fields().AddCanonical(
-      context.struct_type_fields_stack().PeekArray());
-  context.struct_type_fields_stack().PopArray();
+  if (class_info.base_id.is_valid()) {
+    auto base_decl = context.insts().GetAs<SemIR::BaseDecl>(class_info.base_id);
+    base_decl.index =
+        SemIR::ElementIndex{static_cast<int>(struct_type_fields.size())};
+    context.ReplaceInstPreservingConstantValue(class_info.base_id, base_decl);
+    struct_type_fields.push_back(
+        {.name_id = SemIR::NameId::Base,
+         .type_id = context.insts()
+                        .GetAs<SemIR::BaseDecl>(class_info.base_id)
+                        .base_type_id});
+  }
 
   return context.AddInst<SemIR::CompleteTypeWitness>(
       node_id,
       {.type_id = context.GetBuiltinType(SemIR::BuiltinInstKind::WitnessType),
-       .object_repr_id = context.GetStructType(fields_id)});
+       .object_repr_id = context.GetStructType(
+           AddStructTypeFields(context, struct_type_fields))});
 }
 
 auto HandleParseNode(Context& context, Parse::ClassDefinitionId node_id)
@@ -691,6 +715,7 @@ auto HandleParseNode(Context& context, Parse::ClassDefinitionId node_id)
   class_info.complete_type_witness_id = complete_type_witness_id;
 
   context.inst_block_stack().Pop();
+  context.field_decls_stack().PopArray();
 
   FinishGenericDefinition(context, class_info.generic_id);
 

toolchain/check/handle_function.cpp

@@ -212,8 +212,8 @@ static auto BuildFunctionDecl(Context& context,
       parent_scope_inst) {
     if (auto class_decl = parent_scope_inst->TryAs<SemIR::ClassDecl>()) {
       auto& class_info = context.classes().Get(class_decl->class_id);
-      CARBON_CHECK(virtual_modifier != SemIR::Function::VirtualModifier::Impl ||
-                   class_info.is_dynamic);
+      // TODO: If this is an `impl` function, check there's a matching base
+      // function that's impl or virtual.
       class_info.is_dynamic = true;
     }
   }

toolchain/check/testdata/class/fail_adapt_with_subobjects.carbon

@@ -121,7 +121,7 @@ class AdaptWithBaseAndFields {
 // CHECK:STDOUT:   %.loc10_12.2: type = converted %int.make_type_signed, %.loc10_12.1 [template = constants.%i32]
 // CHECK:STDOUT:   adapt_decl %i32
 // CHECK:STDOUT:   %Base.ref: type = name_ref Base, file.%Base.decl [template = constants.%Base]
-// CHECK:STDOUT:   %.loc15: %.5 = base_decl %Base, element0 [template]
+// CHECK:STDOUT:   %.loc15: %.5 = base_decl %Base, element<invalid> [template]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%AdaptWithBase
@@ -171,7 +171,7 @@ class AdaptWithBaseAndFields {
 // CHECK:STDOUT:   %int.make_type_signed.loc13: init type = call constants.%Int(%.loc13_10.1) [template = constants.%i32]
 // CHECK:STDOUT:   %.loc13_10.2: type = value_of_initializer %int.make_type_signed.loc13 [template = constants.%i32]
 // CHECK:STDOUT:   %.loc13_10.3: type = converted %int.make_type_signed.loc13, %.loc13_10.2 [template = constants.%i32]
-// CHECK:STDOUT:   %.loc13_8: %.2 = field_decl n, element0 [template]
+// CHECK:STDOUT:   %.loc13_8: %.2 = field_decl n, element<invalid> [template]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%AdaptWithField
@@ -188,17 +188,17 @@ class AdaptWithBaseAndFields {
 // CHECK:STDOUT:   %int.make_type_signed.loc25: init type = call constants.%Int(%.loc25_10.1) [template = constants.%i32]
 // CHECK:STDOUT:   %.loc25_10.2: type = value_of_initializer %int.make_type_signed.loc25 [template = constants.%i32]
 // CHECK:STDOUT:   %.loc25_10.3: type = converted %int.make_type_signed.loc25, %.loc25_10.2 [template = constants.%i32]
-// CHECK:STDOUT:   %.loc25_8: %.3 = field_decl a, element0 [template]
+// CHECK:STDOUT:   %.loc25_8: %.3 = field_decl a, element<invalid> [template]
 // CHECK:STDOUT:   %.loc26_10.1: Core.IntLiteral = int_value 32 [template = constants.%.1]
 // CHECK:STDOUT:   %int.make_type_signed.loc26: init type = call constants.%Int(%.loc26_10.1) [template = constants.%i32]
 // CHECK:STDOUT:   %.loc26_10.2: type = value_of_initializer %int.make_type_signed.loc26 [template = constants.%i32]
 // CHECK:STDOUT:   %.loc26_10.3: type = converted %int.make_type_signed.loc26, %.loc26_10.2 [template = constants.%i32]
-// CHECK:STDOUT:   %.loc26_8: %.3 = field_decl b, element1 [template]
+// CHECK:STDOUT:   %.loc26_8: %.3 = field_decl b, element<invalid> [template]
 // CHECK:STDOUT:   %.loc27_10.1: Core.IntLiteral = int_value 32 [template = constants.%.1]
 // CHECK:STDOUT:   %int.make_type_signed.loc27: init type = call constants.%Int(%.loc27_10.1) [template = constants.%i32]
 // CHECK:STDOUT:   %.loc27_10.2: type = value_of_initializer %int.make_type_signed.loc27 [template = constants.%i32]
 // CHECK:STDOUT:   %.loc27_10.3: type = converted %int.make_type_signed.loc27, %.loc27_10.2 [template = constants.%i32]
-// CHECK:STDOUT:   %.loc27_8: %.3 = field_decl c, element2 [template]
+// CHECK:STDOUT:   %.loc27_8: %.3 = field_decl c, element<invalid> [template]
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%AdaptWithFields
@@ -250,12 +250,12 @@ class AdaptWithBaseAndFields {
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @AdaptWithBaseAndFields {
 // CHECK:STDOUT:   %Base.ref: type = name_ref Base, file.%Base.decl [template = constants.%Base]
-// CHECK:STDOUT:   %.loc7: %.4 = base_decl %Base, element0 [template]
+// CHECK:STDOUT:   %.loc7: %.4 = base_decl %Base, element<invalid> [template]
 // CHECK:STDOUT:   %.loc8_10.1: Core.IntLiteral = int_value 32 [template = constants.%.5]
 // CHECK:STDOUT:   %int.make_type_signed: init type = call constants.%Int(%.loc8_10.1) [template = constants.%i32]
 // CHECK:STDOUT:   %.loc8_10.2: type = value_of_initializer %int.make_type_signed [template = constants.%i32]
 // CHECK:STDOUT:   %.loc8_10.3: type = converted %int.make_type_signed, %.loc8_10.2 [template = constants.%i32]
-// CHECK:STDOUT:   %.loc8_8: %.6 = field_decl n, element1 [template]
+// CHECK:STDOUT:   %.loc8_8: %.6 = field_decl n, element<invalid> [template]
 // CHECK:STDOUT:   %.loc15_10: %.1 = struct_literal ()
 // CHECK:STDOUT:   %.loc15_11: type = converted %.loc15_10, constants.%.1 [template = constants.%.1]
 // CHECK:STDOUT:   adapt_decl %.1