Address invalid redeclaration and definitions across imports. (#3842)

Note, there's still a TODO "Allow non-extern declarations in the same
library."

toolchain/check/function.cpp

@@ -4,6 +4,7 @@
 
 #include "toolchain/check/function.h"
 
+#include "toolchain/check/merge.h"
 #include "toolchain/check/subst.h"
 
 namespace Carbon::Check {
@@ -251,10 +252,9 @@ static auto CheckIsAllowedRedecl(Context& context, SemIR::LocId loc_id,
   }
 }
 
-// TODO: Detect conflicting cross-file declarations, as well as uses of imported
-// declarations followed by a redeclaration.
 auto MergeFunctionRedecl(Context& context, SemIR::LocId loc_id,
-                         SemIR::Function& new_function, bool new_is_definition,
+                         SemIR::Function& new_function, bool new_is_import,
+                         bool new_is_definition,
                          SemIR::FunctionId prev_function_id,
                          bool prev_is_import) -> bool {
   auto& prev_function = context.functions().Get(prev_function_id);
@@ -275,8 +275,12 @@ auto MergeFunctionRedecl(Context& context, SemIR::LocId loc_id,
     prev_function.return_type_id = new_function.return_type_id;
     prev_function.return_slot_id = new_function.return_slot_id;
   }
-  if (!new_function.is_extern) {
-    prev_function.is_extern = false;
+  if ((prev_is_import && !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;
+    ReplacePrevInstForMerge(context, prev_function.enclosing_scope_id,
+                            prev_function.name_id, new_function.decl_id);
   }
   return true;
 }

toolchain/check/function.h

@@ -43,7 +43,8 @@ auto CheckFunctionTypeMatches(Context& context,
 // If merging is successful, updates the FunctionId on new_function and returns
 // true. Otherwise, returns false. Prints a diagnostic when appropriate.
 auto MergeFunctionRedecl(Context& context, SemIR::LocId loc_id,
-                         SemIR::Function& new_function, bool new_is_definition,
+                         SemIR::Function& new_function, bool new_is_import,
+                         bool new_is_definition,
                          SemIR::FunctionId prev_function_id,
                          bool prev_is_imported) -> bool;
 

toolchain/check/handle_function.cpp

@@ -173,7 +173,8 @@ static auto BuildFunctionDecl(Context& context,
 
     if (auto existing_function_decl =
             prev_inst.inst.TryAs<SemIR::FunctionDecl>()) {
-      if (MergeFunctionRedecl(context, node_id, function_info, is_definition,
+      if (MergeFunctionRedecl(context, node_id, function_info,
+                              /*new_is_import=*/false, is_definition,
                               existing_function_decl->function_id,
                               prev_inst.is_import)) {
         // When merging, use the existing function rather than adding a new one.

toolchain/check/merge.cpp

@@ -67,6 +67,16 @@ static auto ResolveMergeableInst(Context& context, SemIR::InstId inst_id)
   return context.insts().Get(const_id.inst_id());
 }
 
+auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id,
+                             SemIR::NameId name_id, SemIR::InstId new_inst_id)
+    -> void {
+  auto& names = context.name_scopes().Get(scope_id).names;
+  auto it = names.find(name_id);
+  if (it != names.end()) {
+    it->second = new_inst_id;
+  }
+}
+
 auto MergeImportRef(Context& context, SemIR::InstId new_inst_id,
                     SemIR::InstId prev_inst_id) -> void {
   auto new_inst = ResolveMergeableInst(context, new_inst_id);
@@ -93,6 +103,7 @@ auto MergeImportRef(Context& context, SemIR::InstId new_inst_id,
       // emitted, since it will already be added.
       MergeFunctionRedecl(context, context.insts().GetLocId(new_inst_id),
                           new_fn,
+                          /*new_is_import=*/true,
                           /*new_is_definition=*/false, prev_fn_id,
                           /*prev_is_imported=*/true);
       return;

toolchain/check/merge.h

@@ -24,6 +24,13 @@ struct PrevInstForMerge {
 auto ResolvePrevInstForMerge(Context& context, Parse::NodeId node_id,
                              SemIR::InstId prev_inst_id) -> PrevInstForMerge;
 
+// When the prior name lookup result is an import and we are successfully
+// merging, replace the name lookup result with the reference in the current
+// file.
+auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id,
+                             SemIR::NameId name_id, SemIR::InstId new_inst_id)
+    -> void;
+
 // Merges an import ref at new_inst_id another at prev_inst_id. May print a
 // diagnostic if merging is invalid.
 auto MergeImportRef(Context& context, SemIR::InstId new_inst_id,

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

@@ -79,7 +79,7 @@ class C {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = %F.loc4
+// CHECK:STDOUT:     .F = %F.loc12
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.loc4: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT:   %F.loc12: <function> = fn_decl @F [template] {}

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

@@ -383,26 +383,26 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .A = %import_ref.1
-// CHECK:STDOUT:     .B = %import_ref.2
-// CHECK:STDOUT:     .C = %import_ref.3
-// CHECK:STDOUT:     .D = %import_ref.4
 // CHECK:STDOUT:     .NS = %NS
+// CHECK:STDOUT:     .A = %A
+// CHECK:STDOUT:     .B = %B
+// CHECK:STDOUT:     .C = %C
+// CHECK:STDOUT:     .D = %D
 // CHECK:STDOUT:     .a = %a
 // CHECK:STDOUT:     .b = %b.loc13
 // CHECK:STDOUT:     .c = %c.loc14
 // CHECK:STDOUT:     .d = %d
 // CHECK:STDOUT:     .e = %e
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loc_65 [template = imports.%A]
-// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, loc_74 [template = imports.%B]
-// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, loc_89 [template = imports.%C]
-// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, loc_103 [template = imports.%D]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loaded [template = imports.%A]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, loaded [template = imports.%B]
+// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, loaded [template = imports.%C]
+// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, loaded [template = imports.%D]
 // CHECK:STDOUT:   %import_ref.5: <namespace> = import_ref ir1, inst+19, loaded
 // CHECK:STDOUT:   %NS: <namespace> = namespace %import_ref.5, [template] {
-// CHECK:STDOUT:     .E = %import_ref.6
+// CHECK:STDOUT:     .E = %E
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir1, inst+20, loc_115 [template = imports.%E]
+// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir1, inst+20, loaded [template = imports.%E]
 // CHECK:STDOUT:   %A: <function> = fn_decl @A [template] {}
 // CHECK:STDOUT:   %B: <function> = fn_decl @B [template] {
 // CHECK:STDOUT:     %b.loc7_13.1: i32 = param b
@@ -438,37 +438,37 @@ import library "extern_api";
 // CHECK:STDOUT:   %e: ref () = bind_name e, %e.var
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A();
+// CHECK:STDOUT: extern fn @A();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @B(%b: i32) -> i32;
+// CHECK:STDOUT: extern fn @B(%b: i32) -> i32;
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @C(%c: (i32,)) -> {.c: i32};
+// CHECK:STDOUT: extern fn @C(%c: (i32,)) -> {.c: i32};
 // CHECK:STDOUT:
 // CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @E();
+// CHECK:STDOUT: extern fn @E();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%import_ref.1 [template = imports.%A]
+// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%A [template = file.%A]
 // CHECK:STDOUT:   %.loc12: init () = call %A.ref()
 // CHECK:STDOUT:   assign file.%a.var, %.loc12
-// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%import_ref.2 [template = imports.%B]
+// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%B [template = file.%B]
 // CHECK:STDOUT:   %.loc13_16: i32 = int_literal 1 [template = constants.%.5]
 // CHECK:STDOUT:   %.loc13_15: init i32 = call %B.ref(%.loc13_16)
 // CHECK:STDOUT:   assign file.%b.var, %.loc13_15
-// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%import_ref.3 [template = imports.%C]
+// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%C [template = file.%C]
 // CHECK:STDOUT:   %.loc14_23: i32 = int_literal 1 [template = constants.%.5]
 // CHECK:STDOUT:   %.loc14_25.1: (i32,) = tuple_literal (%.loc14_23)
 // CHECK:STDOUT:   %.loc14_25.2: (i32,) = tuple_value (%.loc14_23) [template = constants.%.6]
 // CHECK:STDOUT:   %.loc14_25.3: (i32,) = converted %.loc14_25.1, %.loc14_25.2 [template = constants.%.6]
 // CHECK:STDOUT:   %.loc14_21: init {.c: i32} = call %C.ref(%.loc14_25.3)
 // CHECK:STDOUT:   assign file.%c.var, %.loc14_21
-// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%import_ref.4 [template = imports.%D]
+// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%D [template = file.%D]
 // CHECK:STDOUT:   %.loc15: init () = call %D.ref()
 // CHECK:STDOUT:   assign file.%d.var, %.loc15
 // CHECK:STDOUT:   %NS.ref: <namespace> = name_ref NS, file.%NS [template = file.%NS]
-// CHECK:STDOUT:   %E.ref: <function> = name_ref E, file.%import_ref.6 [template = imports.%E]
+// CHECK:STDOUT:   %E.ref: <function> = name_ref E, file.%E [template = file.%E]
 // CHECK:STDOUT:   %.loc16: init () = call %E.ref()
 // CHECK:STDOUT:   assign file.%e.var, %.loc16
 // CHECK:STDOUT:   return
@@ -487,26 +487,26 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .A = %import_ref.1
-// CHECK:STDOUT:     .B = %import_ref.2
-// CHECK:STDOUT:     .C = %import_ref.3
-// CHECK:STDOUT:     .D = %import_ref.4
 // CHECK:STDOUT:     .NS = %NS
+// CHECK:STDOUT:     .A = %A
+// CHECK:STDOUT:     .B = %B
+// CHECK:STDOUT:     .C = %C
+// CHECK:STDOUT:     .D = %D
 // CHECK:STDOUT:     .a = %a
 // CHECK:STDOUT:     .b = %b.loc13
 // CHECK:STDOUT:     .c = %c.loc14
 // CHECK:STDOUT:     .d = %d
 // CHECK:STDOUT:     .e = %e
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loc_65 [template = imports.%A]
-// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, loc_74 [template = imports.%B]
-// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, loc_89 [template = imports.%C]
-// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, loc_103 [template = imports.%D]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loaded [template = imports.%A]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, loaded [template = imports.%B]
+// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, loaded [template = imports.%C]
+// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, loaded [template = imports.%D]
 // CHECK:STDOUT:   %import_ref.5: <namespace> = import_ref ir1, inst+19, loaded
 // CHECK:STDOUT:   %NS: <namespace> = namespace %import_ref.5, [template] {
-// CHECK:STDOUT:     .E = %import_ref.6
+// CHECK:STDOUT:     .E = %E
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir1, inst+20, loc_115 [template = imports.%E]
+// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir1, inst+20, loaded [template = imports.%E]
 // CHECK:STDOUT:   %A: <function> = fn_decl @A [template] {}
 // CHECK:STDOUT:   %B: <function> = fn_decl @B [template] {
 // CHECK:STDOUT:     %b.loc7_13.1: i32 = param b
@@ -554,25 +554,25 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%import_ref.1 [template = imports.%A]
+// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%A [template = file.%A]
 // CHECK:STDOUT:   %.loc12: init () = call %A.ref()
 // CHECK:STDOUT:   assign file.%a.var, %.loc12
-// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%import_ref.2 [template = imports.%B]
+// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%B [template = file.%B]
 // CHECK:STDOUT:   %.loc13_16: i32 = int_literal 1 [template = constants.%.5]
 // CHECK:STDOUT:   %.loc13_15: init i32 = call %B.ref(%.loc13_16)
 // CHECK:STDOUT:   assign file.%b.var, %.loc13_15
-// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%import_ref.3 [template = imports.%C]
+// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%C [template = file.%C]
 // CHECK:STDOUT:   %.loc14_23: i32 = int_literal 1 [template = constants.%.5]
 // CHECK:STDOUT:   %.loc14_25.1: (i32,) = tuple_literal (%.loc14_23)
 // CHECK:STDOUT:   %.loc14_25.2: (i32,) = tuple_value (%.loc14_23) [template = constants.%.6]
 // CHECK:STDOUT:   %.loc14_25.3: (i32,) = converted %.loc14_25.1, %.loc14_25.2 [template = constants.%.6]
 // CHECK:STDOUT:   %.loc14_21: init {.c: i32} = call %C.ref(%.loc14_25.3)
 // CHECK:STDOUT:   assign file.%c.var, %.loc14_21
-// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%import_ref.4 [template = imports.%D]
+// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%D [template = file.%D]
 // CHECK:STDOUT:   %.loc15: init () = call %D.ref()
 // CHECK:STDOUT:   assign file.%d.var, %.loc15
 // CHECK:STDOUT:   %NS.ref: <namespace> = name_ref NS, file.%NS [template = file.%NS]
-// CHECK:STDOUT:   %E.ref: <function> = name_ref E, file.%import_ref.6 [template = imports.%E]
+// CHECK:STDOUT:   %E.ref: <function> = name_ref E, file.%E [template = file.%E]
 // CHECK:STDOUT:   %.loc16: init () = call %E.ref()
 // CHECK:STDOUT:   assign file.%e.var, %.loc16
 // CHECK:STDOUT:   return
@@ -692,26 +692,26 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .A = %import_ref.1
-// CHECK:STDOUT:     .B = %import_ref.2
-// CHECK:STDOUT:     .C = %import_ref.3
 // CHECK:STDOUT:     .D = %import_ref.4
 // CHECK:STDOUT:     .NS = %NS
+// CHECK:STDOUT:     .A = imports.%A.2
+// CHECK:STDOUT:     .B = imports.%B.2
+// CHECK:STDOUT:     .C = imports.%C.2
 // CHECK:STDOUT:     .a = %a
 // CHECK:STDOUT:     .b = %b
 // CHECK:STDOUT:     .c = %c
 // CHECK:STDOUT:     .d = %d
 // CHECK:STDOUT:     .e = %e
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loc_19 [template = imports.%A.1]
-// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, loc_28 [template = imports.%B.1]
-// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, loc_43 [template = imports.%C.1]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loaded [template = imports.%A.1]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, loaded [template = imports.%B.1]
+// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, loaded [template = imports.%C.1]
 // CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, loc_57 [template = imports.%D.1]
 // CHECK:STDOUT:   %import_ref.5: <namespace> = import_ref ir1, inst+19, loaded
 // CHECK:STDOUT:   %NS: <namespace> = namespace %import_ref.5, [template] {
-// CHECK:STDOUT:     .E = %import_ref.6
+// CHECK:STDOUT:     .E = imports.%E.2
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir1, inst+20, loc_69 [template = imports.%E.1]
+// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir1, inst+20, loaded [template = imports.%E.1]
 // CHECK:STDOUT:   %import_ref.7: <function> = import_ref ir2, inst+1, loaded [template = imports.%A.2]
 // CHECK:STDOUT:   %import_ref.8: <function> = import_ref ir2, inst+5, loaded [template = imports.%B.2]
 // CHECK:STDOUT:   %import_ref.9: <function> = import_ref ir2, inst+17, loaded [template = imports.%C.2]
@@ -758,14 +758,14 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%import_ref.1 [template = imports.%A.1]
+// CHECK:STDOUT:   %A.ref: <function> = name_ref A, imports.%A.2 [template = imports.%A.2]
 // CHECK:STDOUT:   %.loc7: init () = call %A.ref()
 // CHECK:STDOUT:   assign file.%a.var, %.loc7
-// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%import_ref.2 [template = imports.%B.1]
+// CHECK:STDOUT:   %B.ref: <function> = name_ref B, imports.%B.2 [template = imports.%B.2]
 // CHECK:STDOUT:   %.loc8_16: i32 = int_literal 1 [template = constants.%.4]
 // CHECK:STDOUT:   %.loc8_15: init i32 = call %B.ref(%.loc8_16)
 // CHECK:STDOUT:   assign file.%b.var, %.loc8_15
-// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%import_ref.3 [template = imports.%C.1]
+// CHECK:STDOUT:   %C.ref: <function> = name_ref C, imports.%C.2 [template = imports.%C.2]
 // CHECK:STDOUT:   %.loc9_23: i32 = int_literal 1 [template = constants.%.4]
 // CHECK:STDOUT:   %.loc9_25.1: (i32,) = tuple_literal (%.loc9_23)
 // CHECK:STDOUT:   %.loc9_25.2: (i32,) = tuple_value (%.loc9_23) [template = constants.%.5]
@@ -776,7 +776,7 @@ import library "extern_api";
 // CHECK:STDOUT:   %.loc10: init () = call %D.ref()
 // CHECK:STDOUT:   assign file.%d.var, %.loc10
 // CHECK:STDOUT:   %NS.ref: <namespace> = name_ref NS, file.%NS [template = file.%NS]
-// CHECK:STDOUT:   %E.ref: <function> = name_ref E, file.%import_ref.6 [template = imports.%E.1]
+// CHECK:STDOUT:   %E.ref: <function> = name_ref E, imports.%E.2 [template = imports.%E.2]
 // CHECK:STDOUT:   %.loc11: init () = call %E.ref()
 // CHECK:STDOUT:   assign file.%e.var, %.loc11
 // CHECK:STDOUT:   return
@@ -790,12 +790,12 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .A = %import_ref.1
 // CHECK:STDOUT:     .B = %import_ref.2
 // CHECK:STDOUT:     .C = %import_ref.3
 // CHECK:STDOUT:     .D = %import_ref.4
 // CHECK:STDOUT:     .NS = %NS
 // CHECK:STDOUT:     .a = %a
+// CHECK:STDOUT:     .A = %A
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loc_15 [template = imports.%A]
 // CHECK:STDOUT:   %import_ref.2 = import_ref ir1, inst+5, unloaded
@@ -876,12 +876,12 @@ import library "extern_api";
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .A = %import_ref.1
 // CHECK:STDOUT:     .B = %import_ref.2
 // CHECK:STDOUT:     .C = %import_ref.3
 // CHECK:STDOUT:     .D = %import_ref.4
 // CHECK:STDOUT:     .NS = %NS
 // CHECK:STDOUT:     .a = %a
+// CHECK:STDOUT:     .A = %A
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loc_15 [template = imports.%A]
 // CHECK:STDOUT:   %import_ref.2 = import_ref ir1, inst+5, unloaded
@@ -899,7 +899,7 @@ import library "extern_api";
 // CHECK:STDOUT:   %A: <function> = fn_decl @A [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A();
+// CHECK:STDOUT: extern fn @A();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:

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

@@ -74,7 +74,7 @@ extern fn F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = %F.loc4
+// CHECK:STDOUT:     .F = %F.loc12
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.loc4: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT:   %F.loc12: <function> = fn_decl @F [template] {}
@@ -89,7 +89,7 @@ extern fn F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .F = %F.loc4
+// CHECK:STDOUT:     .F = %F.loc12
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %F.loc4: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT:   %F.loc12: <function> = fn_decl @F [template] {}

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

@@ -8,15 +8,21 @@
 // Setup files
 // ============================================================================
 
-// --- a.carbon
+// --- fns.carbon
 
-library "a" api;
+library "fns" api;
 
 fn A() {}
 fn B(b: i32) -> i32 { return b; }
 fn C(c: (i32,)) -> {.c: i32} { return {.c = c[0]}; }
 fn D();
 
+// --- extern.carbon
+
+library "extern" api;
+
+extern fn A();
+
 // ============================================================================
 // Test files
 // ============================================================================
@@ -25,7 +31,7 @@ fn D();
 
 library "basics" api;
 
-import library "a";
+import library "fns";
 
 var a: () = A();
 var b: i32 = B(1);
@@ -35,15 +41,15 @@ var c: {.c: i32} = C((1,));
 
 library "def_ownership" api;
 
-import library "a";
+import library "fns";
 
 // CHECK:STDERR: fail_def_ownership.carbon:[[@LINE+10]]:1: ERROR: Only one library can declare function A without `extern`.
 // CHECK:STDERR: fn A() {};
 // CHECK:STDERR: ^~~~~~~~
 // CHECK:STDERR: fail_def_ownership.carbon:[[@LINE-5]]:1: In import.
-// CHECK:STDERR: import library "a";
+// CHECK:STDERR: import library "fns";
 // CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: a.carbon:4:1: Previously declared here.
+// CHECK:STDERR: fns.carbon:4:1: Previously declared here.
 // CHECK:STDERR: fn A() {}
 // CHECK:STDERR: ^~~~~~~~
 // CHECK:STDERR:
@@ -52,33 +58,39 @@ fn A() {};
 // CHECK:STDERR: fn B(b: i32) -> i32;
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR: fail_def_ownership.carbon:[[@LINE-16]]:1: In import.
-// CHECK:STDERR: import library "a";
+// CHECK:STDERR: import library "fns";
 // CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: a.carbon:5:1: Previously declared here.
+// CHECK:STDERR: fns.carbon:5:1: Previously declared here.
 // CHECK:STDERR: fn B(b: i32) -> i32 { return b; }
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
 fn B(b: i32) -> i32;
 
+// --- redecl_then_def.carbon
+
+library "redecl_then_def" api;
+
+import library "extern";
+
+fn A();
+fn A() {}
+
 // --- fail_mix_extern_decl.carbon
 
 library "mix_extern_decl" api;
 
-import library "a";
+import library "fns";
 
 extern fn D();
-// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+9]]:1: ERROR: Only one library can declare function D without `extern`.
+// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE+6]]:1: ERROR: Redeclaring `extern` function `D` as non-`extern`.
 // CHECK:STDERR: fn D() {}
 // CHECK:STDERR: ^~~~~~~~
-// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-6]]:1: In import.
-// CHECK:STDERR: import library "a";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: a.carbon:7:1: Previously declared here.
-// CHECK:STDERR: fn D();
-// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: fail_mix_extern_decl.carbon:[[@LINE-4]]:1: Previously declared `extern` here.
+// CHECK:STDERR: extern fn D();
+// CHECK:STDERR: ^~~~~~~~~~~~~~
 fn D() {}
 
-// CHECK:STDOUT: --- a.carbon
+// CHECK:STDOUT: --- fns.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT:   %.1: type = tuple_type (type) [template]
@@ -135,6 +147,17 @@ fn D() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @D();
 // CHECK:STDOUT:
+// CHECK:STDOUT: --- extern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .A = %A
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %A: <function> = fn_decl @A [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: extern fn @A();
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- basics.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
@@ -199,10 +222,10 @@ fn D() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .A = %import_ref.1
-// CHECK:STDOUT:     .B = %import_ref.2
 // CHECK:STDOUT:     .C = %import_ref.3
 // CHECK:STDOUT:     .D = %import_ref.4
+// CHECK:STDOUT:     .A = %A
+// CHECK:STDOUT:     .B = %B
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, loaded [template = imports.%A]
 // CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+6, loaded [template = imports.%B]
@@ -223,6 +246,22 @@ fn D() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @B(%b: i32) -> i32;
 // CHECK:STDOUT:
+// CHECK:STDOUT: --- redecl_then_def.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .A = %A.loc6
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %import_ref: <function> = import_ref ir1, inst+1, loaded [template = imports.%A]
+// CHECK:STDOUT:   %A.loc6: <function> = fn_decl @A [template] {}
+// CHECK:STDOUT:   %A.loc7: <function> = fn_decl @A [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @A() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_mix_extern_decl.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -230,14 +269,14 @@ fn D() {}
 // CHECK:STDOUT:     .A = %import_ref.1
 // CHECK:STDOUT:     .B = %import_ref.2
 // CHECK:STDOUT:     .C = %import_ref.3
-// CHECK:STDOUT:     .D = %import_ref.4
+// CHECK:STDOUT:     .D = %D.loc13
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.1 = import_ref ir1, inst+1, unloaded
 // CHECK:STDOUT:   %import_ref.2 = import_ref ir1, inst+6, unloaded
 // CHECK:STDOUT:   %import_ref.3 = import_ref ir1, inst+20, unloaded
 // CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+30, loaded [template = imports.%D]
 // CHECK:STDOUT:   %D.loc6: <function> = fn_decl @D [template] {}
-// CHECK:STDOUT:   %D.loc16: <function> = fn_decl @D [template] {}
+// CHECK:STDOUT:   %D.loc13: <function> = fn_decl @D [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @D() {