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.
This commit is contained in:
Richard Smith
2025-02-04 23:36:55 +00:00
committed by GitHub
parent fcfb1345d5
commit 6af3d050ea
3 changed files with 11 additions and 9 deletions
+7 -5
View File
@@ -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.
+2 -2
View File
@@ -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
+2 -2
View File
@@ -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