فهرست منبع

Move common entity fields to a 'base' struct. (#4161)

I'd considered moving DeclParams uses over, but when handling qualified
names, there's a parse node instead of an instruction. I did try to
unify a couple other uses though, including adding MergeDefinition. I
expect `interface` will use a little more once it's more completely
implemented, but maybe I'm wrong about that.
Jon Ross-Perkins 1 سال پیش
والد
کامیت
bf89652a4d

+ 4 - 7
toolchain/check/call.cpp

@@ -32,8 +32,7 @@ static auto PerformCallToGenericClass(Context& context, Parse::NodeId node_id,
   // Convert the arguments to match the parameters.
   auto converted_args_id = ConvertCallArgs(
       context, node_id, /*self_id=*/SemIR::InstId::Invalid, arg_ids,
-      /*return_storage_id=*/SemIR::InstId::Invalid, class_info.decl_id,
-      specific_id, class_info.implicit_param_refs_id, class_info.param_refs_id);
+      /*return_storage_id=*/SemIR::InstId::Invalid, class_info, specific_id);
   return context.AddInst<SemIR::Call>(node_id,
                                       {.type_id = SemIR::TypeId::TypeType,
                                        .callee_id = callee_id,
@@ -59,9 +58,8 @@ static auto PerformCallToGenericInterface(Context& context,
   // Convert the arguments to match the parameters.
   auto converted_args_id = ConvertCallArgs(
       context, node_id, /*self_id=*/SemIR::InstId::Invalid, arg_ids,
-      /*return_storage_id=*/SemIR::InstId::Invalid, interface_info.decl_id,
-      specific_id, interface_info.implicit_param_refs_id,
-      interface_info.param_refs_id);
+      /*return_storage_id=*/SemIR::InstId::Invalid, interface_info,
+      specific_id);
   return context.AddInst<SemIR::Call>(node_id,
                                       {.type_id = SemIR::TypeId::TypeType,
                                        .callee_id = callee_id,
@@ -156,8 +154,7 @@ auto PerformCall(Context& context, Parse::NodeId node_id,
   // Convert the arguments to match the parameters.
   auto converted_args_id =
       ConvertCallArgs(context, node_id, callee_function.self_id, arg_ids,
-                      return_storage_id, callable.decl_id, specific_id,
-                      callable.implicit_param_refs_id, callable.param_refs_id);
+                      return_storage_id, callable, specific_id);
   auto call_inst_id =
       context.AddInst<SemIR::Call>(node_id, {.type_id = type_id,
                                              .callee_id = callee_id,

+ 10 - 9
toolchain/check/convert.cpp

@@ -1175,13 +1175,13 @@ static auto ConvertSelf(Context& context, SemIR::LocId call_loc_id,
 auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
                      SemIR::InstId self_id,
                      llvm::ArrayRef<SemIR::InstId> arg_refs,
-                     SemIR::InstId return_storage_id, SemIR::InstId callee_id,
-                     SemIR::GenericInstanceId callee_specific_id,
-                     SemIR::InstBlockId implicit_param_refs_id,
-                     SemIR::InstBlockId param_refs_id) -> SemIR::InstBlockId {
+                     SemIR::InstId return_storage_id,
+                     const SemIR::EntityWithParamsBase& callee,
+                     SemIR::GenericInstanceId callee_specific_id)
+    -> SemIR::InstBlockId {
   auto implicit_param_refs =
-      context.inst_blocks().GetOrEmpty(implicit_param_refs_id);
-  auto param_refs = context.inst_blocks().GetOrEmpty(param_refs_id);
+      context.inst_blocks().GetOrEmpty(callee.implicit_param_refs_id);
+  auto param_refs = context.inst_blocks().GetOrEmpty(callee.param_refs_id);
 
   // If sizes mismatch, fail early.
   if (arg_refs.size() != param_refs.size()) {
@@ -1192,7 +1192,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
     context.emitter()
         .Build(call_loc_id, CallArgCountMismatch, arg_refs.size(),
                param_refs.size())
-        .Note(callee_id, InCallToFunction)
+        .Note(callee.decl_id, InCallToFunction)
         .Emit();
     return SemIR::InstBlockId::Invalid;
   }
@@ -1210,7 +1210,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
         context.sem_ir(), implicit_param_id);
     if (param.name_id == SemIR::NameId::SelfValue) {
       auto converted_self_id =
-          ConvertSelf(context, call_loc_id, callee_id, callee_specific_id,
+          ConvertSelf(context, call_loc_id, callee.decl_id, callee_specific_id,
                       addr_pattern, param_id, param, self_id);
       if (converted_self_id == SemIR::InstId::BuiltinError) {
         return SemIR::InstBlockId::Invalid;
@@ -1229,7 +1229,8 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
         CARBON_DIAGNOSTIC(
             InCallToFunctionParam, Note,
             "Initializing parameter {0} of function declared here.", int);
-        builder.Note(callee_id, InCallToFunctionParam, diag_param_index + 1);
+        builder.Note(callee.decl_id, InCallToFunctionParam,
+                     diag_param_index + 1);
       });
 
   // Check type conversions per-element.

+ 4 - 4
toolchain/check/convert.h

@@ -95,10 +95,10 @@ auto ConvertForExplicitAs(Context& context, Parse::NodeId as_node,
 auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
                      SemIR::InstId self_id,
                      llvm::ArrayRef<SemIR::InstId> arg_refs,
-                     SemIR::InstId return_storage_id, SemIR::InstId callee_id,
-                     SemIR::GenericInstanceId callee_specific_id,
-                     SemIR::InstBlockId implicit_param_refs_id,
-                     SemIR::InstBlockId param_refs_id) -> SemIR::InstBlockId;
+                     SemIR::InstId return_storage_id,
+                     const SemIR::EntityWithParamsBase& callee,
+                     SemIR::GenericInstanceId callee_specific_id)
+    -> SemIR::InstBlockId;
 
 // Converts an expression for use as a type.
 auto ExprAsType(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id)

+ 4 - 0
toolchain/check/decl_name_stack.cpp

@@ -380,6 +380,9 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
                                name.params_id);
 
   // Find the scope corresponding to the resolved instruction.
+  // TODO: When diagnosing qualifiers on names, print a diagnostic that talks
+  // about qualifiers instead of redeclarations. Maybe also rename
+  // CheckRedeclParamsMatch.
   CARBON_KIND_SWITCH(context_->insts().Get(name_context.resolved_inst_id)) {
     case CARBON_KIND(SemIR::ClassDecl class_decl): {
       const auto& class_info = context_->classes().Get(class_decl.class_id);
@@ -414,6 +417,7 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context,
     case CARBON_KIND(SemIR::Namespace resolved_inst): {
       auto scope_id = resolved_inst.name_scope_id;
       auto& scope = context_->name_scopes().Get(scope_id);
+      // This is specifically for qualified name handling.
       if (!CheckRedeclParamsMatch(
               *context_, new_params,
               DeclParams(name_context.resolved_inst_id, Parse::NodeId::Invalid,

+ 19 - 0
toolchain/check/decl_name_stack.h

@@ -65,6 +65,8 @@ class Context;
 class DeclNameStack {
  public:
   // Context for declaration name construction.
+  // TODO: Add a helper for class, function, and interface to turn a NameContext
+  // into an EntityWithParamsBase.
   struct NameContext {
     enum class State : int8_t {
       // A context that has not processed any parts of the qualifier.
@@ -86,6 +88,23 @@ class DeclNameStack {
       Error,
     };
 
+    // Combines name information to produce a base struct for entity
+    // construction.
+    auto MakeEntityWithParamsBase(SemIR::InstId decl_id,
+                                  const NameComponent& name)
+        -> SemIR::EntityWithParamsBase {
+      return {
+          .name_id = name_id_for_new_inst(),
+          .parent_scope_id = parent_scope_id_for_new_inst(),
+          .generic_id = SemIR::GenericId::Invalid,
+          .first_param_node_id = name.first_param_node_id,
+          .last_param_node_id = name.last_param_node_id,
+          .implicit_param_refs_id = name.implicit_params_id,
+          .param_refs_id = name.params_id,
+          .decl_id = decl_id,
+      };
+    }
+
     // Returns any name collision found, or Invalid.
     auto prev_inst_id() -> SemIR::InstId;
 

+ 12 - 12
toolchain/check/global_init.cpp

@@ -35,18 +35,18 @@ auto GlobalInit::Finalize() -> void {
 
   auto name_id = context_->sem_ir().identifiers().Add("__global_init");
   context_->sem_ir().functions().Add(
-      {.name_id = SemIR::NameId::ForIdentifier(name_id),
-       .parent_scope_id = SemIR::NameScopeId::Package,
-       .decl_id = SemIR::InstId::Invalid,
-       .generic_id = SemIR::GenericId::Invalid,
-       .first_param_node_id = Parse::NodeId::Invalid,
-       .last_param_node_id = Parse::NodeId::Invalid,
-       .implicit_param_refs_id = SemIR::InstBlockId::Invalid,
-       .param_refs_id = SemIR::InstBlockId::Empty,
-       .return_storage_id = SemIR::InstId::Invalid,
-       .is_extern = false,
-       .return_slot = SemIR::Function::ReturnSlot::Absent,
-       .body_block_ids = {SemIR::InstBlockId::GlobalInit}});
+      {{.name_id = SemIR::NameId::ForIdentifier(name_id),
+        .parent_scope_id = SemIR::NameScopeId::Package,
+        .generic_id = SemIR::GenericId::Invalid,
+        .first_param_node_id = Parse::NodeId::Invalid,
+        .last_param_node_id = Parse::NodeId::Invalid,
+        .implicit_param_refs_id = SemIR::InstBlockId::Invalid,
+        .param_refs_id = SemIR::InstBlockId::Empty,
+        .decl_id = SemIR::InstId::Invalid},
+       {.return_storage_id = SemIR::InstId::Invalid,
+        .is_extern = false,
+        .return_slot = SemIR::Function::ReturnSlot::Absent,
+        .body_block_ids = {SemIR::InstBlockId::GlobalInit}}});
 }
 
 }  // namespace Carbon::Check

+ 6 - 18
toolchain/check/handle_class.cpp

@@ -56,8 +56,7 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
                              SemIR::ClassId prev_class_id, bool prev_is_extern,
                              SemIR::ImportIRId prev_import_ir_id) -> bool {
   auto& prev_class = context.classes().Get(prev_class_id);
-  SemIRLoc prev_loc =
-      prev_class.is_defined() ? prev_class.definition_id : prev_class.decl_id;
+  SemIRLoc prev_loc = prev_class.latest_decl_id();
 
   // Check the generic parameters match, if they were specified.
   if (!CheckRedeclParamsMatch(context, DeclParams(new_class),
@@ -93,11 +92,7 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
   }
 
   if (new_is_definition) {
-    prev_class.first_param_node_id = new_class.first_param_node_id;
-    prev_class.last_param_node_id = new_class.last_param_node_id;
-    prev_class.implicit_param_refs_id = new_class.implicit_param_refs_id;
-    prev_class.param_refs_id = new_class.param_refs_id;
-    prev_class.definition_id = new_class.definition_id;
+    prev_class.MergeDefinition(new_class);
     prev_class.scope_id = new_class.scope_id;
     prev_class.body_block_id = new_class.body_block_id;
     prev_class.adapt_id = new_class.adapt_id;
@@ -224,17 +219,10 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,
 
   // TODO: Store state regarding is_extern.
   SemIR::Class class_info = {
-      .name_id = name_context.name_id_for_new_inst(),
-      .parent_scope_id = name_context.parent_scope_id_for_new_inst(),
-      .generic_id = SemIR::GenericId::Invalid,
-      .first_param_node_id = name.first_param_node_id,
-      .last_param_node_id = name.last_param_node_id,
-      .implicit_param_refs_id = name.implicit_params_id,
-      .param_refs_id = name.params_id,
-      // `.self_type_id` depends on the ClassType, so is set below.
-      .self_type_id = SemIR::TypeId::Invalid,
-      .decl_id = class_decl_id,
-      .inheritance_kind = inheritance_kind};
+      name_context.MakeEntityWithParamsBase(class_decl_id, name),
+      {// `.self_type_id` depends on the ClassType, so is set below.
+       .self_type_id = SemIR::TypeId::Invalid,
+       .inheritance_kind = inheritance_kind}};
 
   MergeOrAddName(context, node_id, name_context, class_decl_id, class_decl,
                  class_info, is_definition, is_extern,

+ 7 - 20
toolchain/check/handle_function.cpp

@@ -108,9 +108,7 @@ static auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc,
                        {.loc = new_loc,
                         .is_definition = new_is_definition,
                         .is_extern = new_function.is_extern},
-                       {.loc = prev_function.definition_id.is_valid()
-                                   ? prev_function.definition_id
-                                   : prev_function.decl_id,
+                       {.loc = prev_function.latest_decl_id(),
                         .is_definition = prev_function.definition_id.is_valid(),
                         .is_extern = prev_function.is_extern},
                        prev_import_ir_id);
@@ -118,11 +116,7 @@ static auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc,
   if (new_is_definition) {
     // Track the signature from the definition, so that IDs in the body
     // match IDs in the signature.
-    prev_function.definition_id = new_function.definition_id;
-    prev_function.first_param_node_id = new_function.first_param_node_id;
-    prev_function.last_param_node_id = new_function.last_param_node_id;
-    prev_function.implicit_param_refs_id = new_function.implicit_param_refs_id;
-    prev_function.param_refs_id = new_function.param_refs_id;
+    prev_function.MergeDefinition(new_function);
     prev_function.return_storage_id = new_function.return_storage_id;
   }
   // The new function might have return slot information if it was imported.
@@ -250,18 +244,11 @@ static auto BuildFunctionDecl(Context& context,
 
   // Build the function entity. This will be merged into an existing function if
   // there is one, or otherwise added to the function store.
-  auto function_info = SemIR::Function{
-      .name_id = name_context.name_id_for_new_inst(),
-      .parent_scope_id = name_context.parent_scope_id_for_new_inst(),
-      .decl_id = decl_id,
-      .generic_id = SemIR::GenericId::Invalid,
-      .first_param_node_id = name.first_param_node_id,
-      .last_param_node_id = name.last_param_node_id,
-      .implicit_param_refs_id = name.implicit_params_id,
-      .param_refs_id = name.params_id,
-      .return_storage_id = return_storage_id,
-      .is_extern = is_extern,
-      .return_slot = return_slot};
+  auto function_info =
+      SemIR::Function{{name_context.MakeEntityWithParamsBase(decl_id, name)},
+                      {.return_storage_id = return_storage_id,
+                       .is_extern = is_extern,
+                       .return_slot = return_slot}};
   if (is_definition) {
     function_info.definition_id = decl_id;
   }

+ 8 - 8
toolchain/check/handle_interface.cpp

@@ -88,14 +88,14 @@ static auto BuildInterfaceDecl(Context& context,
     // interface name here. We should keep track of it even if the name is
     // invalid.
     SemIR::Interface interface_info = {
-        .name_id = name_context.name_id_for_new_inst(),
-        .parent_scope_id = name_context.parent_scope_id_for_new_inst(),
-        .generic_id = generic_id,
-        .first_param_node_id = name.first_param_node_id,
-        .last_param_node_id = name.last_param_node_id,
-        .implicit_param_refs_id = name.implicit_params_id,
-        .param_refs_id = name.params_id,
-        .decl_id = interface_decl_id};
+        {.name_id = name_context.name_id_for_new_inst(),
+         .parent_scope_id = name_context.parent_scope_id_for_new_inst(),
+         .generic_id = generic_id,
+         .first_param_node_id = name.first_param_node_id,
+         .last_param_node_id = name.last_param_node_id,
+         .implicit_param_refs_id = name.implicit_params_id,
+         .param_refs_id = name.params_id,
+         .decl_id = interface_decl_id}};
     interface_decl.interface_id = context.interfaces().Add(interface_info);
     if (interface_info.is_generic()) {
       interface_decl.type_id =

+ 17 - 13
toolchain/check/import.cpp

@@ -20,6 +20,13 @@
 
 namespace Carbon::Check {
 
+// Returns name information for an EntityWithParamsBase.
+template <typename T>
+static auto GetImportNameForEntity(const T& entity)
+    -> std::pair<SemIR::NameId, SemIR::NameScopeId> {
+  return {entity.name_id, entity.parent_scope_id};
+}
+
 // Returns name information for the entity, corresponding to IDs in the import
 // IR rather than the current IR.
 static auto GetImportName(const SemIR::File& import_sem_ir,
@@ -31,31 +38,28 @@ static auto GetImportName(const SemIR::File& import_sem_ir,
     case SemIR::BindSymbolicName::Kind:
     case SemIR::ExportDecl::Kind: {
       auto bind_inst = import_inst.As<SemIR::AnyBindNameOrExportDecl>();
-      const auto& entity_name =
-          import_sem_ir.entity_names().Get(bind_inst.entity_name_id);
-      return {entity_name.name_id, entity_name.parent_scope_id};
+      return GetImportNameForEntity(
+          import_sem_ir.entity_names().Get(bind_inst.entity_name_id));
     }
 
     case CARBON_KIND(SemIR::ClassDecl class_decl): {
-      const auto& class_info = import_sem_ir.classes().Get(class_decl.class_id);
-      return {class_info.name_id, class_info.parent_scope_id};
+      return GetImportNameForEntity(
+          import_sem_ir.classes().Get(class_decl.class_id));
     }
 
     case CARBON_KIND(SemIR::FunctionDecl function_decl): {
-      const auto& function =
-          import_sem_ir.functions().Get(function_decl.function_id);
-      return {function.name_id, function.parent_scope_id};
+      return GetImportNameForEntity(
+          import_sem_ir.functions().Get(function_decl.function_id));
     }
 
     case CARBON_KIND(SemIR::InterfaceDecl interface_decl): {
-      const auto& interface =
-          import_sem_ir.interfaces().Get(interface_decl.interface_id);
-      return {interface.name_id, interface.parent_scope_id};
+      return GetImportNameForEntity(
+          import_sem_ir.interfaces().Get(interface_decl.interface_id));
     }
 
     case CARBON_KIND(SemIR::Namespace ns): {
-      const auto& scope = import_sem_ir.name_scopes().Get(ns.name_scope_id);
-      return {scope.name_id, scope.parent_scope_id};
+      return GetImportNameForEntity(
+          import_sem_ir.name_scopes().Get(ns.name_scope_id));
     }
 
     default:

+ 52 - 67
toolchain/check/import_ref.cpp

@@ -642,6 +642,33 @@ class ImportRefResolver {
                    << name_scope_inst;
   }
 
+  // Given an imported entity base, returns an incomplete, local version of it.
+  //
+  // Most fields are set in the second pass once they're imported. Import enough
+  // of the parameter lists that we know whether this interface is a generic
+  // interface and can build the right constant value for it.
+  //
+  // TODO: Add a better way to represent a generic prior to importing the
+  // parameters.
+  auto GetIncompleteLocalEntityBase(
+      SemIR::InstId decl_id, const SemIR::EntityWithParamsBase& import_base)
+      -> SemIR::EntityWithParamsBase {
+    return {
+        .name_id = GetLocalNameId(import_base.name_id),
+        .parent_scope_id = SemIR::NameScopeId::Invalid,
+        .generic_id = GetLocalGeneric(import_base.generic_id),
+        .first_param_node_id = Parse::NodeId::Invalid,
+        .last_param_node_id = Parse::NodeId::Invalid,
+        .implicit_param_refs_id = import_base.implicit_param_refs_id.is_valid()
+                                      ? SemIR::InstBlockId::Empty
+                                      : SemIR::InstBlockId::Invalid,
+        .param_refs_id = import_base.param_refs_id.is_valid()
+                             ? SemIR::InstBlockId::Empty
+                             : SemIR::InstBlockId::Invalid,
+        .decl_id = decl_id,
+    };
+  }
+
   // Adds ImportRefUnloaded entries for members of the imported scope, for name
   // lookup.
   auto AddNameScopeImportRefs(const SemIR::NameScope& import_scope,
@@ -897,32 +924,12 @@ class ImportRefResolver {
                                    .decl_block_id = SemIR::InstBlockId::Empty};
     auto class_decl_id = context_.AddPlaceholderInstInNoBlock(
         SemIR::LocIdAndInst(AddImportIRInst(import_class.decl_id), class_decl));
-    // TODO: Support for importing generics.
-    auto generic_id = GetLocalGeneric(import_class.generic_id);
     // Regardless of whether ClassDecl is a complete type, we first need an
     // incomplete type so that any references have something to point at.
-    class_decl.class_id = context_.classes().Add({
-        .name_id = GetLocalNameId(import_class.name_id),
-        // These are set in the second pass once we've imported them. Import
-        // enough of the parameter lists that we know whether this class is a
-        // generic class and can build the right constant value for it.
-        // TODO: Add a better way to represent a generic `Class` prior to
-        // importing the parameters.
-        .parent_scope_id = SemIR::NameScopeId::Invalid,
-        .generic_id = generic_id,
-        .first_param_node_id = Parse::NodeId::Invalid,
-        .last_param_node_id = Parse::NodeId::Invalid,
-        .implicit_param_refs_id = import_class.implicit_param_refs_id.is_valid()
-                                      ? SemIR::InstBlockId::Empty
-                                      : SemIR::InstBlockId::Invalid,
-        .param_refs_id = import_class.param_refs_id.is_valid()
-                             ? SemIR::InstBlockId::Empty
-                             : SemIR::InstBlockId::Invalid,
-        .self_type_id = SemIR::TypeId::Invalid,
-        // These fields can be set immediately.
-        .decl_id = class_decl_id,
-        .inheritance_kind = import_class.inheritance_kind,
-    });
+    class_decl.class_id = context_.classes().Add(
+        {GetIncompleteLocalEntityBase(class_decl_id, import_class),
+         {.self_type_id = SemIR::TypeId::Invalid,
+          .inheritance_kind = import_class.inheritance_kind}});
 
     if (import_class.is_generic()) {
       class_decl.type_id = context_.GetGenericClassType(class_decl.class_id);
@@ -1122,10 +1129,7 @@ class ImportRefResolver {
         .type_id = SemIR::TypeId::Invalid,
         .function_id = SemIR::FunctionId::Invalid,
         .decl_block_id = SemIR::InstBlockId::Empty};
-    // Prefer pointing diagnostics towards a definition.
-    auto import_ir_inst_id = AddImportIRInst(function.definition_id.is_valid()
-                                                 ? function.definition_id
-                                                 : function.decl_id);
+    auto import_ir_inst_id = AddImportIRInst(function.latest_decl_id());
     auto function_decl_id = context_.AddPlaceholderInstInNoBlock(
         SemIR::LocIdAndInst(import_ir_inst_id, function_decl));
     // TODO: Implement import for generics.
@@ -1142,23 +1146,23 @@ class ImportRefResolver {
            .name_id = SemIR::NameId::ReturnSlot});
     }
     function_decl.function_id = context_.functions().Add(
-        {.name_id = GetLocalNameId(function.name_id),
-         .parent_scope_id = parent_scope_id,
-         .decl_id = function_decl_id,
-         .generic_id = generic_id,
-         .first_param_node_id = Parse::NodeId::Invalid,
-         .last_param_node_id = Parse::NodeId::Invalid,
-         .implicit_param_refs_id = GetLocalParamRefsId(
-             function.implicit_param_refs_id, implicit_param_const_ids),
-         .param_refs_id =
-             GetLocalParamRefsId(function.param_refs_id, param_const_ids),
-         .return_storage_id = new_return_storage,
-         .is_extern = function.is_extern,
-         .return_slot = function.return_slot,
-         .builtin_function_kind = function.builtin_function_kind,
-         .definition_id = function.definition_id.is_valid()
-                              ? function_decl_id
-                              : SemIR::InstId::Invalid});
+        {{.name_id = GetLocalNameId(function.name_id),
+          .parent_scope_id = parent_scope_id,
+          .generic_id = generic_id,
+          .first_param_node_id = Parse::NodeId::Invalid,
+          .last_param_node_id = Parse::NodeId::Invalid,
+          .implicit_param_refs_id = GetLocalParamRefsId(
+              function.implicit_param_refs_id, implicit_param_const_ids),
+          .param_refs_id =
+              GetLocalParamRefsId(function.param_refs_id, param_const_ids),
+          .decl_id = function_decl_id,
+          .definition_id = function.definition_id.is_valid()
+                               ? function_decl_id
+                               : SemIR::InstId::Invalid},
+         {.return_storage_id = new_return_storage,
+          .is_extern = function.is_extern,
+          .return_slot = function.return_slot,
+          .builtin_function_kind = function.builtin_function_kind}});
     // TODO: Import this or recompute it.
     auto specific_id = SemIR::GenericInstanceId::Invalid;
     function_decl.type_id =
@@ -1247,29 +1251,10 @@ class ImportRefResolver {
         context_.AddPlaceholderInstInNoBlock(SemIR::LocIdAndInst(
             AddImportIRInst(import_interface.decl_id), interface_decl));
 
-    // TODO: Support for importing generics.
-    auto generic_id = GetLocalGeneric(import_interface.generic_id);
     // Start with an incomplete interface.
-    interface_decl.interface_id = context_.interfaces().Add({
-        .name_id = GetLocalNameId(import_interface.name_id),
-        // These are set in the second pass once we've imported them. Import
-        // enough of the parameter lists that we know whether this interface is
-        // a generic interface and can build the right constant value for it.
-        // TODO: Add a better way to represent a generic `Interface` prior to
-        // importing the parameters.
-        .parent_scope_id = SemIR::NameScopeId::Invalid,
-        .generic_id = generic_id,
-        .first_param_node_id = Parse::NodeId::Invalid,
-        .last_param_node_id = Parse::NodeId::Invalid,
-        .implicit_param_refs_id =
-            import_interface.implicit_param_refs_id.is_valid()
-                ? SemIR::InstBlockId::Empty
-                : SemIR::InstBlockId::Invalid,
-        .param_refs_id = import_interface.param_refs_id.is_valid()
-                             ? SemIR::InstBlockId::Empty
-                             : SemIR::InstBlockId::Invalid,
-        .decl_id = interface_decl_id,
-    });
+    interface_decl.interface_id = context_.interfaces().Add(
+        {GetIncompleteLocalEntityBase(interface_decl_id, import_interface),
+         {}});
 
     if (import_interface.is_generic()) {
       interface_decl.type_id =

+ 6 - 7
toolchain/check/merge.h

@@ -44,13 +44,12 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id,
 // Information about the parameters of a declaration, which is common across
 // different kinds of entity such as classes and functions.
 struct DeclParams {
-  template <typename Entity>
-  explicit DeclParams(const Entity& entity)
-      : loc(entity.decl_id),
-        first_param_node_id(entity.first_param_node_id),
-        last_param_node_id(entity.last_param_node_id),
-        implicit_param_refs_id(entity.implicit_param_refs_id),
-        param_refs_id(entity.param_refs_id) {}
+  explicit DeclParams(const SemIR::EntityWithParamsBase& base)
+      : loc(base.decl_id),
+        first_param_node_id(base.first_param_node_id),
+        last_param_node_id(base.last_param_node_id),
+        implicit_param_refs_id(base.implicit_param_refs_id),
+        param_refs_id(base.param_refs_id) {}
 
   DeclParams(SemIRLoc loc, Parse::NodeId first_param_node_id,
              Parse::NodeId last_param_node_id,

+ 3 - 3
toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon

@@ -36,7 +36,7 @@ fn B() {
 // CHECK:STDOUT:     name_scope0:     {inst: inst+0, parent_scope: name_scope<invalid>, has_error: false, extended_scopes: [], names: {name0: inst+1}}
 // CHECK:STDOUT:   entity_names:    {}
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, param_refs: empty, body: [block4]}
+// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, body: [block4]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   generic_instances: {}
@@ -110,8 +110,8 @@ fn B() {
 // CHECK:STDOUT:   entity_names:
 // CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope1, index: comp_time_bind<invalid>}
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, param_refs: empty, body: [block4]}
-// CHECK:STDOUT:     function1:       {name: name1, parent_scope: name_scope1, param_refs: empty}
+// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, body: [block4]}
+// CHECK:STDOUT:     function1:       {name: name1, parent_scope: name_scope1}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   generic_instances: {}

+ 3 - 3
toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon

@@ -36,7 +36,7 @@ fn B() {
 // CHECK:STDOUT:     name_scope0:     {inst: inst+0, parent_scope: name_scope<invalid>, has_error: false, extended_scopes: [], names: {name0: inst+1}}
 // CHECK:STDOUT:   entity_names:    {}
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, param_refs: empty, body: [block4]}
+// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, body: [block4]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   generic_instances: {}
@@ -89,8 +89,8 @@ fn B() {
 // CHECK:STDOUT:   entity_names:
 // CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope1, index: comp_time_bind<invalid>}
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, param_refs: empty, body: [block4]}
-// CHECK:STDOUT:     function1:       {name: name1, parent_scope: name_scope1, param_refs: empty}
+// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, body: [block4]}
+// CHECK:STDOUT:     function1:       {name: name1, parent_scope: name_scope1}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   generic_instances: {}

+ 1 - 1
toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon

@@ -27,7 +27,7 @@ fn Foo(n: ()) -> ((), ()) {
 // CHECK:STDOUT:   entity_names:
 // CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope<invalid>, index: comp_time_bind<invalid>}
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, param_refs: block4, return_storage: inst+13, return_slot: present, body: [block7]}
+// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, return_storage: inst+13, return_slot: present, body: [block7]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:        {}
 // CHECK:STDOUT:   generic_instances: {}

+ 1 - 1
toolchain/check/testdata/basics/no_prelude/raw_ir.carbon

@@ -28,7 +28,7 @@ fn Foo[T:! type](n: T) -> (T, ()) {
 // CHECK:STDOUT:     entity_name0:    {name: name1, parent_scope: name_scope<invalid>, index: comp_time_bind0}
 // CHECK:STDOUT:     entity_name1:    {name: name2, parent_scope: name_scope<invalid>, index: comp_time_bind<invalid>}
 // CHECK:STDOUT:   functions:
-// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, param_refs: block5, return_storage: inst+15, return_slot: present, body: [block12]}
+// CHECK:STDOUT:     function0:       {name: name0, parent_scope: name_scope0, return_storage: inst+15, return_slot: present, body: [block12]}
 // CHECK:STDOUT:   classes:         {}
 // CHECK:STDOUT:   generics:
 // CHECK:STDOUT:     generic0:        {decl: inst+16, bindings: block8}

+ 1 - 0
toolchain/sem_ir/BUILD

@@ -95,6 +95,7 @@ cc_library(
         "constant.h",
         "copy_on_write_block.h",
         "entity_name.h",
+        "entity_with_params_base.h",
         "file.h",
         "function.h",
         "generic.h",

+ 18 - 34
toolchain/sem_ir/class.h

@@ -5,12 +5,13 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_CLASS_H_
 #define CARBON_TOOLCHAIN_SEM_IR_CLASS_H_
 
+#include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::SemIR {
 
-// A class.
-struct Class : public Printable<Class> {
+// Class-specific fields.
+struct ClassFields {
   enum InheritanceKind : int8_t {
     // `abstract class`
     Abstract,
@@ -20,49 +21,17 @@ struct Class : public Printable<Class> {
     Final,
   };
 
-  auto Print(llvm::raw_ostream& out) const -> void {
-    out << "{name: " << name_id << ", parent_scope: " << parent_scope_id << "}";
-  }
-
-  // Determines whether this class has been fully defined. This is false until
-  // we reach the `}` of the class definition.
-  auto is_defined() const -> bool { return object_repr_id.is_valid(); }
-
-  // Determines whether this is a generic class.
-  auto is_generic() const -> bool {
-    return implicit_param_refs_id.is_valid() || param_refs_id.is_valid();
-  }
-
   // The following members always have values, and do not change throughout the
   // lifetime of the class.
 
-  // The class name.
-  NameId name_id;
-  // The parent scope.
-  NameScopeId parent_scope_id;
-  // If this is a generic function, information about the generic.
-  GenericId generic_id;
-  // Parse tree bounds for the parameters, including both implicit and explicit
-  // parameters. These will be compared to match between declaration and
-  // definition.
-  Parse::NodeId first_param_node_id;
-  Parse::NodeId last_param_node_id;
-  // A block containing a single reference instruction per implicit parameter.
-  InstBlockId implicit_param_refs_id;
-  // A block containing a single reference instruction per parameter.
-  InstBlockId param_refs_id;
   // The class type, which is the type of `Self` in the class definition.
   TypeId self_type_id;
-  // The first declaration of the class. This is a ClassDecl.
-  InstId decl_id = InstId::Invalid;
   // The kind of inheritance that this class supports.
   // TODO: The rules here are not yet decided. See #3384.
   InheritanceKind inheritance_kind;
 
   // The following members are set at the `{` of the class definition.
 
-  // The definition of the class. This is a ClassDecl.
-  InstId definition_id = InstId::Invalid;
   // The class scope.
   NameScopeId scope_id = NameScopeId::Invalid;
   // The first block of the class body.
@@ -88,6 +57,21 @@ struct Class : public Printable<Class> {
   TypeId object_repr_id = TypeId::Invalid;
 };
 
+// A class. See EntityWithParamsBase regarding the inheritance here.
+struct Class : public EntityWithParamsBase,
+               public ClassFields,
+               public Printable<Class> {
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "{";
+    PrintBaseFields(out);
+    out << "}";
+  }
+
+  // Determines whether this class has been fully defined. This is false until
+  // we reach the `}` of the class definition.
+  auto is_defined() const -> bool { return object_repr_id.is_valid(); }
+};
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_CLASS_H_

+ 91 - 0
toolchain/sem_ir/entity_with_params_base.h

@@ -0,0 +1,91 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_TOOLCHAIN_SEM_IR_ENTITY_WITH_PARAMS_BASE_H_
+#define CARBON_TOOLCHAIN_SEM_IR_ENTITY_WITH_PARAMS_BASE_H_
+
+#include "toolchain/sem_ir/ids.h"
+
+namespace Carbon::SemIR {
+
+// Common entity fields.
+//
+// `EntityWithParamsBase` would be a base class of entities like `Function`,
+// except that then we couldn't use named initialization (or would need to
+// disable warnings about mixing named and unnamed initialization) due to how
+// C++ handles initialization of base structs. Instead, this is composed with a
+// `Fields` struct to provide an entity's actual struct.
+//
+// For example:
+//   struct FunctionFields {
+//     ... data members ...
+//   };
+//
+//   struct Function : public EntityWithParamsBase,
+//                     public FunctionFields, public Printable<Function> {
+//     ... methods ...
+//   };
+//
+// This achieves a few things:
+//   - Allows named initialization, such as:
+//     `{{.name_id = ...}, {.function_field = ...}}`
+//   - Makes `entity.name_id` access work.
+//   - Allows passing a `EntityWithParamsBase*` when only common fields are
+//     needed.
+//   - Does all this in a way that's vanilla C++.
+struct EntityWithParamsBase {
+  auto PrintBaseFields(llvm::raw_ostream& out) const -> void {
+    out << "name: " << name_id << ", parent_scope: " << parent_scope_id;
+  }
+
+  // When merging a declaration and definition, prefer things which would point
+  // at the definition for diagnostics.
+  auto MergeDefinition(const EntityWithParamsBase& definition) -> void {
+    first_param_node_id = definition.first_param_node_id;
+    last_param_node_id = definition.last_param_node_id;
+    implicit_param_refs_id = definition.implicit_param_refs_id;
+    param_refs_id = definition.param_refs_id;
+    definition_id = definition.definition_id;
+  }
+
+  // Returns the instruction for the latest declaration.
+  auto latest_decl_id() const -> SemIR::InstId {
+    return definition_id.is_valid() ? definition_id : decl_id;
+  }
+
+  // Determines whether this is a generic entity.
+  auto is_generic() const -> bool {
+    return implicit_param_refs_id.is_valid() || param_refs_id.is_valid();
+  }
+
+  // The following members always have values, and do not change throughout the
+  // lifetime of the entity.
+
+  // The class name.
+  NameId name_id;
+  // The parent scope.
+  NameScopeId parent_scope_id;
+  // If this is a generic function, information about the generic.
+  GenericId generic_id;
+  // Parse tree bounds for the parameters, including both implicit and explicit
+  // parameters. These will be compared to match between declaration and
+  // definition.
+  Parse::NodeId first_param_node_id;
+  Parse::NodeId last_param_node_id;
+  // A block containing a single reference instruction per implicit parameter.
+  InstBlockId implicit_param_refs_id;
+  // A block containing a single reference instruction per parameter.
+  InstBlockId param_refs_id;
+  // The first declaration of the entity. This will be a <entity>Decl.
+  InstId decl_id = InstId::Invalid;
+
+  // The following members are set at the `{` of the definition.
+
+  // The first declaration of the entity. This will be a <entity>Decl.
+  InstId definition_id = InstId::Invalid;
+};
+
+}  // namespace Carbon::SemIR
+
+#endif  // CARBON_TOOLCHAIN_SEM_IR_ENTITY_WITH_PARAMS_BASE_H_

+ 40 - 57
toolchain/sem_ir/function.h

@@ -6,13 +6,14 @@
 #define CARBON_TOOLCHAIN_SEM_IR_FUNCTION_H_
 
 #include "toolchain/sem_ir/builtin_function_kind.h"
+#include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::SemIR {
 
-// A function.
-struct Function : public Printable<Function> {
+// Function-specific fields.
+struct FunctionFields {
   // A value that describes whether the function uses a return slot.
   enum class ReturnSlot : int8_t {
     // Not yet known: the function has not been called or defined.
@@ -27,9 +28,44 @@ struct Function : public Printable<Function> {
     Error
   };
 
+  // The following members always have values, and do not change throughout the
+  // lifetime of the function.
+
+  // The storage for the return value, which is a reference expression whose
+  // type is the return type of the function. This may or may not be used by the
+  // function, depending on whether the return type needs a return slot, but is
+  // always present if the function has a declared return type.
+  InstId return_storage_id;
+  // Whether the declaration is extern.
+  bool is_extern;
+
+  // The following member is set on the first call to the function, or at the
+  // point where the function is defined.
+
+  // Whether the function uses a return slot. For a generic function, this
+  // tracks information about the generic, not a specific.
+  ReturnSlot return_slot;
+
+  // The following members are set at the end of a builtin function definition.
+
+  // If this is a builtin function, the corresponding builtin kind.
+  BuiltinFunctionKind builtin_function_kind = BuiltinFunctionKind::None;
+
+  // The following members are accumulated throughout the function definition.
+
+  // A list of the statically reachable code blocks in the body of the
+  // function, in lexical order. The first block is the entry block. This will
+  // be empty for declarations that don't have a visible definition.
+  llvm::SmallVector<InstBlockId> body_block_ids = {};
+};
+
+// A function. See EntityWithParamsBase regarding the inheritance here.
+struct Function : public EntityWithParamsBase,
+                  public FunctionFields,
+                  public Printable<Function> {
   auto Print(llvm::raw_ostream& out) const -> void {
-    out << "{name: " << name_id << ", parent_scope: " << parent_scope_id
-        << ", param_refs: " << param_refs_id;
+    out << "{";
+    PrintBaseFields(out);
     if (return_storage_id.is_valid()) {
       out << ", return_storage: " << return_storage_id;
       out << ", return_slot: ";
@@ -83,59 +119,6 @@ struct Function : public Printable<Function> {
     // On error, we assume no return slot is used.
     return return_slot == ReturnSlot::Present;
   }
-
-  // The following members always have values, and do not change throughout the
-  // lifetime of the function.
-
-  // The function name.
-  NameId name_id;
-  // The parent scope.
-  NameScopeId parent_scope_id;
-  // The first declaration of the function. This is a FunctionDecl.
-  InstId decl_id;
-  // If this is a generic function, information about the generic.
-  GenericId generic_id;
-  // Parse tree bounds for the parameters, including both implicit and explicit
-  // parameters. These will be compared to match between declaration and
-  // definition.
-  Parse::NodeId first_param_node_id;
-  Parse::NodeId last_param_node_id;
-  // A block containing a single reference instruction per implicit parameter.
-  InstBlockId implicit_param_refs_id;
-  // A block containing a single reference instruction per parameter.
-  InstBlockId param_refs_id;
-  // The storage for the return value, which is a reference expression whose
-  // type is the return type of the function. This may or may not be used by the
-  // function, depending on whether the return type needs a return slot, but is
-  // always present if the function has a declared return type.
-  InstId return_storage_id;
-  // Whether the declaration is extern.
-  bool is_extern;
-
-  // The following member is set on the first call to the function, or at the
-  // point where the function is defined.
-
-  // Whether the function uses a return slot. For a generic function, this
-  // tracks information about the generic, not a specific.
-  ReturnSlot return_slot;
-
-  // The following members are set at the end of a builtin function definition.
-
-  // If this is a builtin function, the corresponding builtin kind.
-  BuiltinFunctionKind builtin_function_kind = BuiltinFunctionKind::None;
-
-  // The following members are set at the `{` of the function definition.
-
-  // The definition, if the function has been defined or is currently being
-  // defined. This is a FunctionDecl.
-  InstId definition_id = InstId::Invalid;
-
-  // The following members are accumulated throughout the function definition.
-
-  // A list of the statically reachable code blocks in the body of the
-  // function, in lexical order. The first block is the entry block. This will
-  // be empty for declarations that don't have a visible definition.
-  llvm::SmallVector<InstBlockId> body_block_ids = {};
 };
 
 class File;

+ 24 - 44
toolchain/sem_ir/interface.h

@@ -5,56 +5,15 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_INTERFACE_H_
 #define CARBON_TOOLCHAIN_SEM_IR_INTERFACE_H_
 
+#include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::SemIR {
 
-// An interface.
-struct Interface : public Printable<Interface> {
-  auto Print(llvm::raw_ostream& out) const -> void {
-    out << "{name: " << name_id << ", parent_scope: " << parent_scope_id << "}";
-  }
-
-  // Determines whether this interface has been fully defined. This is false
-  // until we reach the `}` of the interface definition.
-  auto is_defined() const -> bool { return associated_entities_id.is_valid(); }
-
-  // Determines whether we're currently defining the interface. This is true
-  // between the braces of the interface.
-  auto is_being_defined() const -> bool {
-    return definition_id.is_valid() && !is_defined();
-  }
-
-  // Determines whether this is a generic interface.
-  auto is_generic() const -> bool {
-    return implicit_param_refs_id.is_valid() || param_refs_id.is_valid();
-  }
-
-  // The following members always have values, and do not change throughout the
-  // lifetime of the interface.
-
-  // The interface name.
-  NameId name_id;
-  // The parent scope.
-  NameScopeId parent_scope_id;
-  // If this is a generic function, information about the generic.
-  GenericId generic_id;
-  // Parse tree bounds for the parameters, including both implicit and explicit
-  // parameters. These will be compared to match between declaration and
-  // definition.
-  Parse::NodeId first_param_node_id;
-  Parse::NodeId last_param_node_id;
-  // A block containing a single reference instruction per implicit parameter.
-  InstBlockId implicit_param_refs_id;
-  // A block containing a single reference instruction per parameter.
-  InstBlockId param_refs_id;
-  // The first declaration of the interface. This is a InterfaceDecl.
-  InstId decl_id;
-
+// Interface-specific fields.
+struct InterfaceFields {
   // The following members are set at the `{` of the interface definition.
 
-  // The definition of the interface. This is a InterfaceDecl.
-  InstId definition_id = InstId::Invalid;
   // The interface scope.
   NameScopeId scope_id = NameScopeId::Invalid;
   // The first block of the interface body.
@@ -67,6 +26,27 @@ struct Interface : public Printable<Interface> {
   InstBlockId associated_entities_id = InstBlockId::Invalid;
 };
 
+// An interface. See EntityWithParamsBase regarding the inheritance here.
+struct Interface : public EntityWithParamsBase,
+                   public InterfaceFields,
+                   public Printable<Interface> {
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "{";
+    PrintBaseFields(out);
+    out << "}";
+  }
+
+  // Determines whether this interface has been fully defined. This is false
+  // until we reach the `}` of the interface definition.
+  auto is_defined() const -> bool { return associated_entities_id.is_valid(); }
+
+  // Determines whether we're currently defining the interface. This is true
+  // between the braces of the interface.
+  auto is_being_defined() const -> bool {
+    return definition_id.is_valid() && !is_defined();
+  }
+};
+
 }  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEM_IR_INTERFACE_H_