Don't recover from import errors by producing an error constant. (#4892)

Instead, produce a CARBON_FATAL error. Returning an Error constant from
the importer seems reasonable but turns out to not work well in
practice, because it violates the invariant that a constant value should
not have an error as an operand.

Also, don't produce an error constant for a valid ImportRefLoaded that
whose value is not constant; preserve the non-constant value instead.

Author: zygoloid

toolchain/check/import_ref.cpp

@@ -2152,13 +2152,12 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
                                 SemIR::InstId inst_id) -> ResolveResult {
   // Return the constant for the instruction of the imported constant.
   auto constant_id = resolver.import_constant_values().Get(inst_id);
-  if (!constant_id.has_value()) {
-    return ResolveResult::Done(SemIR::ErrorInst::SingletonConstantId);
-  }
+  CARBON_CHECK(constant_id.has_value(),
+               "Loaded import ref has no constant value");
   if (!constant_id.is_constant()) {
     resolver.local_context().TODO(
         inst_id, "Non-constant ImportRefLoaded (comes up with var)");
-    return ResolveResult::Done(SemIR::ErrorInst::SingletonConstantId);
+    return ResolveResult::Done(constant_id);
   }
 
   auto new_constant_id = GetLocalConstantId(
@@ -2839,10 +2838,13 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
       auto constant_inst_id =
           resolver.import_constant_values().GetConstantInstId(inst_id);
       if (constant_inst_id == inst_id) {
+        // Produce a diagnostic to provide a source location with the CHECK
+        // failure.
         resolver.local_context().TODO(
             SemIR::LocId(AddImportIRInst(resolver, inst_id)),
             llvm::formatv("TryResolveInst on {0}", untyped_inst.kind()).str());
-        return {.const_id = SemIR::ErrorInst::SingletonConstantId};
+        CARBON_FATAL("TryResolveInst on unsupported instruction kind {0}",
+                     untyped_inst.kind());
       }
       // Try to resolve the constant value instead. Note that this can only
       // retry once.

toolchain/check/testdata/alias/no_prelude/import.carbon

@@ -280,7 +280,7 @@ var c: () = a_alias_alias;
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
-// CHECK:STDOUT:   %Main.a_alias_alias: ref %empty_tuple.type = import_ref Main//var2, a_alias_alias, loaded [template = <error>]
+// CHECK:STDOUT:   %Main.a_alias_alias: ref %empty_tuple.type = import_ref Main//var2, a_alias_alias, loaded
 // CHECK:STDOUT:   %Main.b = import_ref Main//var2, b, unloaded
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
@@ -305,7 +305,7 @@ var c: () = a_alias_alias;
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %a_alias_alias.ref: ref %empty_tuple.type = name_ref a_alias_alias, imports.%Main.a_alias_alias [template = <error>]
+// CHECK:STDOUT:   %a_alias_alias.ref: ref %empty_tuple.type = name_ref a_alias_alias, imports.%Main.a_alias_alias
 // CHECK:STDOUT:   %.loc11_13: init %empty_tuple.type = tuple_init () to file.%c.var [template = constants.%empty_tuple]
 // CHECK:STDOUT:   %.loc11_1: init %empty_tuple.type = converted %a_alias_alias.ref, %.loc11_13 [template = constants.%empty_tuple]
 // CHECK:STDOUT:   assign file.%c.var, %.loc11_1

toolchain/check/testdata/var/no_prelude/export_name.carbon

@@ -92,7 +92,7 @@ var w: () = v;
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: imports {
-// CHECK:STDOUT:   %Main.v: ref %empty_tuple.type = import_ref Main//export, v, loaded [template = <error>]
+// CHECK:STDOUT:   %Main.v: ref %empty_tuple.type = import_ref Main//export, v, loaded
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -115,7 +115,7 @@ var w: () = v;
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %v.ref: ref %empty_tuple.type = name_ref v, imports.%Main.v [template = <error>]
+// CHECK:STDOUT:   %v.ref: ref %empty_tuple.type = name_ref v, imports.%Main.v
 // CHECK:STDOUT:   %.loc12_13: init %empty_tuple.type = tuple_init () to file.%w.var [template = constants.%empty_tuple]
 // CHECK:STDOUT:   %.loc12_1: init %empty_tuple.type = converted %v.ref, %.loc12_13 [template = constants.%empty_tuple]
 // CHECK:STDOUT:   assign file.%w.var, %.loc12_1