Fix indirect import handling for functions. (#4258)

The particular test this focused on is indirect_two_file in
toolchain/check/testdata/function/definition/no_prelude/extern_library.carbon.

This removes `parent_scope_id_for_new_inst` because I think it's
returning unhelpful results. The use was at the root of incorrect
results for the indirect import chain. `name_id_for_new_inst` is
actually wrapping a union, so it's more important.

The merging of `is_extern` and `first_owning_decl_id` in
`handle_function.cpp` feels like it's less correct with the changes
that've been made to `extern`. This ripples in tests, because the error
recovery shifts.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

toolchain/check/decl_name_stack.h

@@ -96,7 +96,7 @@ class DeclNameStack {
         -> SemIR::EntityWithParamsBase {
       return {
           .name_id = name_id_for_new_inst(),
-          .parent_scope_id = parent_scope_id_for_new_inst(),
+          .parent_scope_id = parent_scope_id,
           .generic_id = SemIR::GenericId::Invalid,
           .first_param_node_id = name.first_param_node_id,
           .last_param_node_id = name.last_param_node_id,
@@ -121,13 +121,6 @@ class DeclNameStack {
                                         : SemIR::NameId::Invalid;
     }
 
-    // Returns the parent_scope_id for a new instruction. This is invalid
-    // when the name resolved.
-    auto parent_scope_id_for_new_inst() -> SemIR::NameScopeId {
-      return state == State::Unresolved ? parent_scope_id
-                                        : SemIR::NameScopeId::Invalid;
-    }
-
     // The current scope when this name began. This is the scope that we will
     // return to at the end of the declaration.
     ScopeIndex initial_scope_index;

toolchain/check/handle_alias.cpp

@@ -36,7 +36,7 @@ auto HandleParseNode(Context& context, Parse::AliasId /*node_id*/) -> bool {
 
   auto entity_name_id = context.entity_names().Add(
       {.name_id = name_context.name_id_for_new_inst(),
-       .parent_scope_id = name_context.parent_scope_id_for_new_inst(),
+       .parent_scope_id = name_context.parent_scope_id,
        .bind_index = SemIR::CompileTimeBindIndex::Invalid});
 
   auto alias_type_id = SemIR::TypeId::Invalid;

toolchain/check/handle_class.cpp

@@ -101,7 +101,7 @@ static auto MergeClassRedecl(Context& context, SemIRLoc new_loc,
       (prev_class.is_extern && !new_class.is_extern)) {
     prev_class.first_owning_decl_id = new_class.first_owning_decl_id;
     ReplacePrevInstForMerge(
-        context, prev_class.parent_scope_id, prev_class.name_id,
+        context, new_class.parent_scope_id, prev_class.name_id,
         new_is_import ? new_loc.inst_id : new_class.first_owning_decl_id);
   }
   return true;

toolchain/check/handle_function.cpp

@@ -10,6 +10,7 @@
 #include "toolchain/check/function.h"
 #include "toolchain/check/generic.h"
 #include "toolchain/check/handle.h"
+#include "toolchain/check/import_ref.h"
 #include "toolchain/check/interface.h"
 #include "toolchain/check/merge.h"
 #include "toolchain/check/modifiers.h"
@@ -87,18 +88,17 @@ static auto MergeFunctionRedecl(Context& context, SemIRLoc new_loc,
                                   prev_function.definition_id.is_valid()),
                        prev_import_ir_id);
 
+  if (!prev_function.first_owning_decl_id.is_valid()) {
+    prev_function.first_owning_decl_id = new_function.first_owning_decl_id;
+  }
   if (new_is_definition) {
     // Track the signature from the definition, so that IDs in the body
     // match IDs in the signature.
     prev_function.MergeDefinition(new_function);
     prev_function.return_storage_id = new_function.return_storage_id;
   }
-  // The new function might have return slot information if it was imported.
-  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.first_owning_decl_id = new_function.first_owning_decl_id;
-    ReplacePrevInstForMerge(context, prev_function.parent_scope_id,
+  if ((prev_import_ir_id.is_valid() && !new_is_import)) {
+    ReplacePrevInstForMerge(context, new_function.parent_scope_id,
                             prev_function.name_id,
                             new_function.first_owning_decl_id);
   }
@@ -122,9 +122,9 @@ static auto TryMergeRedecl(Context& context, Parse::AnyFunctionDeclId node_id,
       prev_function_id = function_decl.function_id;
       break;
     }
-    case CARBON_KIND(SemIR::ImportRefLoaded import_ref): {
+    case SemIR::ImportRefLoaded::Kind: {
       auto import_ir_inst =
-          context.import_ir_insts().Get(import_ref.import_ir_inst_id);
+          GetCanonicalImportIRInst(context, &context.sem_ir(), prev_id);
 
       // Verify the decl so that things like aliases are name conflicts.
       const auto* import_ir =
@@ -226,6 +226,9 @@ static auto BuildFunctionDecl(Context& context,
 
   // Create a new function if this isn't a valid redeclaration.
   if (!function_decl.function_id.is_valid()) {
+    if (function_info.is_extern && context.IsImplFile()) {
+      DiagnoseExternRequiresDeclInApiFile(context, node_id);
+    }
     function_info.generic_id = FinishGenericDecl(context, decl_id);
     function_decl.function_id = context.functions().Add(function_info);
   } else {

toolchain/check/handle_namespace.cpp

@@ -70,7 +70,7 @@ auto HandleParseNode(Context& context, Parse::NamespaceId node_id) -> bool {
   if (!namespace_inst.name_scope_id.is_valid()) {
     namespace_inst.name_scope_id = context.name_scopes().Add(
         namespace_id, name_context.name_id_for_new_inst(),
-        name_context.parent_scope_id_for_new_inst());
+        name_context.parent_scope_id);
   }
 
   context.ReplaceInstBeforeConstantUse(namespace_id, namespace_inst);

toolchain/check/merge.cpp

@@ -16,7 +16,7 @@ CARBON_DIAGNOSTIC(RedeclPrevDecl, Note, "Previously declared here.");
 // Diagnoses a redeclaration which is redundant.
 static auto DiagnoseRedundant(Context& context, Lex::TokenKind decl_kind,
                               SemIR::NameId name_id, SemIRLoc new_loc,
-                              SemIRLoc prev_loc) {
+                              SemIRLoc prev_loc) -> void {
   CARBON_DIAGNOSTIC(RedeclRedundant, Error,
                     "Redeclaration of `{0} {1}` is redundant.", Lex::TokenKind,
                     SemIR::NameId);
@@ -29,7 +29,7 @@ static auto DiagnoseRedundant(Context& context, Lex::TokenKind decl_kind,
 // Diagnoses a redefinition.
 static auto DiagnoseRedef(Context& context, Lex::TokenKind decl_kind,
                           SemIR::NameId name_id, SemIRLoc new_loc,
-                          SemIRLoc prev_loc) {
+                          SemIRLoc prev_loc) -> void {
   CARBON_DIAGNOSTIC(RedeclRedef, Error, "Redefinition of `{0} {1}`.",
                     Lex::TokenKind, SemIR::NameId);
   CARBON_DIAGNOSTIC(RedeclPrevDef, Note, "Previously defined here.");
@@ -42,7 +42,7 @@ static auto DiagnoseRedef(Context& context, Lex::TokenKind decl_kind,
 // Diagnoses an `extern` versus non-`extern` mismatch.
 static auto DiagnoseExternMismatch(Context& context, Lex::TokenKind decl_kind,
                                    SemIR::NameId name_id, SemIRLoc new_loc,
-                                   SemIRLoc prev_loc) {
+                                   SemIRLoc prev_loc) -> void {
   CARBON_DIAGNOSTIC(RedeclExternMismatch, Error,
                     "Redeclarations of `{0} {1}` must match use of `extern`.",
                     Lex::TokenKind, SemIR::NameId);
@@ -56,8 +56,8 @@ static auto DiagnoseExternMismatch(Context& context, Lex::TokenKind decl_kind,
 static auto DiagnoseExternLibraryInImporter(Context& context,
                                             Lex::TokenKind decl_kind,
                                             SemIR::NameId name_id,
-                                            SemIRLoc new_loc,
-                                            SemIRLoc prev_loc) {
+                                            SemIRLoc new_loc, SemIRLoc prev_loc)
+    -> void {
   CARBON_DIAGNOSTIC(ExternLibraryInImporter, Error,
                     "Cannot declare imported `{0} {1}` as `extern library`.",
                     Lex::TokenKind, SemIR::NameId);
@@ -69,7 +69,7 @@ static auto DiagnoseExternLibraryInImporter(Context& context,
 
 // Diagnoses `extern library` pointing to the wrong library.
 static auto DiagnoseExternLibraryIncorrect(Context& context, SemIRLoc new_loc,
-                                           SemIRLoc prev_loc) {
+                                           SemIRLoc prev_loc) -> void {
   CARBON_DIAGNOSTIC(
       ExternLibraryIncorrect, Error,
       "Declaration in {0} doesn't match `extern library` declaration.",
@@ -82,6 +82,14 @@ static auto DiagnoseExternLibraryIncorrect(Context& context, SemIRLoc new_loc,
       .Emit();
 }
 
+auto DiagnoseExternRequiresDeclInApiFile(Context& context, SemIRLoc loc)
+    -> void {
+  CARBON_DIAGNOSTIC(
+      ExternRequiresDeclInApiFile, Error,
+      "`extern` entities must have a declaration in the API file.");
+  context.emitter().Build(loc, ExternRequiresDeclInApiFile).Emit();
+}
+
 // Checks to see if a structurally valid redeclaration is allowed in context.
 // These all still merge.
 auto CheckIsAllowedRedecl(Context& context, Lex::TokenKind decl_kind,
@@ -128,6 +136,12 @@ auto CheckIsAllowedRedecl(Context& context, Lex::TokenKind decl_kind,
   }
 
   // Check for disallowed redeclarations cross-library.
+  if (new_decl.is_extern && context.IsImplFile()) {
+    // We continue after issuing the "missing API declaration" diagnostic,
+    // because it may still be helpful to note other issues with the
+    // declarations.
+    DiagnoseExternRequiresDeclInApiFile(context, new_decl.loc);
+  }
   if (prev_decl.is_extern != new_decl.is_extern) {
     DiagnoseExternMismatch(context, decl_kind, name_id, new_decl.loc,
                            prev_decl.loc);
@@ -146,6 +160,7 @@ auto CheckIsAllowedRedecl(Context& context, Lex::TokenKind decl_kind,
   if (prev_decl.extern_library_id != SemIR::LibraryNameId::Error &&
       prev_decl.extern_library_id != context.sem_ir().library_id()) {
     DiagnoseExternLibraryIncorrect(context, new_decl.loc, prev_decl.loc);
+    return;
   }
 }
 

toolchain/check/merge.h

@@ -11,6 +11,11 @@
 
 namespace Carbon::Check {
 
+// Diagnoses an `extern` declaration that was not preceded by a declaration in
+// the API file.
+auto DiagnoseExternRequiresDeclInApiFile(Context& context, SemIRLoc loc)
+    -> void;
+
 // Information on new and previous declarations for CheckIsAllowedRedecl.
 struct RedeclInfo {
   explicit RedeclInfo(SemIR::EntityWithParamsBase params, SemIRLoc loc,

toolchain/check/testdata/function/declaration/import.carbon

@@ -724,17 +724,17 @@ import library "extern_api";
 // CHECK:STDOUT:   %e: ref %.1 = bind_name e, %e.var
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @A();
+// CHECK:STDOUT: fn @A();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @B(%b: i32) -> i32;
+// CHECK:STDOUT: fn @B(%b: i32) -> i32;
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @C(%c: %.3) -> %.4;
+// CHECK:STDOUT: fn @C(%c: %.3) -> %.4;
 // CHECK:STDOUT:
 // CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @E();
+// CHECK:STDOUT: fn @E();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:

toolchain/check/testdata/function/declaration/no_prelude/extern.carbon

@@ -143,13 +143,13 @@ class C {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = %F.decl.loc12
+// CHECK:STDOUT:     .F = %F.decl.loc4
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.decl.loc4: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT:   %F.decl.loc12: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT: extern fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_member_extern.carbon
 // CHECK:STDOUT:

toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon

@@ -234,7 +234,7 @@ extern library "extern_of_import" fn F();
 // CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT: extern fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_extern_library_redecl.carbon
 // CHECK:STDOUT:
@@ -377,5 +377,5 @@ extern library "extern_of_import" fn F();
 // CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @F();
+// CHECK:STDOUT: fn @F();
 // CHECK:STDOUT:

toolchain/check/testdata/function/declaration/no_prelude/implicit_import.carbon

@@ -148,7 +148,7 @@ extern fn A();
 // CHECK:STDOUT:   %A.decl: %A.type = fn_decl @A [template = constants.%A] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A();
+// CHECK:STDOUT: extern fn @A();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- extern_impl.carbon
 // CHECK:STDOUT:
@@ -188,5 +188,5 @@ extern fn A();
 // CHECK:STDOUT:   %A.decl: %A.type = fn_decl @A [template = constants.%A] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @A();
+// CHECK:STDOUT: fn @A();
 // CHECK:STDOUT:

toolchain/check/testdata/function/definition/import.carbon

@@ -88,6 +88,16 @@ import library "extern";
 // CHECK:STDERR:
 fn A();
 
+// CHECK:STDERR: fail_redecl_then_def.carbon:[[@LINE+10]]:1: ERROR: Redeclarations of `fn A` must match use of `extern`.
+// CHECK:STDERR: fn A() {}
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: fail_redecl_then_def.carbon:[[@LINE-17]]:1: In import.
+// CHECK:STDERR: import library "extern";
+// CHECK:STDERR: ^~~~~~
+// CHECK:STDERR: extern.carbon:4:1: Previously declared here.
+// CHECK:STDERR: extern fn A();
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR:
 fn A() {}
 
 // --- fail_mix_extern_decl.carbon
@@ -96,7 +106,7 @@ library "mix_extern_decl";
 
 import library "fns";
 
-// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+10]]:1: ERROR: Redeclarations of `fn D` must match use of `extern`.
+// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+9]]:1: ERROR: Redeclarations of `fn D` must match use of `extern`.
 // CHECK:STDERR: extern fn D();
 // CHECK:STDERR: ^~~~~~~~~~~~~~
 // CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-5]]:1: In import.
@@ -105,15 +115,8 @@ import library "fns";
 // CHECK:STDERR: fns.carbon:7:1: Previously declared here.
 // CHECK:STDERR: fn D();
 // CHECK:STDERR: ^~~~~~~
-// CHECK:STDERR:
 extern fn D();
 
-// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+6]]:1: ERROR: Redeclarations of `fn D` must match use of `extern`.
-// CHECK:STDERR: fn D() {}
-// CHECK:STDERR: ^~~~~~~~
-// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-5]]:1: Previously declared here.
-// CHECK:STDERR: extern fn D();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
 fn D() {}
 
 // CHECK:STDOUT: --- fns.carbon
@@ -430,10 +433,10 @@ fn D() {}
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %default.import = import <invalid>
 // CHECK:STDOUT:   %A.decl.loc16: %A.type = fn_decl @A [template = constants.%A] {}
-// CHECK:STDOUT:   %A.decl.loc18: %A.type = fn_decl @A [template = constants.%A] {}
+// CHECK:STDOUT:   %A.decl.loc28: %A.type = fn_decl @A [template = constants.%A] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A() {
+// CHECK:STDOUT: extern fn @A() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -467,13 +470,13 @@ fn D() {}
 // CHECK:STDOUT:     .A = imports.%import_ref.1
 // CHECK:STDOUT:     .B = imports.%import_ref.2
 // CHECK:STDOUT:     .C = imports.%import_ref.3
-// CHECK:STDOUT:     .D = %D.decl.loc24
+// CHECK:STDOUT:     .D = %D.decl.loc15
 // CHECK:STDOUT:     .Core = imports.%Core
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %default.import = import <invalid>
-// CHECK:STDOUT:   %D.decl.loc16: %D.type = fn_decl @D [template = constants.%D] {}
-// CHECK:STDOUT:   %D.decl.loc24: %D.type = fn_decl @D [template = constants.%D] {}
+// CHECK:STDOUT:   %D.decl.loc15: %D.type = fn_decl @D [template = constants.%D] {}
+// CHECK:STDOUT:   %D.decl.loc17: %D.type = fn_decl @D [template = constants.%D] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @D() {

toolchain/check/testdata/function/definition/no_prelude/extern.carbon

@@ -74,6 +74,13 @@ extern fn F();
 // CHECK:STDERR: ^~~~~~~~~~~~~~
 // CHECK:STDERR:
 fn F();
+// CHECK:STDERR: fail_extern_diag_suppressed.carbon:[[@LINE+7]]:1: ERROR: Redeclarations of `fn F` must match use of `extern`.
+// CHECK:STDERR: fn F() {}
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: fail_extern_diag_suppressed.carbon:[[@LINE-12]]:1: Previously declared here.
+// CHECK:STDERR: extern fn F();
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR:
 fn F() {}
 
 // --- fail_extern_decl_after_def.carbon
@@ -81,14 +88,28 @@ fn F() {}
 library "extern_decl_after_def";
 
 fn F() {}
-// CHECK:STDERR: fail_extern_decl_after_def.carbon:[[@LINE+6]]:1: ERROR: Redeclaration of `fn F` is redundant.
+// CHECK:STDERR: fail_extern_decl_after_def.carbon:[[@LINE+7]]:1: ERROR: Redeclaration of `fn F` is redundant.
 // CHECK:STDERR: extern fn F();
 // CHECK:STDERR: ^~~~~~~~~~~~~~
 // CHECK:STDERR: fail_extern_decl_after_def.carbon:[[@LINE-4]]:1: Previously declared here.
 // CHECK:STDERR: fn F() {}
 // CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR:
 extern fn F();
 
+// --- in_impl.carbon
+
+library "in_impl";
+
+// --- fail_in_impl.impl.carbon
+
+impl library "in_impl";
+
+// CHECK:STDERR: fail_in_impl.impl.carbon:[[@LINE+3]]:1: ERROR: `extern` entities must have a declaration in the API file.
+// CHECK:STDERR: extern fn F() {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~
+extern fn F() {}
+
 // CHECK:STDOUT: --- extern_def.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -183,13 +204,13 @@ extern fn F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = %F.decl.loc12
+// CHECK:STDOUT:     .F = %F.decl.loc4
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.decl.loc4: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT:   %F.decl.loc12: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: extern fn @F() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -225,14 +246,14 @@ extern fn F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = %F.decl.loc12
+// CHECK:STDOUT:     .F = %F.decl.loc4
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.decl.loc4: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT:   %F.decl.loc12: %F.type = fn_decl @F [template = constants.%F] {}
-// CHECK:STDOUT:   %F.decl.loc13: %F.type = fn_decl @F [template = constants.%F] {}
+// CHECK:STDOUT:   %F.decl.loc20: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: extern fn @F() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -250,7 +271,7 @@ extern fn F();
 // CHECK:STDOUT:     .F = %F.decl.loc4
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.decl.loc4: %F.type = fn_decl @F [template = constants.%F] {}
-// CHECK:STDOUT:   %F.decl.loc11: %F.type = fn_decl @F [template = constants.%F] {}
+// CHECK:STDOUT:   %F.decl.loc12: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F() {
@@ -258,3 +279,31 @@ extern fn F();
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: --- in_impl.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_in_impl.impl.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %F.type: type = fn_type @F [template]
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT:   %F: %F.type = struct_value () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .F = %F.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %default.import.loc2_6.1 = import <invalid>
+// CHECK:STDOUT:   %default.import.loc2_6.2 = import <invalid>
+// CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: extern fn @F() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

toolchain/check/testdata/function/definition/no_prelude/extern_library.carbon

@@ -78,22 +78,14 @@ library "indirect_two_file";
 
 import library "indirect_two_file_extern";
 
-// --- fail_todo_indirect_two_file.impl.carbon
+// --- fail_indirect_two_file.impl.carbon
 
 impl library "indirect_two_file";
 
-// CHECK:STDERR: fail_todo_indirect_two_file.impl.carbon:[[@LINE+12]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fail_indirect_two_file.impl.carbon:[[@LINE+4]]:1: ERROR: `extern` entities must have a declaration in the API file.
 // CHECK:STDERR: extern fn F() {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_indirect_two_file.impl.carbon:[[@LINE-5]]:6: In import.
-// CHECK:STDERR: impl library "indirect_two_file";
-// CHECK:STDERR:      ^~~~~~~
-// CHECK:STDERR: indirect_two_file.carbon:4:1: In import.
-// CHECK:STDERR: import library "indirect_two_file_extern";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: indirect_two_file_extern.carbon:4:1: Name is previously declared here.
-// CHECK:STDERR: extern library "indirect_two_file" fn F();
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:
 extern fn F() {}
 
 // --- in_impl_extern.carbon
@@ -106,12 +98,15 @@ extern library "in_impl" fn F();
 
 library "in_impl";
 
-// --- in_impl.impl.carbon
+// --- fail_in_impl.impl.carbon
 
 impl library "in_impl";
 
 import library "in_impl_extern";
 
+// CHECK:STDERR: fail_in_impl.impl.carbon:[[@LINE+3]]:1: ERROR: `extern` entities must have a declaration in the API file.
+// CHECK:STDERR: extern fn F() {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~~
 extern fn F() {}
 
 // CHECK:STDOUT: --- one_file_extern.carbon
@@ -266,7 +261,7 @@ extern fn F() {}
 // CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: extern fn @F() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -301,14 +296,12 @@ extern fn F() {}
 // CHECK:STDOUT:   %default.import = import <invalid>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_indirect_two_file.impl.carbon
+// CHECK:STDOUT: --- fail_indirect_two_file.impl.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %F.type: type = fn_type @F [template]
 // CHECK:STDOUT:   %.1: type = tuple_type () [template]
 // CHECK:STDOUT:   %F: %F.type = struct_value () [template]
-// CHECK:STDOUT:   %.type: type = fn_type @.1 [template]
-// CHECK:STDOUT:   %.2: %.type = struct_value () [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
@@ -317,16 +310,14 @@ extern fn F() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = imports.%import_ref
+// CHECK:STDOUT:     .F = %F.decl
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %default.import.loc2_6.1 = import <invalid>
 // CHECK:STDOUT:   %default.import.loc2_6.2 = import <invalid>
-// CHECK:STDOUT:   %.decl: %.type = fn_decl @.1 [template = constants.%.2] {}
+// CHECK:STDOUT:   %F.decl: %F.type = fn_decl @F [template = constants.%F] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @F();
-// CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @.1() {
+// CHECK:STDOUT: extern fn @F() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -354,7 +345,7 @@ extern fn F() {}
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- in_impl.impl.carbon
+// CHECK:STDOUT: --- fail_in_impl.impl.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %F.type: type = fn_type @F [template]

toolchain/check/testdata/function/definition/no_prelude/implicit_import.carbon

@@ -211,7 +211,7 @@ fn B() {}
 // CHECK:STDOUT:   %A.decl: %A.type = fn_decl @A [template = constants.%A] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A() {
+// CHECK:STDOUT: extern fn @A() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -254,7 +254,7 @@ fn B() {}
 // CHECK:STDOUT:   %A.decl: %A.type = fn_decl @A [template = constants.%A] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: extern fn @A() {
+// CHECK:STDOUT: fn @A() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }

toolchain/diagnostics/diagnostic_kind.def

@@ -169,6 +169,7 @@ CARBON_DIAGNOSTIC_KIND(RedeclParamSyntaxPrevious)
 CARBON_DIAGNOSTIC_KIND(ExternLibraryInImporter)
 CARBON_DIAGNOSTIC_KIND(ExternLibraryIncorrect)
 CARBON_DIAGNOSTIC_KIND(ExternLibraryExpected)
+CARBON_DIAGNOSTIC_KIND(ExternRequiresDeclInApiFile)
 
 // Function call checking.
 CARBON_DIAGNOSTIC_KIND(AddrSelfIsNonRef)