Implement merging of conflicts found during import. (#3819)

Note we only identify conflicts between libraries in the current package
during import.

This restructures the BUILD because of dependency cycles between cpp
files... We're going to need context's name lookup to handle things such
as merging, merging requires function logic, function logic requires
context access. Per discussion, going with a single large cc_library for
now rather than trying to split out small libraries.

I'm envisioning the new merge.* as a hub for cross-declaration merge
logic. Note function.cpp is already pretty sizable, and I think it may
lean a little function-specific even if there are some utilities that
could be split out.

toolchain/check/BUILD

@@ -54,10 +54,13 @@ cc_library(
         "convert.cpp",
         "decl_name_stack.cpp",
         "eval.cpp",
+        "function.cpp",
         "import_ref.cpp",
         "inst_block_stack.cpp",
+        "merge.cpp",
         "modifiers.cpp",
         "return.cpp",
+        "subst.cpp",
     ],
     hdrs = [
         "context.h",
@@ -66,12 +69,15 @@ cc_library(
         "decl_state.h",
         "diagnostic_helpers.h",
         "eval.h",
+        "function.h",
         "import_ref.h",
         "inst_block_stack.h",
+        "merge.h",
         "modifiers.h",
         "param_and_arg_refs_stack.h",
         "pending_block.h",
         "return.h",
+        "subst.h",
     ],
     deps = [
         ":node_stack",
@@ -104,7 +110,6 @@ cc_library(
     deps = [
         ":call",
         ":context",
-        ":function",
         ":impl",
         ":import",
         ":interface",
@@ -155,26 +160,12 @@ cc_library(
     ],
 )
 
-cc_library(
-    name = "function",
-    srcs = ["function.cpp"],
-    hdrs = ["function.h"],
-    deps = [
-        ":context",
-        ":subst",
-        "//common:check",
-        "//toolchain/sem_ir:file",
-    ],
-)
-
 cc_library(
     name = "impl",
     srcs = ["impl.cpp"],
     hdrs = ["impl.h"],
     deps = [
         ":context",
-        ":function",
-        ":subst",
         "//common:check",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/sem_ir:file",
@@ -219,7 +210,6 @@ cc_library(
     hdrs = ["member_access.h"],
     deps = [
         ":context",
-        ":subst",
         "//common:check",
         "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/sem_ir:file",
@@ -262,22 +252,6 @@ cc_library(
     ],
 )
 
-cc_library(
-    name = "subst",
-    srcs = ["subst.cpp"],
-    hdrs = ["subst.h"],
-    deps = [
-        ":context",
-        "//common:check",
-        "//toolchain/diagnostics:diagnostic_emitter",
-        "//toolchain/sem_ir:file",
-        "//toolchain/sem_ir:ids",
-        "//toolchain/sem_ir:inst",
-        "//toolchain/sem_ir:inst_kind",
-        "@llvm-project//llvm:Support",
-    ],
-)
-
 glob_sh_run(
     args = [
         "$(location //toolchain/driver:carbon)",

toolchain/check/function.cpp

@@ -193,7 +193,7 @@ auto CheckFunctionTypeMatches(Context& context,
 
 // Checks to see if a structurally valid redeclaration is allowed in context.
 // These all still merge.
-static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
+static auto CheckIsAllowedRedecl(Context& context, SemIR::LocationId loc_id,
                                  SemIR::Function& new_function,
                                  bool new_is_definition,
                                  SemIR::Function& prev_function,
@@ -207,7 +207,7 @@ static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
           "Only one library can declare function {0} without `extern`.",
           SemIR::NameId);
       context.emitter()
-          .Build(node_id, FunctionNonExternRedecl, prev_function.name_id)
+          .Build(loc_id, FunctionNonExternRedecl, prev_function.name_id)
           .Note(prev_function.decl_id, FunctionPreviousDecl)
           .Emit();
       return;
@@ -218,7 +218,7 @@ static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
                         "Redundant redeclaration of function {0}.",
                         SemIR::NameId);
       context.emitter()
-          .Build(node_id, FunctionRedecl, prev_function.name_id)
+          .Build(loc_id, FunctionRedecl, prev_function.name_id)
           .Note(prev_function.decl_id, FunctionPreviousDecl)
           .Emit();
       return;
@@ -229,7 +229,7 @@ static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
       CARBON_DIAGNOSTIC(FunctionPreviousDefinition, Note,
                         "Previously defined here.");
       context.emitter()
-          .Build(node_id, FunctionRedefinition, prev_function.name_id)
+          .Build(loc_id, FunctionRedefinition, prev_function.name_id)
           .Note(prev_function.definition_id, FunctionPreviousDefinition)
           .Emit();
       return;
@@ -243,7 +243,7 @@ static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
       CARBON_DIAGNOSTIC(FunctionPreviousExternDecl, Note,
                         "Previously declared `extern` here.");
       context.emitter()
-          .Build(node_id, FunctionDefiningExtern, prev_function.name_id)
+          .Build(loc_id, FunctionDefiningExtern, prev_function.name_id)
           .Note(prev_function.decl_id, FunctionPreviousExternDecl)
           .Emit();
       return;
@@ -253,7 +253,7 @@ static auto CheckIsAllowedRedecl(Context& context, Parse::NodeId node_id,
 
 // TODO: Detect conflicting cross-file declarations, as well as uses of imported
 // declarations followed by a redeclaration.
-auto MergeFunctionRedecl(Context& context, Parse::NodeId node_id,
+auto MergeFunctionRedecl(Context& context, SemIR::LocationId loc_id,
                          SemIR::Function& new_function, bool new_is_definition,
                          SemIR::FunctionId prev_function_id,
                          bool prev_is_import) -> bool {
@@ -263,7 +263,7 @@ auto MergeFunctionRedecl(Context& context, Parse::NodeId node_id,
     return false;
   }
 
-  CheckIsAllowedRedecl(context, node_id, new_function, new_is_definition,
+  CheckIsAllowedRedecl(context, loc_id, new_function, new_is_definition,
                        prev_function, prev_is_import);
 
   if (new_is_definition) {

toolchain/check/function.h

@@ -8,6 +8,7 @@
 #include "toolchain/check/context.h"
 #include "toolchain/check/subst.h"
 #include "toolchain/sem_ir/function.h"
+#include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::Check {
 
@@ -27,7 +28,7 @@ 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, Parse::NodeId node_id,
+auto MergeFunctionRedecl(Context& context, SemIR::LocationId loc_id,
                          SemIR::Function& new_function, bool new_is_definition,
                          SemIR::FunctionId prev_function_id,
                          bool prev_is_imported) -> bool;

toolchain/check/import.cpp

@@ -6,6 +6,7 @@
 
 #include "common/check.h"
 #include "toolchain/check/context.h"
+#include "toolchain/check/merge.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/sem_ir/file.h"
 #include "toolchain/sem_ir/ids.h"
@@ -242,15 +243,11 @@ auto ImportLibraryFromCurrentPackage(Context& context,
     } else {
       // Leave a placeholder that the inst comes from the other IR.
       auto target_id = context.AddImportRef(ir_id, import_inst_id);
-      // TODO: When importing from other packages, the scope's names should
-      // be changed to allow for ambiguous names. When importing from the
-      // current package, as is currently being done, we should issue a
-      // diagnostic on conflicts.
       auto [it, success] = context.name_scopes()
                                .Get(enclosing_scope_id)
                                .names.insert({name_id, target_id});
       if (!success) {
-        context.DiagnoseDuplicateName(target_id, it->second);
+        MergeImportRef(context, target_id, it->second);
       }
     }
   }

toolchain/check/merge.cpp

@@ -0,0 +1,81 @@
+// 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
+
+#include "toolchain/check/merge.h"
+
+#include "toolchain/check/function.h"
+#include "toolchain/check/import_ref.h"
+
+namespace Carbon::Check {
+
+// Returns the instruction to consider when merging the given inst_id. Returns
+// nullopt if merging is infeasible and no diagnostic should be printed.
+static auto ResolveMergeableInst(Context& context, SemIR::InstId inst_id)
+    -> std::optional<SemIR::Inst> {
+  auto inst = context.insts().Get(inst_id);
+  switch (inst.kind()) {
+    case SemIR::ImportRefUnused::Kind:
+      // Resolve before merging.
+      TryResolveImportRefUnused(context, inst_id);
+      break;
+
+    case SemIR::ImportRefUsed::Kind:
+      // Already resolved.
+      break;
+
+    case SemIR::Namespace::Kind:
+      // Return back the namespace directly.
+      return inst;
+
+    default:
+      CARBON_FATAL() << "Unexpected inst kind passed to ResolveMergeableInst: "
+                     << inst;
+  }
+
+  auto const_id = context.constant_values().Get(inst_id);
+  // TODO: Function and type declarations are constant, but `var` declarations
+  // are non-constant and should still merge.
+  if (!const_id.is_constant()) {
+    return std::nullopt;
+  }
+  return context.insts().Get(const_id.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);
+  auto prev_inst = ResolveMergeableInst(context, prev_inst_id);
+  if (!new_inst || !prev_inst) {
+    // TODO: Once `var` declarations get an associated instruction for handling,
+    // it might be more appropriate to return without diagnosing here, to handle
+    // invalid declarations.
+    context.DiagnoseDuplicateName(new_inst_id, prev_inst_id);
+    return;
+  }
+
+  if (new_inst->kind() != prev_inst->kind()) {
+    context.DiagnoseDuplicateName(new_inst_id, prev_inst_id);
+    return;
+  }
+
+  switch (new_inst->kind()) {
+    case SemIR::FunctionDecl::Kind: {
+      auto new_fn = context.functions().Get(
+          new_inst->As<SemIR::FunctionDecl>().function_id);
+      auto prev_fn_id = prev_inst->As<SemIR::FunctionDecl>().function_id;
+      // TODO: May need to "spoil" the new function to prevent it from being
+      // emitted, since it will already be added.
+      MergeFunctionRedecl(context, context.insts().GetLocationId(new_inst_id),
+                          new_fn,
+                          /*new_is_definition=*/false, prev_fn_id,
+                          /*prev_is_imported=*/true);
+      return;
+    }
+    default:
+      context.TODO(new_inst_id, "Merging not yet supported.");
+      return;
+  }
+}
+
+}  // namespace Carbon::Check

toolchain/check/merge.h

@@ -0,0 +1,20 @@
+// 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_CHECK_MERGE_H_
+#define CARBON_TOOLCHAIN_CHECK_MERGE_H_
+
+#include "toolchain/check/context.h"
+#include "toolchain/sem_ir/ids.h"
+
+namespace Carbon::Check {
+
+// 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,
+                    SemIR::InstId prev_inst_id) -> void;
+
+}  // namespace Carbon::Check
+
+#endif  // CARBON_TOOLCHAIN_CHECK_MERGE_H_

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

@@ -61,7 +61,7 @@ class C {
 // CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT: extern fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_redecl.carbon
 // CHECK:STDOUT:
@@ -73,7 +73,7 @@ class C {
 // CHECK:STDOUT:   %F.loc12: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT: extern fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_redecl_extern.carbon
 // CHECK:STDOUT:

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

@@ -73,63 +73,11 @@ var b: i32 = B(1);
 var c: {.c: i32} = C((1,));
 var d: () = D();
 
-// --- fail_todo_merge.carbon
+// --- merge.carbon
 
 library "merge" api;
 
 import library "api";
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE+52]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:4:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: extern fn A();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE-7]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:4:1: Name is previously declared here.
-// CHECK:STDERR: fn A();
-// CHECK:STDERR: ^~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE+39]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:5:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: extern fn B(b: i32) -> i32;
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE-20]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:5:1: Name is previously declared here.
-// CHECK:STDERR: fn B(b: i32) -> i32;
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE+26]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:6:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: extern fn C(c: (i32,)) -> {.c: i32};
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE-33]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:6:1: Name is previously declared here.
-// CHECK:STDERR: fn C(c: (i32,)) -> {.c: i32};
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE+13]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:7:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: extern fn D();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge.carbon:[[@LINE-46]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:7:1: Name is previously declared here.
-// CHECK:STDERR: extern fn D();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
-// CHECK:STDERR:
 import library "extern_api";
 
 var a: () = A();
@@ -137,62 +85,11 @@ var b: i32 = B(1);
 var c: {.c: i32} = C((1,));
 var d: () = D();
 
-// --- fail_todo_merge_reverse.carbon
+// --- merge_reverse.carbon
 
 library "merge_reverse" api;
 
 import library "extern_api";
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE+51]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:4:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: fn A();
-// CHECK:STDERR: ^~~~~~~
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE-7]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:4:1: Name is previously declared here.
-// CHECK:STDERR: extern fn A();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE+38]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:5:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: fn B(b: i32) -> i32;
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE-20]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:5:1: Name is previously declared here.
-// CHECK:STDERR: extern fn B(b: i32) -> i32;
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE+25]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:6:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: fn C(c: (i32,)) -> {.c: i32};
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE-33]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:6:1: Name is previously declared here.
-// CHECK:STDERR: extern fn C(c: (i32,)) -> {.c: i32};
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE+12]]:1: In import.
-// CHECK:STDERR: import library "api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: api.carbon:7:1: ERROR: Duplicate name being declared in the same scope.
-// CHECK:STDERR: extern fn D();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_merge_reverse.carbon:[[@LINE-46]]:1: In import.
-// CHECK:STDERR: import library "extern_api";
-// CHECK:STDERR: ^~~~~~
-// CHECK:STDERR: extern_api.carbon:7:1: Name is previously declared here.
-// CHECK:STDERR: extern fn D();
-// CHECK:STDERR: ^~~~~~~~~~~~~~
 import library "api";
 
 var a: () = A();
@@ -238,7 +135,7 @@ var d: () = D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @C(%c: (i32,)) -> {.c: i32};
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @D();
+// CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- extern_api.carbon
 // CHECK:STDOUT:
@@ -272,13 +169,13 @@ var d: () = D();
 // CHECK:STDOUT:   %D: <function> = fn_decl @D [template] {}
 // 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: fn @D();
+// CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- basics.carbon
 // CHECK:STDOUT:
@@ -326,7 +223,7 @@ var d: () = D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @C(%c: (i32,)) -> {.c: i32};
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @D();
+// CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
@@ -412,7 +309,7 @@ var d: () = D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @C(%c: (i32,)) -> {.c: i32};
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @D();
+// CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
@@ -492,13 +389,13 @@ var d: () = D();
 // CHECK:STDOUT:   %d: ref () = bind_name d, %d.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: fn @D();
+// CHECK:STDOUT: extern fn @D();
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
@@ -522,14 +419,14 @@ var d: () = D();
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_merge.carbon
+// CHECK:STDOUT: --- merge.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %.1: type = tuple_type () [template]
-// CHECK:STDOUT:   %.2: i32 = int_literal 1 [template]
-// CHECK:STDOUT:   %.3: type = struct_type {.c: i32} [template]
-// CHECK:STDOUT:   %.4: type = tuple_type (i32) [template]
-// CHECK:STDOUT:   %.5: (i32,) = tuple_value (%.2) [template]
+// CHECK:STDOUT:   %.1: type = tuple_type (i32) [template]
+// CHECK:STDOUT:   %.2: type = struct_type {.c: i32} [template]
+// CHECK:STDOUT:   %.3: type = tuple_type () [template]
+// CHECK:STDOUT:   %.4: i32 = int_literal 1 [template]
+// CHECK:STDOUT:   %.5: (i32,) = tuple_value (%.4) [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -543,67 +440,75 @@ var d: () = D();
 // CHECK:STDOUT:     .c = %c
 // CHECK:STDOUT:     .d = %d
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%A]
-// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, used [template = imports.%B]
-// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, used [template = imports.%C]
-// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, used [template = imports.%D]
-// CHECK:STDOUT:   %import_ref.5 = import_ref ir2, inst+1, unused
-// CHECK:STDOUT:   %import_ref.6 = import_ref ir2, inst+5, unused
-// CHECK:STDOUT:   %import_ref.7 = import_ref ir2, inst+17, unused
-// CHECK:STDOUT:   %import_ref.8 = import_ref ir2, inst+18, unused
-// CHECK:STDOUT:   %.loc59_9.1: () = tuple_literal ()
-// CHECK:STDOUT:   %.loc59_9.2: type = converted %.loc59_9.1, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%A.1]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, used [template = imports.%B.1]
+// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, used [template = imports.%C.1]
+// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, used [template = imports.%D.1]
+// CHECK:STDOUT:   %import_ref.5: <function> = import_ref ir2, inst+1, used [template = imports.%A.2]
+// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir2, inst+5, used [template = imports.%B.2]
+// CHECK:STDOUT:   %import_ref.7: <function> = import_ref ir2, inst+17, used [template = imports.%C.2]
+// CHECK:STDOUT:   %import_ref.8: <function> = import_ref ir2, inst+18, used [template = imports.%D.2]
+// CHECK:STDOUT:   %.loc7_9.1: () = tuple_literal ()
+// CHECK:STDOUT:   %.loc7_9.2: type = converted %.loc7_9.1, constants.%.3 [template = constants.%.3]
 // CHECK:STDOUT:   %a.var: ref () = var a
 // CHECK:STDOUT:   %a: ref () = bind_name a, %a.var
 // CHECK:STDOUT:   %b.var: ref i32 = var b
 // CHECK:STDOUT:   %b: ref i32 = bind_name b, %b.var
-// CHECK:STDOUT:   %.loc61: type = struct_type {.c: i32} [template = constants.%.3]
+// CHECK:STDOUT:   %.loc9: type = struct_type {.c: i32} [template = constants.%.2]
 // CHECK:STDOUT:   %c.var: ref {.c: i32} = var c
 // CHECK:STDOUT:   %c: ref {.c: i32} = bind_name c, %c.var
-// CHECK:STDOUT:   %.loc62_9.1: () = tuple_literal ()
-// CHECK:STDOUT:   %.loc62_9.2: type = converted %.loc62_9.1, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:   %.loc10_9.1: () = tuple_literal ()
+// CHECK:STDOUT:   %.loc10_9.2: type = converted %.loc10_9.1, constants.%.3 [template = constants.%.3]
 // CHECK:STDOUT:   %d.var: ref () = var d
 // CHECK:STDOUT:   %d: ref () = bind_name d, %d.var
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A();
+// CHECK:STDOUT: extern fn @A.1();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @B(%b: i32) -> i32;
+// CHECK:STDOUT: fn @A.2();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @C(%c: (i32,)) -> {.c: i32};
+// CHECK:STDOUT: extern fn @B.1(%b: i32) -> i32;
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @D();
+// CHECK:STDOUT: fn @B.2(%b: i32) -> i32;
+// CHECK:STDOUT:
+// CHECK:STDOUT: extern fn @C.1(%c: (i32,)) -> {.c: i32};
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @C.2(%c: (i32,)) -> {.c: i32};
+// CHECK:STDOUT:
+// CHECK:STDOUT: extern fn @D.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: extern fn @D.2();
 // 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:   %.loc59: init () = call %A.ref()
-// CHECK:STDOUT:   assign file.%a.var, %.loc59
-// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%import_ref.2 [template = imports.%B]
-// CHECK:STDOUT:   %.loc60_16: i32 = int_literal 1 [template = constants.%.2]
-// CHECK:STDOUT:   %.loc60_15: init i32 = call %B.ref(%.loc60_16)
-// CHECK:STDOUT:   assign file.%b.var, %.loc60_15
-// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%import_ref.3 [template = imports.%C]
-// CHECK:STDOUT:   %.loc61_23: i32 = int_literal 1 [template = constants.%.2]
-// CHECK:STDOUT:   %.loc61_25.1: (i32,) = tuple_literal (%.loc61_23)
-// CHECK:STDOUT:   %.loc61_25.2: (i32,) = tuple_value (%.loc61_23) [template = constants.%.5]
-// CHECK:STDOUT:   %.loc61_25.3: (i32,) = converted %.loc61_25.1, %.loc61_25.2 [template = constants.%.5]
-// CHECK:STDOUT:   %.loc61_21: init {.c: i32} = call %C.ref(%.loc61_25.3)
-// CHECK:STDOUT:   assign file.%c.var, %.loc61_21
-// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%import_ref.4 [template = imports.%D]
-// CHECK:STDOUT:   %.loc62: init () = call %D.ref()
-// CHECK:STDOUT:   assign file.%d.var, %.loc62
+// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%import_ref.1 [template = imports.%A.1]
+// 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:   %.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:   %.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]
+// CHECK:STDOUT:   %.loc9_25.3: (i32,) = converted %.loc9_25.1, %.loc9_25.2 [template = constants.%.5]
+// CHECK:STDOUT:   %.loc9_21: init {.c: i32} = call %C.ref(%.loc9_25.3)
+// CHECK:STDOUT:   assign file.%c.var, %.loc9_21
+// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%import_ref.4 [template = imports.%D.1]
+// CHECK:STDOUT:   %.loc10: init () = call %D.ref()
+// CHECK:STDOUT:   assign file.%d.var, %.loc10
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_todo_merge_reverse.carbon
+// CHECK:STDOUT: --- merge_reverse.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %.1: type = tuple_type () [template]
-// CHECK:STDOUT:   %.2: i32 = int_literal 1 [template]
-// CHECK:STDOUT:   %.3: type = struct_type {.c: i32} [template]
-// CHECK:STDOUT:   %.4: type = tuple_type (i32) [template]
-// CHECK:STDOUT:   %.5: (i32,) = tuple_value (%.2) [template]
+// CHECK:STDOUT:   %.1: type = tuple_type (i32) [template]
+// CHECK:STDOUT:   %.2: type = struct_type {.c: i32} [template]
+// CHECK:STDOUT:   %.3: type = tuple_type () [template]
+// CHECK:STDOUT:   %.4: i32 = int_literal 1 [template]
+// CHECK:STDOUT:   %.5: (i32,) = tuple_value (%.4) [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -617,56 +522,64 @@ var d: () = D();
 // CHECK:STDOUT:     .c = %c
 // CHECK:STDOUT:     .d = %d
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%A]
-// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, used [template = imports.%B]
-// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, used [template = imports.%C]
-// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, used [template = imports.%D]
-// CHECK:STDOUT:   %import_ref.5 = import_ref ir2, inst+1, unused
-// CHECK:STDOUT:   %import_ref.6 = import_ref ir2, inst+5, unused
-// CHECK:STDOUT:   %import_ref.7 = import_ref ir2, inst+17, unused
-// CHECK:STDOUT:   %import_ref.8 = import_ref ir2, inst+18, unused
-// CHECK:STDOUT:   %.loc58_9.1: () = tuple_literal ()
-// CHECK:STDOUT:   %.loc58_9.2: type = converted %.loc58_9.1, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%A.1]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir1, inst+5, used [template = imports.%B.1]
+// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir1, inst+17, used [template = imports.%C.1]
+// CHECK:STDOUT:   %import_ref.4: <function> = import_ref ir1, inst+18, used [template = imports.%D.1]
+// CHECK:STDOUT:   %import_ref.5: <function> = import_ref ir2, inst+1, used [template = imports.%A.2]
+// CHECK:STDOUT:   %import_ref.6: <function> = import_ref ir2, inst+5, used [template = imports.%B.2]
+// CHECK:STDOUT:   %import_ref.7: <function> = import_ref ir2, inst+17, used [template = imports.%C.2]
+// CHECK:STDOUT:   %import_ref.8: <function> = import_ref ir2, inst+18, used [template = imports.%D.2]
+// CHECK:STDOUT:   %.loc7_9.1: () = tuple_literal ()
+// CHECK:STDOUT:   %.loc7_9.2: type = converted %.loc7_9.1, constants.%.3 [template = constants.%.3]
 // CHECK:STDOUT:   %a.var: ref () = var a
 // CHECK:STDOUT:   %a: ref () = bind_name a, %a.var
 // CHECK:STDOUT:   %b.var: ref i32 = var b
 // CHECK:STDOUT:   %b: ref i32 = bind_name b, %b.var
-// CHECK:STDOUT:   %.loc60: type = struct_type {.c: i32} [template = constants.%.3]
+// CHECK:STDOUT:   %.loc9: type = struct_type {.c: i32} [template = constants.%.2]
 // CHECK:STDOUT:   %c.var: ref {.c: i32} = var c
 // CHECK:STDOUT:   %c: ref {.c: i32} = bind_name c, %c.var
-// CHECK:STDOUT:   %.loc61_9.1: () = tuple_literal ()
-// CHECK:STDOUT:   %.loc61_9.2: type = converted %.loc61_9.1, constants.%.1 [template = constants.%.1]
+// CHECK:STDOUT:   %.loc10_9.1: () = tuple_literal ()
+// CHECK:STDOUT:   %.loc10_9.2: type = converted %.loc10_9.1, constants.%.3 [template = constants.%.3]
 // CHECK:STDOUT:   %d.var: ref () = var d
 // CHECK:STDOUT:   %d: ref () = bind_name d, %d.var
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @A();
+// CHECK:STDOUT: fn @A.1();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @B(%b: i32) -> i32;
+// CHECK:STDOUT: fn @A.2();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @C(%c: (i32,)) -> {.c: i32};
+// CHECK:STDOUT: fn @B.1(%b: i32) -> i32;
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @B.2(%b: i32) -> i32;
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @C.1(%c: (i32,)) -> {.c: i32};
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @C.2(%c: (i32,)) -> {.c: i32};
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @D();
+// CHECK:STDOUT: extern fn @D.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: extern fn @D.2();
 // 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:   %.loc58: init () = call %A.ref()
-// CHECK:STDOUT:   assign file.%a.var, %.loc58
-// CHECK:STDOUT:   %B.ref: <function> = name_ref B, file.%import_ref.2 [template = imports.%B]
-// CHECK:STDOUT:   %.loc59_16: i32 = int_literal 1 [template = constants.%.2]
-// CHECK:STDOUT:   %.loc59_15: init i32 = call %B.ref(%.loc59_16)
-// CHECK:STDOUT:   assign file.%b.var, %.loc59_15
-// CHECK:STDOUT:   %C.ref: <function> = name_ref C, file.%import_ref.3 [template = imports.%C]
-// CHECK:STDOUT:   %.loc60_23: i32 = int_literal 1 [template = constants.%.2]
-// CHECK:STDOUT:   %.loc60_25.1: (i32,) = tuple_literal (%.loc60_23)
-// CHECK:STDOUT:   %.loc60_25.2: (i32,) = tuple_value (%.loc60_23) [template = constants.%.5]
-// CHECK:STDOUT:   %.loc60_25.3: (i32,) = converted %.loc60_25.1, %.loc60_25.2 [template = constants.%.5]
-// CHECK:STDOUT:   %.loc60_21: init {.c: i32} = call %C.ref(%.loc60_25.3)
-// CHECK:STDOUT:   assign file.%c.var, %.loc60_21
-// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%import_ref.4 [template = imports.%D]
-// CHECK:STDOUT:   %.loc61: init () = call %D.ref()
-// CHECK:STDOUT:   assign file.%d.var, %.loc61
+// CHECK:STDOUT:   %A.ref: <function> = name_ref A, file.%import_ref.1 [template = imports.%A.1]
+// 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:   %.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:   %.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]
+// CHECK:STDOUT:   %.loc9_25.3: (i32,) = converted %.loc9_25.1, %.loc9_25.2 [template = constants.%.5]
+// CHECK:STDOUT:   %.loc9_21: init {.c: i32} = call %C.ref(%.loc9_25.3)
+// CHECK:STDOUT:   assign file.%c.var, %.loc9_21
+// CHECK:STDOUT:   %D.ref: <function> = name_ref D, file.%import_ref.4 [template = imports.%D.1]
+// CHECK:STDOUT:   %.loc10: init () = call %D.ref()
+// CHECK:STDOUT:   assign file.%d.var, %.loc10
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_first.carbon

@@ -81,10 +81,12 @@ fn NS.Bar() {}
 // CHECK:STDOUT:     .Bar = %Bar
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %import_ref.2 = import_ref ir1, inst+2, unused
-// CHECK:STDOUT:   %import_ref.3 = import_ref ir2, inst+1, unused
+// CHECK:STDOUT:   %import_ref.3: <function> = import_ref ir2, inst+1, used [template = imports.%NS]
 // CHECK:STDOUT:   %Bar: <function> = fn_decl @Bar [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: fn @NS();
+// CHECK:STDOUT:
 // CHECK:STDOUT: fn @Bar() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return

toolchain/check/testdata/packages/cross_package_import.carbon

@@ -199,7 +199,7 @@ fn Other.G() {}
 // CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT: extern fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- other_fn_conflict.carbon
 // CHECK:STDOUT:
@@ -300,7 +300,7 @@ fn Other.G() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.1();
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F.2();
+// CHECK:STDOUT: extern fn @F.2();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- main_unused_other_ambiguous.carbon
 // CHECK:STDOUT:

toolchain/check/testdata/packages/fail_conflict_no_namespaces.carbon

@@ -62,7 +62,9 @@ import library "var";
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .Foo = %import_ref.1
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref.1 = import_ref ir1, inst+1, unused
-// CHECK:STDOUT:   %import_ref.2 = import_ref ir2, inst+2, unused
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%Foo]
+// CHECK:STDOUT:   %import_ref.2: ref i32 = import_ref ir2, inst+2, used
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: fn @Foo();
+// CHECK:STDOUT:

toolchain/sem_ir/formatter.cpp

@@ -771,7 +771,13 @@ class Formatter {
   auto FormatFunction(FunctionId id) -> void {
     const Function& fn = sem_ir_.functions().Get(id);
 
-    out_ << "\nfn ";
+    out_ << "\n";
+
+    if (fn.is_extern) {
+      out_ << "extern ";
+    }
+
+    out_ << "fn ";
     FormatFunctionName(id);
 
     llvm::SaveAndRestore function_scope(scope_, inst_namer_.GetScopeFor(id));