Skip to content

Commit 02ea39f

Browse files
bricknerbbricknerb
authored
Avoid crashing when importing a C++ function indirectly (#6085)
Return an error instruction instead and output a `TODO`. Part of #6060.
1 parent f29515f commit 02ea39f

File tree

2 files changed

+58
-24
lines changed

2 files changed

+58
-24
lines changed

‎toolchain/check/import_ref.cpp‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,26 +1837,22 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, InstT inst)
18371837
resolver, {.type_id = SemIR::TypeType::TypeId, .inner_id = inner_id});
18381838
}
18391839

1840-
// TODO: This is a WIP attempt to solve the failing test
1841-
// https://github.com/carbon-language/carbon-lang/blob/508a88e2a995c9f3342b019cee6948c162004b68/toolchain/check/testdata/interop/cpp/import.carbon.
1842-
// Adding this method solves the failure `TryResolveInst on unsupported
1843-
// instruction kind CppOverloadSetType`. However there is a new failure
1844-
// `./toolchain/base/value_store.h:111: id.index < size_: inst27` and this still
1845-
// remains a WIP.
18461840
static auto TryResolveTypedInst(ImportRefResolver& resolver,
1847-
SemIR::CppOverloadSetType inst,
1848-
SemIR::InstId inst_id, SemIR::Inst untyped_inst)
1841+
SemIR::CppOverloadSetType inst)
18491842
-> ResolveResult {
1850-
resolver.local_context().TODO(SemIR::LocId::None,
1851-
"Unsupported: Importing C++ functions that "
1852-
"require thunks indirectly called here");
1853-
auto inst_constant_id = resolver.import_constant_values().Get(inst_id);
1854-
if (!inst_constant_id.is_constant()) {
1855-
CARBON_CHECK(untyped_inst.Is<SemIR::BindName>(),
1856-
"TryResolveInst on non-constant instruction {0}", inst);
1857-
return ResolveResult::Done(SemIR::ConstantId::NotConstant);
1858-
}
1859-
return ResolveResult::Done(inst_constant_id);
1843+
// Supporting C++ overload resolution of imported functions is a large task,
1844+
// which might require serializing and deserializing AST for using decl ids,
1845+
// using modules and/or linking ASTs.
1846+
resolver.local_context().TODO(
1847+
SemIR::LocId::None,
1848+
llvm::formatv("Unsupported: Importing C++ function `{0}` indirectly",
1849+
resolver.import_ir().names().GetAsStringIfIdentifier(
1850+
resolver.import_ir()
1851+
.cpp_overload_sets()
1852+
.Get(inst.overload_set_id)
1853+
.name_id)));
1854+
return ResolveResult::Done(SemIR::ErrorInst::ConstantId,
1855+
SemIR::ErrorInst::InstId);
18601856
}
18611857

18621858
static auto TryResolveTypedInst(ImportRefResolver& resolver,
@@ -3182,7 +3178,7 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
31823178
return TryResolveTypedInst(resolver, inst);
31833179
}
31843180
case CARBON_KIND(SemIR::CppOverloadSetType inst): {
3185-
return TryResolveTypedInst(resolver, inst, inst_id, untyped_inst);
3181+
return TryResolveTypedInst(resolver, inst);
31863182
}
31873183
case CARBON_KIND(SemIR::ExportDecl inst): {
31883184
return TryResolveTypedInst(resolver, inst);

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

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,20 @@ import Cpp library "function.h";
6464
alias FooShort = Cpp.foo_short;
6565
alias FooInt = Cpp.foo_int;
6666

67-
// --- todo_import_function_api.carbon
67+
// --- fail_todo_import_function_api.carbon
68+
// CHECK:STDERR: fail_todo_import_function_api.carbon: error: semantics TODO: `Unsupported: Importing C++ function `foo_short` indirectly` [SemanticsTodo]
69+
// CHECK:STDERR:
70+
// CHECK:STDERR: fail_todo_import_function_api.carbon: error: semantics TODO: `Unsupported: Importing C++ function `foo_int` indirectly` [SemanticsTodo]
71+
// CHECK:STDERR:
6872

6973
library "[[@TEST_NAME]]";
7074

7175
import library "function_api";
7276

73-
// TODO: Fix this test as a follow-up of https://github.com/carbon-language/carbon-lang/pull/5891.
7477
fn F() {
7578
//@dump-sem-ir-begin
76-
// FooShort(8 as i16);
77-
// FooInt(9);
79+
FooShort(8 as i16);
80+
FooInt(9);
7881
//@dump-sem-ir-end
7982
}
8083

@@ -98,16 +101,51 @@ fn F() {
98101
// CHECK:STDOUT: <elided>
99102
// CHECK:STDOUT: }
100103
// CHECK:STDOUT:
101-
// CHECK:STDOUT: --- todo_import_function_api.carbon
104+
// CHECK:STDOUT: --- fail_todo_import_function_api.carbon
102105
// CHECK:STDOUT:
103106
// CHECK:STDOUT: constants {
107+
// CHECK:STDOUT: %int_8.b85: Core.IntLiteral = int_value 8 [concrete]
108+
// CHECK:STDOUT: %int_16: Core.IntLiteral = int_value 16 [concrete]
109+
// CHECK:STDOUT: %i16: type = class_type @Int, @Int(%int_16) [concrete]
110+
// CHECK:STDOUT: %As.type.771: type = facet_type <@As, @As(%i16)> [concrete]
111+
// CHECK:STDOUT: %As.Convert.type.be5: type = fn_type @As.Convert, @As(%i16) [concrete]
112+
// CHECK:STDOUT: %To: Core.IntLiteral = bind_symbolic_name To, 0 [symbolic]
113+
// 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]
114+
// CHECK:STDOUT: %Core.IntLiteral.as.As.impl.Convert.d2c: %Core.IntLiteral.as.As.impl.Convert.type.565 = struct_value () [symbolic]
115+
// CHECK:STDOUT: %As.impl_witness.2d2: <witness> = impl_witness imports.%As.impl_witness_table.5ad, @Core.IntLiteral.as.As.impl(%int_16) [concrete]
116+
// 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]
117+
// CHECK:STDOUT: %Core.IntLiteral.as.As.impl.Convert.97a: %Core.IntLiteral.as.As.impl.Convert.type.38a = struct_value () [concrete]
118+
// CHECK:STDOUT: %As.facet: %As.type.771 = facet_value Core.IntLiteral, (%As.impl_witness.2d2) [concrete]
119+
// CHECK:STDOUT: %.026: type = fn_type_with_self_type %As.Convert.type.be5, %As.facet [concrete]
120+
// CHECK:STDOUT: %Core.IntLiteral.as.As.impl.Convert.bound: <bound method> = bound_method %int_8.b85, %Core.IntLiteral.as.As.impl.Convert.97a [concrete]
121+
// 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]
122+
// CHECK:STDOUT: %bound_method: <bound method> = bound_method %int_8.b85, %Core.IntLiteral.as.As.impl.Convert.specific_fn [concrete]
123+
// CHECK:STDOUT: %int_8.823: %i16 = int_value 8 [concrete]
124+
// CHECK:STDOUT: %int_9: Core.IntLiteral = int_value 9 [concrete]
104125
// CHECK:STDOUT: }
105126
// CHECK:STDOUT:
106127
// CHECK:STDOUT: imports {
128+
// CHECK:STDOUT: %Main.FooShort: <error> = import_ref Main//function_api, FooShort, loaded [concrete = <error>]
129+
// CHECK:STDOUT: %Main.FooInt: <error> = import_ref Main//function_api, FooInt, loaded [concrete = <error>]
130+
// 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)]
131+
// CHECK:STDOUT: %As.impl_witness_table.5ad = impl_witness_table (%Core.import_ref.99c), @Core.IntLiteral.as.As.impl [concrete]
107132
// CHECK:STDOUT: }
108133
// CHECK:STDOUT:
109134
// CHECK:STDOUT: fn @F() {
110135
// CHECK:STDOUT: !entry:
136+
// CHECK:STDOUT: %FooShort.ref: <error> = name_ref FooShort, imports.%Main.FooShort [concrete = <error>]
137+
// CHECK:STDOUT: %int_8: Core.IntLiteral = int_value 8 [concrete = constants.%int_8.b85]
138+
// CHECK:STDOUT: %int_16: Core.IntLiteral = int_value 16 [concrete = constants.%int_16]
139+
// CHECK:STDOUT: %i16: type = class_type @Int, @Int(constants.%int_16) [concrete = constants.%i16]
140+
// CHECK:STDOUT: %impl.elem0: %.026 = impl_witness_access constants.%As.impl_witness.2d2, element0 [concrete = constants.%Core.IntLiteral.as.As.impl.Convert.97a]
141+
// CHECK:STDOUT: %bound_method.loc12_14.1: <bound method> = bound_method %int_8, %impl.elem0 [concrete = constants.%Core.IntLiteral.as.As.impl.Convert.bound]
142+
// 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]
143+
// CHECK:STDOUT: %bound_method.loc12_14.2: <bound method> = bound_method %int_8, %specific_fn [concrete = constants.%bound_method]
144+
// CHECK:STDOUT: %Core.IntLiteral.as.As.impl.Convert.call: init %i16 = call %bound_method.loc12_14.2(%int_8) [concrete = constants.%int_8.823]
145+
// CHECK:STDOUT: %.loc12_14.1: %i16 = value_of_initializer %Core.IntLiteral.as.As.impl.Convert.call [concrete = constants.%int_8.823]
146+
// CHECK:STDOUT: %.loc12_14.2: %i16 = converted %int_8, %.loc12_14.1 [concrete = constants.%int_8.823]
147+
// CHECK:STDOUT: %FooInt.ref: <error> = name_ref FooInt, imports.%Main.FooInt [concrete = <error>]
148+
// CHECK:STDOUT: %int_9: Core.IntLiteral = int_value 9 [concrete = constants.%int_9]
111149
// CHECK:STDOUT: <elided>
112150
// CHECK:STDOUT: }
113151
// CHECK:STDOUT:

0 commit comments

Comments
 (0)