Skip to content

Commit 201e408

Browse files
danakjdanakj
authored
Type completion of facet types is separate from Identifying (#6385)
Identifying a facet type is an operation on a pair of (self type, facet type). It substitutes that self in as the `Self` of any require declarations in order to form the set of (self type, SpecificInterface) pairs that constitute the requirements of the IdentifiedFacetType. Currently we don't pass around any self type, and assume all require declarations are written against `Self` but this will change in the future. By contrast, type completion is done in the abstract and does not form specifics for the require declarations. The purpose of type completion is to enumerate the scopes where name lookup can occur and ensure they are completed. With this change, type completion is: - No longer built on top of identification for facet types. - Recursively ensures all `extend` scopes are complete since name lookup can find symbols in them. We add some test cases that demonstrate consistency between a resolving the specific of a generic class, and a generic interface/constraint, both used in a type position. In all cases, an invalid specific is not materialized for the type completion when the specific's arguments are used in a non-extend context. But they specific is materialized and checked for type completion when in an extend context (extend impl or extend require). Type completion itself does not need to recurse into named constraints or interfaces as the `extend require` declarations require the type to be complete immediately, just as for `extend impl` in a class. We had a test (`fail_incomplete_where.carbon`) with `impl as J where .Self impls K` and `J` is incomplete, which used to be diagnosed but no longer is, because we don't require non-extend interfaces to be complete in type completion, nor in identification. The test was trying to test the presence of rewrite constraints though, which it didn't even use. So we remove the diagnostic that we can't hit anymore and replaced it with a TODO, and add a test that should reach that TODO once qualified rewrite constraints work.
1 parent 56bbced commit 201e408

17 files changed

+1827
-534
lines changed

‎toolchain/check/eval.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,14 @@ static auto ResolveSpecificDeclForInst(EvalContext& eval_context,
867867
for (const auto& interface : info.self_impls_constraints) {
868868
ResolveSpecificDeclForSpecificId(eval_context, interface.specific_id);
869869
}
870+
for (const auto& constraint : info.extend_named_constraints) {
871+
ResolveSpecificDeclForSpecificId(eval_context,
872+
constraint.specific_id);
873+
}
874+
for (const auto& constraint : info.self_impls_named_constraints) {
875+
ResolveSpecificDeclForSpecificId(eval_context,
876+
constraint.specific_id);
877+
}
870878
break;
871879
}
872880
case CARBON_KIND(SemIR::SpecificId specific_id): {

‎toolchain/check/facet_type.cpp‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,15 @@ static auto WitnessQueryMatchesInterface(
5555
static auto IncompleteFacetTypeDiagnosticBuilder(
5656
Context& context, SemIR::LocId loc_id, SemIR::TypeInstId facet_type_inst_id,
5757
bool is_definition) -> DiagnosticBuilder {
58-
if (is_definition) {
59-
CARBON_DIAGNOSTIC(ImplAsIncompleteFacetTypeDefinition, Error,
60-
"definition of impl as incomplete facet type {0}",
61-
InstIdAsType);
62-
return context.emitter().Build(loc_id, ImplAsIncompleteFacetTypeDefinition,
63-
facet_type_inst_id);
64-
} else {
65-
CARBON_DIAGNOSTIC(
66-
ImplAsIncompleteFacetTypeRewrites, Error,
67-
"declaration of impl as incomplete facet type {0} with rewrites",
68-
InstIdAsType);
69-
return context.emitter().Build(loc_id, ImplAsIncompleteFacetTypeRewrites,
70-
facet_type_inst_id);
71-
}
58+
// TODO: Remove this parameter. Facet types don't need to be complete for impl
59+
// declarations, unless there's a rewrite into `.Self`. But that completeness
60+
// is checked/required by the member access of the rewrite.
61+
CARBON_CHECK(is_definition);
62+
CARBON_DIAGNOSTIC(ImplAsIncompleteFacetTypeDefinition, Error,
63+
"definition of impl as incomplete facet type {0}",
64+
InstIdAsType);
65+
return context.emitter().Build(loc_id, ImplAsIncompleteFacetTypeDefinition,
66+
facet_type_inst_id);
7267
}
7368

7469
auto GetImplWitnessAccessWithoutSubstitution(Context& context,
@@ -105,6 +100,9 @@ auto InitialFacetTypeImplWitness(
105100
.specific_id = self_specific_id});
106101
}
107102

103+
// The presence of any rewrite constraints requires that we know how many
104+
// entries to allocate in the witness table, which requires the entire facet
105+
// type to be complete, even if this was a declaration.
108106
if (!RequireCompleteType(
109107
context, facet_type_id, SemIR::LocId(facet_type_inst_id), [&] {
110108
return IncompleteFacetTypeDiagnosticBuilder(

‎toolchain/check/handle_require.cpp‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ static auto TypeStructureReferencesSelf(
154154

155155
struct ValidateRequireResult {
156156
SemIR::FacetType facet_type;
157+
SemIR::TypeId facet_type_type_id;
157158
const SemIR::IdentifiedFacetType* identified;
158159
};
159160

@@ -164,12 +165,14 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
164165
SemIR::InstId constraint_inst_id,
165166
SemIR::InstId scope_inst_id)
166167
-> std::optional<ValidateRequireResult> {
167-
auto constraint_constant_value_inst_id =
168-
context.constant_values().GetConstantInstId(constraint_inst_id);
169-
auto constraint_facet_type = context.insts().TryGetAs<SemIR::FacetType>(
170-
constraint_constant_value_inst_id);
168+
auto constraint_constant_value_id =
169+
context.constant_values().Get(constraint_inst_id);
170+
auto constraint_type_id =
171+
SemIR::TypeId::ForTypeConstant(constraint_constant_value_id);
172+
auto constraint_facet_type =
173+
context.types().TryGetAs<SemIR::FacetType>(constraint_type_id);
171174
if (!constraint_facet_type) {
172-
if (constraint_constant_value_inst_id != SemIR::ErrorInst::InstId) {
175+
if (constraint_constant_value_id != SemIR::ErrorInst::ConstantId) {
173176
CARBON_DIAGNOSTIC(
174177
RequireImplsMissingFacetType, Error,
175178
"`require` declaration constrained by a non-facet type; "
@@ -218,6 +221,7 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
218221
}
219222

220223
return ValidateRequireResult{.facet_type = *constraint_facet_type,
224+
.facet_type_type_id = constraint_type_id,
221225
.identified = &identified};
222226
}
223227

@@ -244,7 +248,7 @@ auto HandleParseNode(Context& context, Parse::RequireDeclId node_id) -> bool {
244248
return true;
245249
}
246250

247-
auto [constraint_facet_type, identified] = *validated;
251+
auto [constraint_facet_type, constraint_type_id, identified] = *validated;
248252
if (identified->required_interfaces().empty()) {
249253
// A `require T impls type` adds no actual constraints, so nothing to do.
250254
DiscardGenericDecl(context);
@@ -271,8 +275,27 @@ auto HandleParseNode(Context& context, Parse::RequireDeclId node_id) -> bool {
271275
require_impls_decl.require_impls_id = require_impls_id;
272276
ReplaceInstBeforeConstantUse(context, decl_id, require_impls_decl);
273277

274-
context.require_impls_stack().AppendToTop(require_impls_id);
278+
// We look for a complete type after BuildGenericDecl, so that the resulting
279+
// RequireCompleteType instruction is part of the enclosing interface or named
280+
// constraint generic definition. Then requiring enclosing entity to be
281+
// complete will resolve that definition (via ResolveSpecificDefinition()) and
282+
// also construct a specific for the `constraint_inst_id`, finding any
283+
// monomorphization errors that result.
284+
if (extend) {
285+
if (!RequireCompleteType(
286+
context, constraint_type_id, SemIR::LocId(constraint_inst_id), [&] {
287+
CARBON_DIAGNOSTIC(RequireImplsIncompleteFacetType, Error,
288+
"`extend require` of incomplete facet type {0}",
289+
InstIdAsType);
290+
return context.emitter().Build(constraint_inst_id,
291+
RequireImplsIncompleteFacetType,
292+
constraint_inst_id);
293+
})) {
294+
return true;
295+
}
296+
}
275297

298+
context.require_impls_stack().AppendToTop(require_impls_id);
276299
return true;
277300
}
278301

‎toolchain/check/interface.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ static auto GetGenericArgsWithSelfType(Context& context,
122122
arg_ids.reserve(std::max(reserve_args_size, interface_args.size() + 1));
123123

124124
// Start with the enclosing arguments from the interface.
125-
arg_ids.assign(interface_args.begin(), interface_args.end());
125+
llvm::append_range(arg_ids, interface_args);
126126

127127
// Add the `Self` argument.
128128
arg_ids.push_back(GetSelfFacet(context, interface_specific_id, generic_id,

‎toolchain/check/testdata/facet/fail_incomplete.carbon‎

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -10,99 +10,6 @@
1010
// TIP: To dump output, run:
1111
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/facet/fail_incomplete.carbon
1212

13-
// --- fail_incomplete_interface.carbon
14-
library "[[@TEST_NAME]]";
15-
16-
interface X;
17-
class C {}
18-
19-
// Requires X identified.
20-
impl C as X;
21-
22-
// Requires X complete.
23-
// CHECK:STDERR: fail_incomplete_interface.carbon:[[@LINE+7]]:1: error: definition of impl as incomplete facet type `X` [ImplAsIncompleteFacetTypeDefinition]
24-
// CHECK:STDERR: impl C as X {}
25-
// CHECK:STDERR: ^~~~~~~~~~~~~
26-
// CHECK:STDERR: fail_incomplete_interface.carbon:[[@LINE-10]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
27-
// CHECK:STDERR: interface X;
28-
// CHECK:STDERR: ^~~~~~~~~~~~
29-
// CHECK:STDERR:
30-
impl C as X {}
31-
32-
// --- fail_incomplete_interface_without_forward_decl.carbon
33-
library "[[@TEST_NAME]]";
34-
35-
interface X;
36-
class C {}
37-
38-
// Requires X complete.
39-
// CHECK:STDERR: fail_incomplete_interface_without_forward_decl.carbon:[[@LINE+7]]:1: error: definition of impl as incomplete facet type `X` [ImplAsIncompleteFacetTypeDefinition]
40-
// CHECK:STDERR: impl C as X {}
41-
// CHECK:STDERR: ^~~~~~~~~~~~~
42-
// CHECK:STDERR: fail_incomplete_interface_without_forward_decl.carbon:[[@LINE-7]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
43-
// CHECK:STDERR: interface X;
44-
// CHECK:STDERR: ^~~~~~~~~~~~
45-
// CHECK:STDERR:
46-
impl C as X {}
47-
48-
// --- fail_unidentified_constraint.carbon
49-
library "[[@TEST_NAME]]";
50-
51-
constraint X;
52-
class C {}
53-
54-
// Requires X identified.
55-
// CHECK:STDERR: fail_unidentified_constraint.carbon:[[@LINE+7]]:1: error: facet type `X` cannot be identified in `impl as` [ImplOfUnidentifiedFacetType]
56-
// CHECK:STDERR: impl C as X;
57-
// CHECK:STDERR: ^~~~~~~~~~~~
58-
// CHECK:STDERR: fail_unidentified_constraint.carbon:[[@LINE-7]]:1: note: constraint was forward declared here [NamedConstraintForwardDeclaredHere]
59-
// CHECK:STDERR: constraint X;
60-
// CHECK:STDERR: ^~~~~~~~~~~~~
61-
// CHECK:STDERR:
62-
impl C as X;
63-
64-
// --- nested_require_incomplete_interface.carbon
65-
library "[[@TEST_NAME]]";
66-
67-
interface Z;
68-
constraint Y {
69-
require impls Z;
70-
}
71-
interface X {
72-
require impls Y;
73-
}
74-
75-
class C {}
76-
77-
// Requires X identified.
78-
impl C as X;
79-
80-
// Requires X complete.
81-
impl C as X {}
82-
83-
// --- fail_incomplete_through_constraint.carbon
84-
library "[[@TEST_NAME]]";
85-
86-
interface Z;
87-
constraint Y {
88-
extend require impls Z;
89-
}
90-
91-
class C {}
92-
93-
// Requires Y identified.
94-
impl C as Y;
95-
96-
// Requires Y complete.
97-
// CHECK:STDERR: fail_incomplete_through_constraint.carbon:[[@LINE+7]]:1: error: definition of impl as incomplete facet type `Y` [ImplAsIncompleteFacetTypeDefinition]
98-
// CHECK:STDERR: impl C as Y {}
99-
// CHECK:STDERR: ^~~~~~~~~~~~~
100-
// CHECK:STDERR: fail_incomplete_through_constraint.carbon:[[@LINE-14]]:1: note: interface was forward declared here [InterfaceForwardDeclaredHere]
101-
// CHECK:STDERR: interface Z;
102-
// CHECK:STDERR: ^~~~~~~~~~~~
103-
// CHECK:STDERR:
104-
impl C as Y {}
105-
10613
// --- fail_impl_lookup_incomplete.carbon
10714
library "[[@TEST_NAME]]";
10815

‎toolchain/check/testdata/generic/extend_type_completion.carbon‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
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/int.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/generic/extend_type_completion.carbon
10+
// TIP: To dump output, run:
11+
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/generic/extend_type_completion.carbon
12+
13+
// --- class_impl_doesnt_need_complete_interface.carbon
14+
library "[[@TEST_NAME]]";
15+
16+
interface K(T:! type) {}
17+
18+
class C(N:! i32) {
19+
impl as K(array(i32, N)) {}
20+
}
21+
22+
// C does not extend K so the type of K is not completed. No error.
23+
var v: C(-1);
24+
25+
// --- fail_class_extend_impl_does_need_complete_interface.carbon
26+
library "[[@TEST_NAME]]";
27+
28+
interface K(T:! type) {}
29+
30+
class C(N:! i32) {
31+
// CHECK:STDERR: fail_class_extend_impl_does_need_complete_interface.carbon:[[@LINE+3]]:18: error: array bound of -1 is negative [ArrayBoundNegative]
32+
// CHECK:STDERR: extend impl as K(array(i32, N)) {}
33+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
34+
extend impl as K(array(i32, N)) {}
35+
}
36+
37+
// C extends K so the type of K is completed, but is invalid.
38+
// CHECK:STDERR: fail_class_extend_impl_does_need_complete_interface.carbon:[[@LINE+4]]:8: note: in `C(-1)` used here [ResolvingSpecificHere]
39+
// CHECK:STDERR: var v: C(-1);
40+
// CHECK:STDERR: ^~~~~
41+
// CHECK:STDERR:
42+
var v: C(-1);
43+
44+
// --- interface_require_impls_doesnt_need_complete_interface.carbon
45+
library "[[@TEST_NAME]]";
46+
47+
interface K(T:! type) {}
48+
interface J(N:! i32) {
49+
require impls K(array(i32, N));
50+
}
51+
52+
// J does not extend K so the type of K is not completed. No error.
53+
var v: J(-1);
54+
55+
// --- fail_interface_extend_require_impls_does_need_complete_interface.carbon
56+
library "[[@TEST_NAME]]";
57+
58+
interface K(T:! type) {}
59+
interface J(N:! i32) {
60+
// CHECK:STDERR: fail_interface_extend_require_impls_does_need_complete_interface.carbon:[[@LINE+3]]:24: error: array bound of -1 is negative [ArrayBoundNegative]
61+
// CHECK:STDERR: extend require impls K(array(i32, N));
62+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
63+
extend require impls K(array(i32, N));
64+
}
65+
interface I(N:! i32) {
66+
// CHECK:STDERR: fail_interface_extend_require_impls_does_need_complete_interface.carbon:[[@LINE+3]]:24: note: in `J(-1)` used here [ResolvingSpecificHere]
67+
// CHECK:STDERR: extend require impls J(N);
68+
// CHECK:STDERR: ^~~~
69+
extend require impls J(N);
70+
}
71+
72+
// I extends J extends K so the type of K is completed, but is invalid.
73+
//
74+
// TODO: The error location should be the type, like in the class case above. We
75+
// need a location for the type in context.bind_name_map() to use as the
76+
// location to Convert().
77+
//
78+
// CHECK:STDERR: fail_interface_extend_require_impls_does_need_complete_interface.carbon:[[@LINE+4]]:1: note: in `I(-1)` used here [ResolvingSpecificHere]
79+
// CHECK:STDERR: var v: I(-1);
80+
// CHECK:STDERR: ^~~~~~~~~~~~
81+
// CHECK:STDERR:
82+
var v: I(-1);
83+
84+
// --- constraint_require_impls_doesnt_need_complete_interface.carbon
85+
library "[[@TEST_NAME]]";
86+
87+
interface K(T:! type) {}
88+
constraint J(N:! i32) {
89+
require impls K(array(i32, N));
90+
}
91+
92+
// J does not extend K so the type of K is not completed. No error.
93+
var v: J(-1);
94+
95+
// --- fail_constraint_extend_require_impls_does_need_complete_interface.carbon
96+
library "[[@TEST_NAME]]";
97+
98+
interface K(T:! type) {}
99+
constraint J(N:! i32) {
100+
// CHECK:STDERR: fail_constraint_extend_require_impls_does_need_complete_interface.carbon:[[@LINE+3]]:24: error: array bound of -1 is negative [ArrayBoundNegative]
101+
// CHECK:STDERR: extend require impls K(array(i32, N));
102+
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
103+
extend require impls K(array(i32, N));
104+
}
105+
constraint I(N:! i32) {
106+
// CHECK:STDERR: fail_constraint_extend_require_impls_does_need_complete_interface.carbon:[[@LINE+3]]:24: note: in `{}` used here [ResolvingSpecificHere]
107+
// CHECK:STDERR: extend require impls J(N);
108+
// CHECK:STDERR: ^~~~
109+
extend require impls J(N);
110+
}
111+
112+
// I extends J extends K so the type of K is completed, but is invalid.
113+
// CHECK:STDERR: fail_constraint_extend_require_impls_does_need_complete_interface.carbon:[[@LINE+4]]:1: note: in `{}` used here [ResolvingSpecificHere]
114+
// CHECK:STDERR: var v: I(-1);
115+
// CHECK:STDERR: ^~~~~~~~~~~~
116+
// CHECK:STDERR:
117+
var v: I(-1);
118+
119+
// --- interface_require_impls_doesnt_need_complete_self.carbon
120+
library "[[@TEST_NAME]]";
121+
122+
interface K {}
123+
interface J(N:! i32) {
124+
require array(Self, N) impls K;
125+
}
126+
127+
// J does not extend K so the type of Self impling K is not completed. No error.
128+
var v: J(-1);
129+
130+
// --- constraint_require_impls_doesnt_need_complete_self.carbon
131+
library "[[@TEST_NAME]]";
132+
133+
interface K {}
134+
constraint J(N:! i32) {
135+
require array(Self, N) impls K;
136+
}
137+
138+
// J does not extend K so the type of Self impling K is not completed. No error.
139+
var v: J(-1);

0 commit comments

Comments
 (0)