Avoid crashing when importing a C++ function indirectly (#6085)

Return an error instruction instead and output a `TODO`.

Part of #6060.

toolchain/check/import_ref.cpp

@@ -1837,26 +1837,22 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, InstT inst)
       resolver, {.type_id = SemIR::TypeType::TypeId, .inner_id = inner_id});
 }
 
-// TODO: This is a WIP attempt to solve the failing test
-// https://github.com/carbon-language/carbon-lang/blob/508a88e2a995c9f3342b019cee6948c162004b68/toolchain/check/testdata/interop/cpp/import.carbon.
-// Adding this method solves the failure `TryResolveInst on unsupported
-// instruction kind CppOverloadSetType`. However there is a new failure
-// `./toolchain/base/value_store.h:111: id.index < size_: inst27` and this still
-// remains a WIP.
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
-                                SemIR::CppOverloadSetType inst,
-                                SemIR::InstId inst_id, SemIR::Inst untyped_inst)
+                                SemIR::CppOverloadSetType inst)
     -> ResolveResult {
-  resolver.local_context().TODO(SemIR::LocId::None,
-                                "Unsupported: Importing C++ functions that "
-                                "require thunks indirectly called here");
-  auto inst_constant_id = resolver.import_constant_values().Get(inst_id);
-  if (!inst_constant_id.is_constant()) {
-    CARBON_CHECK(untyped_inst.Is<SemIR::BindName>(),
-                 "TryResolveInst on non-constant instruction {0}", inst);
-    return ResolveResult::Done(SemIR::ConstantId::NotConstant);
-  }
-  return ResolveResult::Done(inst_constant_id);
+  // Supporting C++ overload resolution of imported functions is a large task,
+  // which might require serializing and deserializing AST for using decl ids,
+  // using modules and/or linking ASTs.
+  resolver.local_context().TODO(
+      SemIR::LocId::None,
+      llvm::formatv("Unsupported: Importing C++ function `{0}` indirectly",
+                    resolver.import_ir().names().GetAsStringIfIdentifier(
+                        resolver.import_ir()
+                            .cpp_overload_sets()
+                            .Get(inst.overload_set_id)
+                            .name_id)));
+  return ResolveResult::Done(SemIR::ErrorInst::ConstantId,
+                             SemIR::ErrorInst::InstId);
 }
 
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -3182,7 +3178,7 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
       return TryResolveTypedInst(resolver, inst);
     }
     case CARBON_KIND(SemIR::CppOverloadSetType inst): {
-      return TryResolveTypedInst(resolver, inst, inst_id, untyped_inst);
+      return TryResolveTypedInst(resolver, inst);
     }
     case CARBON_KIND(SemIR::ExportDecl inst): {
       return TryResolveTypedInst(resolver, inst);

toolchain/check/testdata/interop/cpp/import.carbon

@@ -64,17 +64,20 @@ import Cpp library "function.h";
 alias FooShort = Cpp.foo_short;
 alias FooInt = Cpp.foo_int;
 
-// --- todo_import_function_api.carbon
+// --- fail_todo_import_function_api.carbon
+// CHECK:STDERR: fail_todo_import_function_api.carbon: error: semantics TODO: `Unsupported: Importing C++ function `foo_short` indirectly` [SemanticsTodo]
+// CHECK:STDERR:
+// CHECK:STDERR: fail_todo_import_function_api.carbon: error: semantics TODO: `Unsupported: Importing C++ function `foo_int` indirectly` [SemanticsTodo]
+// CHECK:STDERR:
 
 library "[[@TEST_NAME]]";
 
 import library "function_api";
 
-// TODO: Fix this test as a follow-up of https://github.com/carbon-language/carbon-lang/pull/5891.
 fn F() {
   //@dump-sem-ir-begin
-  // FooShort(8 as i16);
-  // FooInt(9);
+  FooShort(8 as i16);
+  FooInt(9);
   //@dump-sem-ir-end
 }
 
@@ -98,16 +101,51 @@ fn F() {
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- todo_import_function_api.carbon
+// CHECK:STDOUT: --- fail_todo_import_function_api.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %int_8.b85: Core.IntLiteral = int_value 8 [concrete]
+// CHECK:STDOUT:   %int_16: Core.IntLiteral = int_value 16 [concrete]
+// CHECK:STDOUT:   %i16: type = class_type @Int, @Int(%int_16) [concrete]
+// CHECK:STDOUT:   %As.type.771: type = facet_type <@As, @As(%i16)> [concrete]
+// CHECK:STDOUT:   %As.Convert.type.be5: type = fn_type @As.Convert, @As(%i16) [concrete]
+// CHECK:STDOUT:   %To: Core.IntLiteral = bind_symbolic_name To, 0 [symbolic]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.type.565: type = fn_type @Core.IntLiteral.as.As.impl.Convert, @Core.IntLiteral.as.As.impl(%To) [symbolic]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.d2c: %Core.IntLiteral.as.As.impl.Convert.type.565 = struct_value () [symbolic]
+// CHECK:STDOUT:   %As.impl_witness.2d2: <witness> = impl_witness imports.%As.impl_witness_table.5ad, @Core.IntLiteral.as.As.impl(%int_16) [concrete]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.type.38a: type = fn_type @Core.IntLiteral.as.As.impl.Convert, @Core.IntLiteral.as.As.impl(%int_16) [concrete]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.97a: %Core.IntLiteral.as.As.impl.Convert.type.38a = struct_value () [concrete]
+// CHECK:STDOUT:   %As.facet: %As.type.771 = facet_value Core.IntLiteral, (%As.impl_witness.2d2) [concrete]
+// CHECK:STDOUT:   %.026: type = fn_type_with_self_type %As.Convert.type.be5, %As.facet [concrete]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.bound: <bound method> = bound_method %int_8.b85, %Core.IntLiteral.as.As.impl.Convert.97a [concrete]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.specific_fn: <specific function> = specific_function %Core.IntLiteral.as.As.impl.Convert.97a, @Core.IntLiteral.as.As.impl.Convert(%int_16) [concrete]
+// CHECK:STDOUT:   %bound_method: <bound method> = bound_method %int_8.b85, %Core.IntLiteral.as.As.impl.Convert.specific_fn [concrete]
+// CHECK:STDOUT:   %int_8.823: %i16 = int_value 8 [concrete]
+// CHECK:STDOUT:   %int_9: Core.IntLiteral = int_value 9 [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
+// CHECK:STDOUT:   %Main.FooShort: <error> = import_ref Main//function_api, FooShort, loaded [concrete = <error>]
+// CHECK:STDOUT:   %Main.FooInt: <error> = import_ref Main//function_api, FooInt, loaded [concrete = <error>]
+// CHECK:STDOUT:   %Core.import_ref.99c: @Core.IntLiteral.as.As.impl.%Core.IntLiteral.as.As.impl.Convert.type (%Core.IntLiteral.as.As.impl.Convert.type.565) = import_ref Core//prelude/parts/int, loc32_39, loaded [symbolic = @Core.IntLiteral.as.As.impl.%Core.IntLiteral.as.As.impl.Convert (constants.%Core.IntLiteral.as.As.impl.Convert.d2c)]
+// CHECK:STDOUT:   %As.impl_witness_table.5ad = impl_witness_table (%Core.import_ref.99c), @Core.IntLiteral.as.As.impl [concrete]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F() {
 // CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %FooShort.ref: <error> = name_ref FooShort, imports.%Main.FooShort [concrete = <error>]
+// CHECK:STDOUT:   %int_8: Core.IntLiteral = int_value 8 [concrete = constants.%int_8.b85]
+// CHECK:STDOUT:   %int_16: Core.IntLiteral = int_value 16 [concrete = constants.%int_16]
+// CHECK:STDOUT:   %i16: type = class_type @Int, @Int(constants.%int_16) [concrete = constants.%i16]
+// CHECK:STDOUT:   %impl.elem0: %.026 = impl_witness_access constants.%As.impl_witness.2d2, element0 [concrete = constants.%Core.IntLiteral.as.As.impl.Convert.97a]
+// CHECK:STDOUT:   %bound_method.loc12_14.1: <bound method> = bound_method %int_8, %impl.elem0 [concrete = constants.%Core.IntLiteral.as.As.impl.Convert.bound]
+// CHECK:STDOUT:   %specific_fn: <specific function> = specific_function %impl.elem0, @Core.IntLiteral.as.As.impl.Convert(constants.%int_16) [concrete = constants.%Core.IntLiteral.as.As.impl.Convert.specific_fn]
+// CHECK:STDOUT:   %bound_method.loc12_14.2: <bound method> = bound_method %int_8, %specific_fn [concrete = constants.%bound_method]
+// CHECK:STDOUT:   %Core.IntLiteral.as.As.impl.Convert.call: init %i16 = call %bound_method.loc12_14.2(%int_8) [concrete = constants.%int_8.823]
+// CHECK:STDOUT:   %.loc12_14.1: %i16 = value_of_initializer %Core.IntLiteral.as.As.impl.Convert.call [concrete = constants.%int_8.823]
+// CHECK:STDOUT:   %.loc12_14.2: %i16 = converted %int_8, %.loc12_14.1 [concrete = constants.%int_8.823]
+// CHECK:STDOUT:   %FooInt.ref: <error> = name_ref FooInt, imports.%Main.FooInt [concrete = <error>]
+// CHECK:STDOUT:   %int_9: Core.IntLiteral = int_value 9 [concrete = constants.%int_9]
 // CHECK:STDOUT:   <elided>
 // CHECK:STDOUT: }
 // CHECK:STDOUT: