Skip to content

Commit 81c2b3b

Browse files
jonmeowjonmeow
authored
Handle some more errors in interfaces without crashing. (#6155)
This fixes and tests two crashes related to what I was observing [on #toolchain](https://discord.com/channels/655572317891461132/655578254970716160/1422723976483831848). The approach to Cpp imports taken in #6086 is problematic because it returns before the work stack is completed, and doesn't store the resulting constants. This fixes the approach taken in that PR.
1 parent 12fa65e commit 81c2b3b

File tree

3 files changed

+146
-10
lines changed

3 files changed

+146
-10
lines changed

‎toolchain/check/import_ref.cpp‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -450,9 +450,6 @@ class ImportRefResolver : public ImportContext {
450450

451451
// Step 1: check for a constant value.
452452
auto existing = FindResolvedConstId(work.inst_id);
453-
if (existing.const_id == SemIR::ErrorInst::ConstantId) {
454-
return existing.const_id;
455-
}
456453
if (existing.const_id.has_value() && !work.retry_with_constant_value) {
457454
work_stack_.pop_back();
458455
continue;
@@ -592,7 +589,11 @@ class ImportRefResolver : public ImportContext {
592589
if (ir_inst.ir_id() == SemIR::ImportIRId::Cpp) {
593590
local_context().TODO(SemIR::LocId::None,
594591
"Unsupported: Importing C++ indirectly");
595-
return {.const_id = SemIR::ErrorInst::ConstantId};
592+
SetResolvedConstId(inst_id, result.indirect_insts,
593+
SemIR::ErrorInst::ConstantId);
594+
result.const_id = SemIR::ErrorInst::ConstantId;
595+
result.indirect_insts.clear();
596+
return result;
596597
}
597598

598599
const auto* prev_ir = cursor_ir;
@@ -1794,6 +1795,11 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
17941795
auto class_const_id = GetLocalConstantId(
17951796
resolver,
17961797
resolver.import_classes().Get(inst.class_id).first_owning_decl_id);
1798+
if (class_const_id == SemIR::ErrorInst::ConstantId) {
1799+
// TODO: It should be possible to remove this once C++ imports work.
1800+
return ResolveResult::Done(class_const_id);
1801+
}
1802+
17971803
auto specific_data = GetLocalSpecificData(resolver, inst.specific_id);
17981804
if (resolver.HasNewWork()) {
17991805
return ResolveResult::Retry();

‎toolchain/check/interface.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,13 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id,
181181
SemIR::TypeId self_type_id,
182182
SemIR::InstId self_witness_id)
183183
-> SemIR::TypeId {
184-
auto decl =
185-
context.insts().Get(context.constant_values().GetConstantInstId(decl_id));
184+
auto decl_constant_inst_id =
185+
context.constant_values().GetConstantInstId(decl_id);
186+
if (decl_constant_inst_id == SemIR::ErrorInst::InstId) {
187+
return SemIR::ErrorInst::TypeId;
188+
}
189+
190+
auto decl = context.insts().Get(decl_constant_inst_id);
186191
if (auto assoc_const = decl.TryAs<SemIR::AssociatedConstantDecl>()) {
187192
// Form a specific for the associated constant, and grab the type from
188193
// there.
@@ -195,8 +200,9 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id,
195200
auto const_specific_id = MakeSpecific(context, loc_id, generic_id, arg_ids);
196201
return SemIR::GetTypeOfInstInSpecific(context.sem_ir(), const_specific_id,
197202
decl_id);
198-
} else if (auto fn = context.types().TryGetAs<SemIR::FunctionType>(
199-
decl.type_id())) {
203+
}
204+
205+
if (auto fn = context.types().TryGetAs<SemIR::FunctionType>(decl.type_id())) {
200206
// Form the type of the function within the interface, and attach the `Self`
201207
// type.
202208
auto interface_fn_type_id = SemIR::GetTypeOfInstInSpecific(
@@ -208,9 +214,9 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id,
208214
return GetFunctionTypeWithSelfType(
209215
context, context.types().GetInstId(interface_fn_type_id),
210216
self_facet_id);
211-
} else {
212-
CARBON_FATAL("Unexpected kind for associated constant {0}", decl);
213217
}
218+
219+
CARBON_FATAL("Unexpected kind for associated constant {0}", decl);
214220
}
215221

216222
} // namespace Carbon::Check

‎toolchain/check/testdata/impl/using_invalid_interface.carbon‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
2+
// Exceptions. See /LICENSE for license information.
3+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
4+
//
5+
// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/convert.carbon
6+
//
7+
// AUTOUPDATE
8+
// TIP: To test this file alone, run:
9+
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/using_invalid_interface.carbon
10+
// TIP: To dump output, run:
11+
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/using_invalid_interface.carbon
12+
13+
// --- fail_invalid_interface.carbon
14+
library "[[@TEST_NAME]]";
15+
16+
interface I {}
17+
18+
class ArgToI(T:! I) {}
19+
20+
interface IMisuse {
21+
let Arg:! type;
22+
// CHECK:STDERR: fail_invalid_interface.carbon:[[@LINE+7]]:26: error: cannot convert type `Arg` into type implementing `I` [ConversionFailureTypeToFacet]
23+
// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(Arg);
24+
// CHECK:STDERR: ^~~~~~~~~~~
25+
// CHECK:STDERR: fail_invalid_interface.carbon:[[@LINE-7]]:14: note: initializing generic parameter `T` declared here [InitializingGenericParam]
26+
// CHECK:STDERR: class ArgToI(T:! I) {}
27+
// CHECK:STDERR: ^
28+
// CHECK:STDERR:
29+
fn Op[self: Self]() -> ArgToI(Arg);
30+
}
31+
32+
// --- fail_misusing_invalid_interface.carbon
33+
library "[[@TEST_NAME]]";
34+
// This import boundary is significant.
35+
import library "invalid_interface";
36+
37+
class C {
38+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+4]]:32: error: name `T` not found [NameNotFound]
39+
// CHECK:STDERR: impl as IMisuse where .Arg = T {
40+
// CHECK:STDERR: ^
41+
// CHECK:STDERR:
42+
impl as IMisuse where .Arg = T {
43+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+4]]:35: error: name `T` not found [NameNotFound]
44+
// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(T) {
45+
// CHECK:STDERR: ^
46+
// CHECK:STDERR:
47+
fn Op[self: Self]() -> ArgToI(T) {
48+
return {};
49+
}
50+
}
51+
}
52+
53+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+8]]:21: error: missing object argument in method call [MissingObjectInMethodCall]
54+
// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
55+
// CHECK:STDERR: ^~~~~~
56+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE-21]]:1: in import [InImport]
57+
// CHECK:STDERR: fail_invalid_interface.carbon:16:3: note: calling function declared here [InCallToFunction]
58+
// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(Arg);
59+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
60+
// CHECK:STDERR:
61+
fn F(T:! IMisuse) { T.Op(); }
62+
63+
fn G() {
64+
var c: C;
65+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+10]]:3: error: cannot implicitly convert non-type value of type `C` into type implementing `IMisuse` [ConversionFailureNonTypeToFacet]
66+
// CHECK:STDERR: F(c);
67+
// CHECK:STDERR: ^~~~
68+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+7]]:3: note: type `C` does not implement interface `Core.ImplicitAs(IMisuse)` [MissingImplInMemberAccessNote]
69+
// CHECK:STDERR: F(c);
70+
// CHECK:STDERR: ^~~~
71+
// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE-10]]:6: note: initializing generic parameter `T` declared here [InitializingGenericParam]
72+
// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
73+
// CHECK:STDERR: ^
74+
// CHECK:STDERR:
75+
F(c);
76+
}
77+
78+
// --- fail_using_invalid_interface.carbon
79+
library "[[@TEST_NAME]]";
80+
// This import boundary is significant.
81+
import library "invalid_interface";
82+
83+
class ArgC {}
84+
85+
class C {
86+
impl as IMisuse where .Arg = ArgC {
87+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+8]]:28: error: cannot convert type `ArgC` into type implementing `I` [ConversionFailureTypeToFacet]
88+
// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(ArgC) {
89+
// CHECK:STDERR: ^~~~~~~~~~~~
90+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-9]]:1: in import [InImport]
91+
// CHECK:STDERR: fail_invalid_interface.carbon:5:14: note: initializing generic parameter `T` declared here [InitializingGenericParam]
92+
// CHECK:STDERR: class ArgToI(T:! I) {}
93+
// CHECK:STDERR: ^
94+
// CHECK:STDERR:
95+
fn Op[self: Self]() -> ArgToI(ArgC) {
96+
return {};
97+
}
98+
}
99+
}
100+
101+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+8]]:21: error: missing object argument in method call [MissingObjectInMethodCall]
102+
// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
103+
// CHECK:STDERR: ^~~~~~
104+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-23]]:1: in import [InImport]
105+
// CHECK:STDERR: fail_invalid_interface.carbon:16:3: note: calling function declared here [InCallToFunction]
106+
// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(Arg);
107+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
108+
// CHECK:STDERR:
109+
fn F(T:! IMisuse) { T.Op(); }
110+
111+
fn G() {
112+
var c: C;
113+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+10]]:3: error: cannot implicitly convert non-type value of type `C` into type implementing `IMisuse` [ConversionFailureNonTypeToFacet]
114+
// CHECK:STDERR: F(c);
115+
// CHECK:STDERR: ^~~~
116+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+7]]:3: note: type `C` does not implement interface `Core.ImplicitAs(IMisuse)` [MissingImplInMemberAccessNote]
117+
// CHECK:STDERR: F(c);
118+
// CHECK:STDERR: ^~~~
119+
// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-10]]:6: note: initializing generic parameter `T` declared here [InitializingGenericParam]
120+
// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); }
121+
// CHECK:STDERR: ^
122+
// CHECK:STDERR:
123+
F(c);
124+
}

0 commit comments

Comments
 (0)