Add fields for extern to EntityWithParamsBase (#4206)

This adds fields to `EntityWithParamsBase` to reflect the intention with
`extern library` design. I'm renaming `decl_id` because it shouldn't be
expected to be assigned anymore. import_ref.cpp I'm deliberately keeping
on `first_owning_decl_id` (which will break when importing `extern
library` declarations). Most other cases are for diagnostics, and I'm
using `latest_decl_id` to try and get the closest declaration to the
error. Note I'm partly splitting out this PR to show the test effect,
which apparently we don't test related cases.

toolchain/check/context.cpp

@@ -198,7 +198,7 @@ auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
   } else {
     CARBON_DIAGNOSTIC(ClassForwardDeclaredHere, Note,
                       "Class was forward declared here.");
-    builder.Note(class_info.decl_id, ClassForwardDeclaredHere);
+    builder.Note(class_info.latest_decl_id(), ClassForwardDeclaredHere);
   }
 }
 
@@ -214,7 +214,7 @@ auto Context::NoteUndefinedInterface(SemIR::InterfaceId interface_id,
   } else {
     CARBON_DIAGNOSTIC(InterfaceForwardDeclaredHere, Note,
                       "Interface was forward declared here.");
-    builder.Note(interface_info.decl_id, InterfaceForwardDeclaredHere);
+    builder.Note(interface_info.latest_decl_id(), InterfaceForwardDeclaredHere);
   }
 }
 

toolchain/check/convert.cpp

@@ -1193,7 +1193,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.decl_id, InCallToFunction)
+        .Note(callee.latest_decl_id(), InCallToFunction)
         .Emit();
     return SemIR::InstBlockId::Invalid;
   }
@@ -1210,9 +1210,9 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
     auto [param_id, param] = SemIR::Function::GetParamFromParamRefId(
         context.sem_ir(), implicit_param_id);
     if (param.name_id == SemIR::NameId::SelfValue) {
-      auto converted_self_id =
-          ConvertSelf(context, call_loc_id, callee.decl_id, callee_specific_id,
-                      addr_pattern, param_id, param, self_id);
+      auto converted_self_id = ConvertSelf(
+          context, call_loc_id, callee.latest_decl_id(), callee_specific_id,
+          addr_pattern, param_id, param, self_id);
       if (converted_self_id == SemIR::InstId::BuiltinError) {
         return SemIR::InstBlockId::Invalid;
       }
@@ -1230,7 +1230,7 @@ auto ConvertCallArgs(Context& context, SemIR::LocId call_loc_id,
         CARBON_DIAGNOSTIC(
             InCallToFunctionParam, Note,
             "Initializing parameter {0} of function declared here.", int);
-        builder.Note(callee.decl_id, InCallToFunctionParam,
+        builder.Note(callee.latest_decl_id(), InCallToFunctionParam,
                      diag_param_index + 1);
       });
 

toolchain/check/decl_name_stack.h

@@ -90,8 +90,8 @@ class DeclNameStack {
 
     // Combines name information to produce a base struct for entity
     // construction.
-    auto MakeEntityWithParamsBase(SemIR::InstId decl_id,
-                                  const NameComponent& name)
+    auto MakeEntityWithParamsBase(const NameComponent& name,
+                                  SemIR::InstId decl_id, bool is_extern)
         -> SemIR::EntityWithParamsBase {
       return {
           .name_id = name_id_for_new_inst(),
@@ -101,7 +101,10 @@ class DeclNameStack {
           .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,
+          .is_extern = is_extern,
+          .extern_library_id = StringLiteralValueId::Invalid,
+          .non_owning_decl_id = SemIR::InstId::Invalid,
+          .first_owning_decl_id = decl_id,
       };
     }
 

toolchain/check/function.cpp

@@ -41,21 +41,21 @@ auto CheckFunctionTypeMatches(Context& context,
         "Function redeclaration differs because no return type is provided.");
     auto diag =
         new_return_type_id.is_valid()
-            ? context.emitter().Build(new_function.decl_id,
+            ? context.emitter().Build(new_function.latest_decl_id(),
                                       FunctionRedeclReturnTypeDiffers,
                                       new_return_type_id)
-            : context.emitter().Build(new_function.decl_id,
+            : context.emitter().Build(new_function.latest_decl_id(),
                                       FunctionRedeclReturnTypeDiffersNoReturn);
     if (prev_return_type_id.is_valid()) {
       CARBON_DIAGNOSTIC(FunctionRedeclReturnTypePrevious, Note,
                         "Previously declared with return type `{0}`.",
                         SemIR::TypeId);
-      diag.Note(prev_function.decl_id, FunctionRedeclReturnTypePrevious,
-                prev_return_type_id);
+      diag.Note(prev_function.latest_decl_id(),
+                FunctionRedeclReturnTypePrevious, prev_return_type_id);
     } else {
       CARBON_DIAGNOSTIC(FunctionRedeclReturnTypePreviousNoReturn, Note,
                         "Previously declared with no return type.");
-      diag.Note(prev_function.decl_id,
+      diag.Note(prev_function.latest_decl_id(),
                 FunctionRedeclReturnTypePreviousNoReturn);
     }
     diag.Emit();

toolchain/check/global_init.cpp

@@ -42,9 +42,11 @@ auto GlobalInit::Finalize() -> void {
         .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,
+        .extern_library_id = StringLiteralValueId::Invalid,
+        .non_owning_decl_id = SemIR::InstId::Invalid,
+        .first_owning_decl_id = SemIR::InstId::Invalid},
+       {.return_storage_id = SemIR::InstId::Invalid,
         .body_block_ids = {SemIR::InstBlockId::GlobalInit}}});
 }
 

toolchain/check/handle_class.cpp

@@ -102,10 +102,10 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
 
   if ((prev_import_ir_id.is_valid() && !new_is_import) ||
       (prev_is_extern && !new_is_extern)) {
-    prev_class.decl_id = new_class.decl_id;
+    prev_class.first_owning_decl_id = new_class.first_owning_decl_id;
     ReplacePrevInstForMerge(
         context, prev_class.parent_scope_id, prev_class.name_id,
-        new_is_import ? new_loc.inst_id : new_class.decl_id);
+        new_is_import ? new_loc.inst_id : new_class.first_owning_decl_id);
   }
   return true;
 }
@@ -219,7 +219,7 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id,
 
   // TODO: Store state regarding is_extern.
   SemIR::Class class_info = {
-      name_context.MakeEntityWithParamsBase(class_decl_id, name),
+      name_context.MakeEntityWithParamsBase(name, class_decl_id, is_extern),
       {// `.self_type_id` depends on the ClassType, so is set below.
        .self_type_id = SemIR::TypeId::Invalid,
        .inheritance_kind = inheritance_kind}};

toolchain/check/handle_function.cpp

@@ -100,9 +100,10 @@ static auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc,
   if ((prev_import_ir_id.is_valid() && !new_is_import) ||
       (prev_function.is_extern && !new_function.is_extern)) {
     prev_function.is_extern = new_function.is_extern;
-    prev_function.decl_id = new_function.decl_id;
+    prev_function.first_owning_decl_id = new_function.first_owning_decl_id;
     ReplacePrevInstForMerge(context, prev_function.parent_scope_id,
-                            prev_function.name_id, new_function.decl_id);
+                            prev_function.name_id,
+                            new_function.first_owning_decl_id);
   }
   return true;
 }
@@ -151,7 +152,7 @@ static auto TryMergeRedecl(Context& context, Parse::AnyFunctionDeclId node_id,
   }
 
   if (!prev_function_id.is_valid()) {
-    context.DiagnoseDuplicateName(function_info.decl_id, prev_id);
+    context.DiagnoseDuplicateName(function_info.latest_decl_id(), prev_id);
     return;
   }
 
@@ -216,8 +217,8 @@ 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_context.MakeEntityWithParamsBase(decl_id, name)},
-      {.return_storage_id = return_storage_id, .is_extern = is_extern}};
+      {name_context.MakeEntityWithParamsBase(name, decl_id, is_extern)},
+      {.return_storage_id = return_storage_id}};
   if (is_definition) {
     function_info.definition_id = decl_id;
   }
@@ -244,7 +245,7 @@ static auto BuildFunctionDecl(Context& context,
   if (!name_context.prev_inst_id().is_valid()) {
     // At interface scope, a function declaration introduces an associated
     // function.
-    auto lookup_result_id = function_info.decl_id;
+    auto lookup_result_id = decl_id;
     if (parent_scope_inst && !name_context.has_qualifiers) {
       if (auto interface_scope =
               parent_scope_inst->TryAs<SemIR::InterfaceDecl>()) {

toolchain/check/handle_interface.cpp

@@ -75,7 +75,7 @@ static auto BuildInterfaceDecl(Context& context,
                              {.loc = node_id,
                               .is_definition = is_definition,
                               .is_extern = false},
-                             {.loc = existing_interface.decl_id,
+                             {.loc = existing_interface.latest_decl_id(),
                               .is_definition = existing_interface.is_defined(),
                               .is_extern = false},
                              /*prev_import_ir_id=*/SemIR::ImportIRId::Invalid);
@@ -103,8 +103,8 @@ static auto BuildInterfaceDecl(Context& context,
     // there was an error in the qualifier, we will have lost track of the
     // interface name here. We should keep track of it even if the name is
     // invalid.
-    SemIR::Interface interface_info = {
-        name_context.MakeEntityWithParamsBase(interface_decl_id, name)};
+    SemIR::Interface interface_info = {name_context.MakeEntityWithParamsBase(
+        name, interface_decl_id, /*is_extern=*/false)};
     interface_info.generic_id = FinishGenericDecl(context, interface_decl_id);
     interface_decl.interface_id = context.interfaces().Add(interface_info);
     if (interface_info.has_parameters()) {

toolchain/check/impl.cpp

@@ -25,7 +25,8 @@ static auto NoteAssociatedFunction(Context& context,
   CARBON_DIAGNOSTIC(ImplAssociatedFunctionHere, Note,
                     "Associated function {0} declared here.", SemIR::NameId);
   const auto& function = context.functions().Get(function_id);
-  builder.Note(function.decl_id, ImplAssociatedFunctionHere, function.name_id);
+  builder.Note(function.latest_decl_id(), ImplAssociatedFunctionHere,
+               function.name_id);
 }
 
 // Gets the self specific of a generic declaration that is an interface member,

toolchain/check/import_ref.cpp

@@ -838,6 +838,7 @@ class ImportRefResolver {
   // of the parameter lists that we know whether this interface is a generic
   // interface and can build the right constant value for it.
   //
+  // TODO: Support extern.
   // TODO: Add a better way to represent a generic prior to importing the
   // parameters.
   auto GetIncompleteLocalEntityBase(
@@ -855,7 +856,10 @@ class ImportRefResolver {
         .param_refs_id = import_base.param_refs_id.is_valid()
                              ? SemIR::InstBlockId::Empty
                              : SemIR::InstBlockId::Invalid,
-        .decl_id = decl_id,
+        .is_extern = import_base.is_extern,
+        .extern_library_id = StringLiteralValueId::Invalid,
+        .non_owning_decl_id = SemIR::InstId::Invalid,
+        .first_owning_decl_id = decl_id,
     };
   }
 
@@ -1176,8 +1180,9 @@ class ImportRefResolver {
     SemIR::ClassDecl class_decl = {.type_id = SemIR::TypeId::TypeType,
                                    .class_id = SemIR::ClassId::Invalid,
                                    .decl_block_id = SemIR::InstBlockId::Empty};
-    auto class_decl_id = context_.AddPlaceholderInstInNoBlock(
-        SemIR::LocIdAndInst(AddImportIRInst(import_class.decl_id), class_decl));
+    auto class_decl_id =
+        context_.AddPlaceholderInstInNoBlock(SemIR::LocIdAndInst(
+            AddImportIRInst(import_class.latest_decl_id()), class_decl));
     // 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(
@@ -1200,13 +1205,14 @@ class ImportRefResolver {
                           SemIR::Class& new_class,
                           SemIR::ConstantId object_repr_const_id,
                           SemIR::InstId base_id) -> void {
-    new_class.definition_id = new_class.decl_id;
+    new_class.definition_id = new_class.first_owning_decl_id;
 
     new_class.object_repr_id =
         context_.GetTypeIdForTypeConstant(object_repr_const_id);
 
     new_class.scope_id = context_.name_scopes().Add(
-        new_class.decl_id, SemIR::NameId::Invalid, new_class.parent_scope_id);
+        new_class.first_owning_decl_id, SemIR::NameId::Invalid,
+        new_class.parent_scope_id);
     auto& new_scope = context_.name_scopes().Get(new_class.scope_id);
     const auto& import_scope =
         import_ir_.name_scopes().Get(import_class.scope_id);
@@ -1301,8 +1307,8 @@ class ImportRefResolver {
 
   auto TryResolveTypedInst(SemIR::ClassType inst) -> ResolveResult {
     CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
-    auto class_const_id =
-        GetLocalConstantId(import_ir_.classes().Get(inst.class_id).decl_id);
+    auto class_const_id = GetLocalConstantId(
+        import_ir_.classes().Get(inst.class_id).first_owning_decl_id);
     auto specific_data = GetLocalSpecificData(inst.specific_id);
     if (HasNewWork()) {
       return Retry();
@@ -1373,7 +1379,6 @@ class ImportRefResolver {
     function_decl.function_id = context_.functions().Add(
         {GetIncompleteLocalEntityBase(function_decl_id, import_function),
          {.return_storage_id = SemIR::InstId::Invalid,
-          .is_extern = import_function.is_extern,
           .builtin_function_kind = import_function.builtin_function_kind}});
 
     function_decl.type_id =
@@ -1457,7 +1462,7 @@ class ImportRefResolver {
     }
 
     if (import_function.definition_id.is_valid()) {
-      new_function.definition_id = new_function.decl_id;
+      new_function.definition_id = new_function.first_owning_decl_id;
     }
 
     return ResolveAsConstant(function_const_id);
@@ -1466,7 +1471,7 @@ class ImportRefResolver {
   auto TryResolveTypedInst(SemIR::FunctionType inst) -> ResolveResult {
     CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
     auto fn_val_id = GetLocalConstantInstId(
-        import_ir_.functions().Get(inst.function_id).decl_id);
+        import_ir_.functions().Get(inst.function_id).first_owning_decl_id);
     auto specific_data = GetLocalSpecificData(inst.specific_id);
     if (HasNewWork()) {
       return Retry();
@@ -1483,8 +1488,8 @@ class ImportRefResolver {
 
   auto TryResolveTypedInst(SemIR::GenericClassType inst) -> ResolveResult {
     CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
-    auto class_val_id =
-        GetLocalConstantInstId(import_ir_.classes().Get(inst.class_id).decl_id);
+    auto class_val_id = GetLocalConstantInstId(
+        import_ir_.classes().Get(inst.class_id).first_owning_decl_id);
     if (HasNewWork()) {
       return Retry();
     }
@@ -1498,7 +1503,7 @@ class ImportRefResolver {
   auto TryResolveTypedInst(SemIR::GenericInterfaceType inst) -> ResolveResult {
     CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
     auto interface_val_id = GetLocalConstantInstId(
-        import_ir_.interfaces().Get(inst.interface_id).decl_id);
+        import_ir_.interfaces().Get(inst.interface_id).first_owning_decl_id);
     if (HasNewWork()) {
       return Retry();
     }
@@ -1537,7 +1542,8 @@ class ImportRefResolver {
         .decl_block_id = SemIR::InstBlockId::Empty};
     auto interface_decl_id =
         context_.AddPlaceholderInstInNoBlock(SemIR::LocIdAndInst(
-            AddImportIRInst(import_interface.decl_id), interface_decl));
+            AddImportIRInst(import_interface.first_owning_decl_id),
+            interface_decl));
 
     // Start with an incomplete interface.
     interface_decl.interface_id = context_.interfaces().Add(
@@ -1561,7 +1567,7 @@ class ImportRefResolver {
                               SemIR::Interface& new_interface,
                               SemIR::InstId self_param_id) -> void {
     new_interface.scope_id = context_.name_scopes().Add(
-        new_interface.decl_id, SemIR::NameId::Invalid,
+        new_interface.first_owning_decl_id, SemIR::NameId::Invalid,
         new_interface.parent_scope_id);
     auto& new_scope = context_.name_scopes().Get(new_interface.scope_id);
     const auto& import_scope =
@@ -1647,7 +1653,7 @@ class ImportRefResolver {
   auto TryResolveTypedInst(SemIR::InterfaceType inst) -> ResolveResult {
     CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType);
     auto interface_const_id = GetLocalConstantId(
-        import_ir_.interfaces().Get(inst.interface_id).decl_id);
+        import_ir_.interfaces().Get(inst.interface_id).first_owning_decl_id);
     auto specific_data = GetLocalSpecificData(inst.specific_id);
     if (HasNewWork()) {
       return Retry();

toolchain/check/merge.h

@@ -45,7 +45,7 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id,
 // different kinds of entity such as classes and functions.
 struct DeclParams {
   explicit DeclParams(const SemIR::EntityWithParamsBase& base)
-      : loc(base.decl_id),
+      : loc(base.latest_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),

toolchain/check/return.cpp

@@ -33,7 +33,7 @@ static auto NoteNoReturnTypeProvided(Context::DiagnosticBuilder& diag,
                                      const SemIR::Function& function) {
   CARBON_DIAGNOSTIC(ReturnTypeOmittedNote, Note,
                     "There was no return type provided.");
-  diag.Note(function.decl_id, ReturnTypeOmittedNote);
+  diag.Note(function.latest_decl_id(), ReturnTypeOmittedNote);
 }
 
 // Produces a note describing the return type of the given function.

toolchain/sem_ir/entity_with_params_base.h

@@ -51,7 +51,13 @@ struct EntityWithParamsBase {
 
   // Returns the instruction for the latest declaration.
   auto latest_decl_id() const -> SemIR::InstId {
-    return definition_id.is_valid() ? definition_id : decl_id;
+    if (definition_id.is_valid()) {
+      return definition_id;
+    }
+    if (first_owning_decl_id.is_valid()) {
+      return first_owning_decl_id;
+    }
+    return non_owning_decl_id;
   }
 
   // Determines whether this entity has any parameter lists.
@@ -77,12 +83,21 @@ struct EntityWithParamsBase {
   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;
+  // True if declarations are `extern`.
+  bool is_extern;
+  // For an `extern library` declaration, the library name.
+  StringLiteralValueId extern_library_id;
+  // The non-owning declaration of the entity, if present. This will be a
+  // <entity>Decl.
+  InstId non_owning_decl_id;
+  // The first owning declaration of the entity, if present. This will be a
+  // <entity>Decl. It may either be a forward declaration, or the same as
+  // `definition_id`.
+  InstId first_owning_decl_id;
 
   // The following members are set at the `{` of the definition.
 
-  // The first declaration of the entity. This will be a <entity>Decl.
+  // The definition of the entity. This will be a <entity>Decl.
   InstId definition_id = InstId::Invalid;
 };
 

toolchain/sem_ir/function.h

@@ -22,8 +22,6 @@ struct FunctionFields {
   // 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.