Skip to content

Commit 5e3bb52

Browse files
jonmeowdanakj
andauthored
Add builtin functions for destroy, with special requirements in facet types (#6035)
This is in support of a goal of changing the blanket `destroy` impl to use (roughly): ``` private fn CanAggregateDestroy() -> type = "type.can_aggregate_destroy"; // Handles aggregate type destruction. impl forall [AggregateDestroyT:! CanAggregateDestroy()] AggregateDestroyT as Destroy { fn Op[addr self: Self*]() = "type.aggregate_destroy"; } ``` That isn't done here because there's still other issues that migrating raises. What this *does* do is add the builtin functions, and in particular, support to `FacetTypeInfo` to make `CanAggregateDestroy` work. The "special requirement" approach in `FacetTypeInfo` allows us to support restricting a blanket impl under the current approach of impls. Maybe we'll find a cleaner approach that can work in the future, but this fits into the current model by propagating similar to other requirements. I'm using an enum mask because we have a number of similar things to add (e.g. copy, move) but I'm not sure we need a full vector. A few alternatives considered were: - Supporting syntax more like `where .Self impls TypeCanAggregateDestroy(.Self, SupportedInterface, UnsupportedInterface)`. I think it'd be a little cleaner, but requires better compile-time evaluation in order to assess the type of the call. Right now it's expected to be a `FacetType` too early to make this work, and I was concerned about pouring too much more time down this route. - Providing an actual interface, in particular doing name lookup back into `Core.` for an interface. This would've added name lookup overhead, and the question of whether an `impl` exists. - Generating an interface. This avoids the name lookup, but would still raise the question of whether an `impl` should also be generated. Work I've previously done generating interfaces for class destruction also feels complex to both write and understand (an unfortunate issue). - Still modeling as an `ImplsConstraint`, for example by defining a special `InterfaceId::CanAggregateDestroy = -2` similar to what we do on other ids. I was hesitant because of how this expands the number of modes of `InterfaceId`, and things for consuming code to watch out for, for what feels like a relatively niche set of use-cases that are only interface-like. --------- Co-authored-by: Dana Jansens <[email protected]>
1 parent 3ec0bcb commit 5e3bb52

File tree

401 files changed

+20589
-20111
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

401 files changed

+20589
-20111
lines changed

toolchain/check/eval.cpp

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,10 @@ static auto GetConstantFacetTypeInfo(EvalContext& eval_context,
617617
SemIR::LocId loc_id,
618618
const SemIR::FacetTypeInfo& orig,
619619
Phase* phase) -> SemIR::FacetTypeInfo {
620-
SemIR::FacetTypeInfo info;
620+
SemIR::FacetTypeInfo info = {
621+
.builtin_constraint_mask = orig.builtin_constraint_mask,
622+
// TODO: Process other requirements.
623+
.other_requirements = orig.other_requirements};
621624

622625
info.extend_constraints.reserve(orig.extend_constraints.size());
623626
for (const auto& interface : orig.extend_constraints) {
@@ -661,9 +664,6 @@ static auto GetConstantFacetTypeInfo(EvalContext& eval_context,
661664
rewrite = {.lhs_id = lhs_id, .rhs_id = rhs_id};
662665
}
663666

664-
// TODO: Process other requirements.
665-
info.other_requirements = orig.other_requirements;
666-
667667
info.Canonicalize();
668668
return info;
669669
}
@@ -1646,7 +1646,8 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
16461646
case SemIR::BuiltinFunctionKind::None:
16471647
CARBON_FATAL("Not a builtin function.");
16481648

1649-
case SemIR::BuiltinFunctionKind::NoOp: {
1649+
case SemIR::BuiltinFunctionKind::NoOp:
1650+
case SemIR::BuiltinFunctionKind::TypeAggregateDestroy: {
16501651
// Return an empty tuple value.
16511652
auto type_id = GetTupleType(eval_context.context(), {});
16521653
return MakeConstantResult(
@@ -1656,6 +1657,18 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
16561657
phase);
16571658
}
16581659

1660+
case SemIR::BuiltinFunctionKind::TypeCanAggregateDestroy: {
1661+
CARBON_CHECK(arg_ids.empty());
1662+
auto id = eval_context.facet_types().Add(
1663+
{.builtin_constraint_mask =
1664+
SemIR::BuiltinConstraintMask::TypeCanAggregateDestroy});
1665+
return MakeConstantResult(
1666+
eval_context.context(),
1667+
SemIR::FacetType{.type_id = SemIR::TypeType::TypeId,
1668+
.facet_type_id = id},
1669+
phase);
1670+
}
1671+
16591672
case SemIR::BuiltinFunctionKind::PrimitiveCopy: {
16601673
return context.constant_values().Get(arg_ids[0]);
16611674
}
@@ -2183,7 +2196,7 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
21832196
auto typed_inst = inst.As<SemIR::WhereExpr>();
21842197

21852198
Phase phase = Phase::Concrete;
2186-
SemIR::FacetTypeInfo info = {.other_requirements = false};
2199+
SemIR::FacetTypeInfo info;
21872200

21882201
// Add the constraints from the `WhereExpr` instruction into `info`.
21892202
if (typed_inst.requirements_id.has_value()) {
@@ -2204,6 +2217,7 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
22042217
info.extend_constraints.append(base_info.extend_constraints);
22052218
info.self_impls_constraints.append(base_info.self_impls_constraints);
22062219
info.rewrite_constraints.append(base_info.rewrite_constraints);
2220+
info.builtin_constraint_mask.Add(base_info.builtin_constraint_mask);
22072221
info.other_requirements |= base_info.other_requirements;
22082222
}
22092223
} else if (auto rewrite =
@@ -2242,14 +2256,15 @@ auto TryEvalTypedInst<SemIR::WhereExpr>(EvalContext& eval_context,
22422256
// Other requirements are copied in.
22432257
llvm::append_range(info.rewrite_constraints,
22442258
more_info.rewrite_constraints);
2259+
info.builtin_constraint_mask.Add(more_info.builtin_constraint_mask);
22452260
info.other_requirements |= more_info.other_requirements;
22462261
}
22472262
} else {
22482263
// TODO: Handle `impls` constraints beyond `.Self impls`.
22492264
info.other_requirements = true;
22502265
}
22512266
} else {
2252-
// TODO: Handle other requirements
2267+
// TODO: Handle other requirements.
22532268
info.other_requirements = true;
22542269
}
22552270
}

toolchain/check/facet_type.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ namespace Carbon::Check {
2525
auto FacetTypeFromInterface(Context& context, SemIR::InterfaceId interface_id,
2626
SemIR::SpecificId specific_id) -> SemIR::FacetType {
2727
auto info =
28-
SemIR::FacetTypeInfo{.extend_constraints = {{interface_id, specific_id}},
29-
.other_requirements = false};
28+
SemIR::FacetTypeInfo{.extend_constraints = {{interface_id, specific_id}}};
3029
info.Canonicalize();
3130
SemIR::FacetTypeId facet_type_id = context.facet_types().Add(info);
3231
return {.type_id = SemIR::TypeType::TypeId, .facet_type_id = facet_type_id};

toolchain/check/impl_lookup.cpp

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -183,25 +183,32 @@ static auto FindAndDiagnoseImplLookupCycle(
183183
return false;
184184
}
185185

186+
struct InterfacesFromConstantId {
187+
llvm::SmallVector<SemIR::SpecificInterface> interfaces;
188+
SemIR::BuiltinConstraintMask builtin_constraint_mask;
189+
bool other_requirements;
190+
};
191+
186192
// Gets the set of `SpecificInterface`s that are required by a facet type
187-
// (as a constant value).
193+
// (as a constant value), and any special requirements.
188194
static auto GetInterfacesFromConstantId(
189-
Context& context, SemIR::ConstantId query_facet_type_const_id,
190-
bool& has_other_requirements)
191-
-> llvm::SmallVector<SemIR::SpecificInterface> {
195+
Context& context, SemIR::ConstantId query_facet_type_const_id)
196+
-> InterfacesFromConstantId {
192197
auto facet_type_inst_id =
193198
context.constant_values().GetInstId(query_facet_type_const_id);
194199
auto facet_type_inst =
195200
context.insts().GetAs<SemIR::FacetType>(facet_type_inst_id);
196201
const auto& facet_type_info =
197202
context.facet_types().Get(facet_type_inst.facet_type_id);
198-
has_other_requirements = facet_type_info.other_requirements;
199203
auto identified_id = RequireIdentifiedFacetType(context, facet_type_inst);
200204
auto interfaces_array_ref =
201205
context.identified_facet_types().Get(identified_id).required_interfaces();
202206
// Returns a copy to avoid use-after-free when the identified_facet_types
203207
// store resizes.
204-
return {interfaces_array_ref.begin(), interfaces_array_ref.end()};
208+
return {
209+
.interfaces = {interfaces_array_ref.begin(), interfaces_array_ref.end()},
210+
.builtin_constraint_mask = facet_type_info.builtin_constraint_mask,
211+
.other_requirements = facet_type_info.other_requirements};
205212
}
206213

207214
static auto GetWitnessIdForImpl(Context& context, SemIR::LocId loc_id,
@@ -257,8 +264,8 @@ static auto GetWitnessIdForImpl(Context& context, SemIR::LocId loc_id,
257264
CARBON_CHECK(deduced_constraint_facet_type_info.extend_constraints.size() ==
258265
1);
259266

260-
if (deduced_constraint_facet_type_info.other_requirements) {
261-
// TODO: Remove this when other requirements goes away.
267+
if (deduced_constraint_facet_type_info.other_requirements ||
268+
!deduced_constraint_facet_type_info.builtin_constraint_mask.empty()) {
262269
return EvalImplLookupResult::MakeNone();
263270
}
264271

@@ -535,6 +542,15 @@ static auto GetOrAddLookupImplWitness(Context& context, SemIR::LocId loc_id,
535542
return context.constant_values().GetInstId(witness_const_id);
536543
}
537544

545+
// Returns true if the `Self` supports aggregate destruction.
546+
static auto TypeCanAggregateDestroy(Context& context,
547+
SemIR::ConstantId query_self_const_id)
548+
-> bool {
549+
auto inst = context.insts().Get(
550+
context.constant_values().GetInstId(query_self_const_id));
551+
return inst.Is<SemIR::StructType>() || inst.Is<SemIR::TupleType>();
552+
}
553+
538554
auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
539555
SemIR::ConstantId query_self_const_id,
540556
SemIR::ConstantId query_facet_type_const_id)
@@ -557,13 +573,17 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
557573
context.constant_values().GetInstId(query_facet_type_const_id)));
558574
}
559575

560-
bool has_other_requirements = false;
561-
auto interfaces = GetInterfacesFromConstantId(
562-
context, query_facet_type_const_id, has_other_requirements);
563-
if (has_other_requirements) {
576+
auto [interfaces, builtin_constraint_mask, other_requirements] =
577+
GetInterfacesFromConstantId(context, query_facet_type_const_id);
578+
if (other_requirements) {
564579
// TODO: Remove this when other requirements go away.
565580
return SemIR::InstBlockId::None;
566581
}
582+
if (builtin_constraint_mask.HasAnyOf(
583+
SemIR::BuiltinConstraintMask::TypeCanAggregateDestroy) &&
584+
!TypeCanAggregateDestroy(context, query_self_const_id)) {
585+
return SemIR::InstBlockId::None;
586+
}
567587
if (interfaces.empty()) {
568588
return SemIR::InstBlockId::Empty;
569589
}

toolchain/check/import_ref.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2564,6 +2564,7 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
25642564
}
25652565

25662566
SemIR::FacetTypeInfo local_facet_type_info = {
2567+
.builtin_constraint_mask = import_facet_type_info.builtin_constraint_mask,
25672568
.other_requirements = import_facet_type_info.other_requirements};
25682569
local_facet_type_info.extend_constraints.reserve(
25692570
import_facet_type_info.extend_constraints.size());

toolchain/check/subst.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,10 @@ static auto PopOperand(Context& context, Worklist& worklist,
228228
case CARBON_KIND(SemIR::FacetTypeId facet_type_id): {
229229
const auto& old_facet_type_info =
230230
context.facet_types().Get(facet_type_id);
231-
SemIR::FacetTypeInfo new_facet_type_info;
231+
SemIR::FacetTypeInfo new_facet_type_info = {
232+
.builtin_constraint_mask =
233+
old_facet_type_info.builtin_constraint_mask,
234+
.other_requirements = old_facet_type_info.other_requirements};
232235
// Since these were added to a stack, we get them back in reverse order.
233236
new_facet_type_info.rewrite_constraints.resize(
234237
old_facet_type_info.rewrite_constraints.size(),
@@ -259,8 +262,6 @@ static auto PopOperand(Context& context, Worklist& worklist,
259262
.interface_id = old_constraint.interface_id,
260263
.specific_id = pop_specific(old_constraint.specific_id)};
261264
}
262-
new_facet_type_info.other_requirements =
263-
old_facet_type_info.other_requirements;
264265
new_facet_type_info.Canonicalize();
265266
return context.facet_types().Add(new_facet_type_info).index;
266267
}

toolchain/check/testdata/alias/export_name.carbon

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -318,15 +318,15 @@ var d: D* = &c;
318318
// CHECK:STDOUT: %Copy.type: type = facet_type <@Copy> [concrete]
319319
// CHECK:STDOUT: %Copy.Op.type: type = fn_type @Copy.Op [concrete]
320320
// CHECK:STDOUT: %T.8b3: type = bind_symbolic_name T, 0 [symbolic]
321-
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.type.f23: type = fn_type @ptr.as.Copy.impl.Op, @ptr.as.Copy.impl(%T.8b3) [symbolic]
322-
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.abf: %ptr.as.Copy.impl.Op.type.f23 = struct_value () [symbolic]
323-
// CHECK:STDOUT: %Copy.impl_witness.a94: <witness> = impl_witness imports.%Copy.impl_witness_table.a71, @ptr.as.Copy.impl(%C) [concrete]
324-
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.type.0f5: type = fn_type @ptr.as.Copy.impl.Op, @ptr.as.Copy.impl(%C) [concrete]
325-
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.fd1: %ptr.as.Copy.impl.Op.type.0f5 = struct_value () [concrete]
326-
// CHECK:STDOUT: %Copy.facet.9a6: %Copy.type = facet_value %ptr.019, (%Copy.impl_witness.a94) [concrete]
327-
// CHECK:STDOUT: %.a98: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet.9a6 [concrete]
328-
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.bound: <bound method> = bound_method %addr, %ptr.as.Copy.impl.Op.fd1 [concrete]
329-
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.specific_fn: <specific function> = specific_function %ptr.as.Copy.impl.Op.fd1, @ptr.as.Copy.impl.Op(%C) [concrete]
321+
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.type.31f: type = fn_type @ptr.as.Copy.impl.Op, @ptr.as.Copy.impl(%T.8b3) [symbolic]
322+
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.8a8: %ptr.as.Copy.impl.Op.type.31f = struct_value () [symbolic]
323+
// CHECK:STDOUT: %Copy.impl_witness.999: <witness> = impl_witness imports.%Copy.impl_witness_table.53c, @ptr.as.Copy.impl(%C) [concrete]
324+
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.type.c3f: type = fn_type @ptr.as.Copy.impl.Op, @ptr.as.Copy.impl(%C) [concrete]
325+
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.fb8: %ptr.as.Copy.impl.Op.type.c3f = struct_value () [concrete]
326+
// CHECK:STDOUT: %Copy.facet.9e3: %Copy.type = facet_value %ptr.019, (%Copy.impl_witness.999) [concrete]
327+
// CHECK:STDOUT: %.7e9: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet.9e3 [concrete]
328+
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.bound: <bound method> = bound_method %addr, %ptr.as.Copy.impl.Op.fb8 [concrete]
329+
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.specific_fn: <specific function> = specific_function %ptr.as.Copy.impl.Op.fb8, @ptr.as.Copy.impl.Op(%C) [concrete]
330330
// CHECK:STDOUT: %bound_method: <bound method> = bound_method %addr, %ptr.as.Copy.impl.Op.specific_fn [concrete]
331331
// CHECK:STDOUT: }
332332
// CHECK:STDOUT:
@@ -341,8 +341,8 @@ var d: D* = &c;
341341
// CHECK:STDOUT: %Main.import_ref.8db: <witness> = import_ref Main//export_orig, inst24 [indirect], loaded [concrete = constants.%complete_type.357]
342342
// CHECK:STDOUT: %Main.import_ref.6a9 = import_ref Main//export_orig, inst25 [indirect], unloaded
343343
// CHECK:STDOUT: %Core.Copy: type = import_ref Core//prelude/parts/copy, Copy, loaded [concrete = constants.%Copy.type]
344-
// CHECK:STDOUT: %Core.import_ref.de9: @ptr.as.Copy.impl.%ptr.as.Copy.impl.Op.type (%ptr.as.Copy.impl.Op.type.f23) = import_ref Core//prelude/parts/copy, loc36_31, loaded [symbolic = @ptr.as.Copy.impl.%ptr.as.Copy.impl.Op (constants.%ptr.as.Copy.impl.Op.abf)]
345-
// CHECK:STDOUT: %Copy.impl_witness_table.a71 = impl_witness_table (%Core.import_ref.de9), @ptr.as.Copy.impl [concrete]
344+
// CHECK:STDOUT: %Core.import_ref.0e4: @ptr.as.Copy.impl.%ptr.as.Copy.impl.Op.type (%ptr.as.Copy.impl.Op.type.31f) = import_ref Core//prelude/parts/copy, loc36_31, loaded [symbolic = @ptr.as.Copy.impl.%ptr.as.Copy.impl.Op (constants.%ptr.as.Copy.impl.Op.8a8)]
345+
// CHECK:STDOUT: %Copy.impl_witness_table.53c = impl_witness_table (%Core.import_ref.0e4), @ptr.as.Copy.impl [concrete]
346346
// CHECK:STDOUT: }
347347
// CHECK:STDOUT:
348348
// CHECK:STDOUT: file {
@@ -389,7 +389,7 @@ var d: D* = &c;
389389
// CHECK:STDOUT: assign file.%c.var, %.loc7_1
390390
// CHECK:STDOUT: %c.ref: ref %C = name_ref c, file.%c [concrete = file.%c.var]
391391
// CHECK:STDOUT: %addr: %ptr.019 = addr_of %c.ref [concrete = constants.%addr]
392-
// CHECK:STDOUT: %impl.elem0: %.a98 = impl_witness_access constants.%Copy.impl_witness.a94, element0 [concrete = constants.%ptr.as.Copy.impl.Op.fd1]
392+
// CHECK:STDOUT: %impl.elem0: %.7e9 = impl_witness_access constants.%Copy.impl_witness.999, element0 [concrete = constants.%ptr.as.Copy.impl.Op.fb8]
393393
// CHECK:STDOUT: %bound_method.loc8_13.1: <bound method> = bound_method %addr, %impl.elem0 [concrete = constants.%ptr.as.Copy.impl.Op.bound]
394394
// CHECK:STDOUT: %specific_fn: <specific function> = specific_function %impl.elem0, @ptr.as.Copy.impl.Op(constants.%C) [concrete = constants.%ptr.as.Copy.impl.Op.specific_fn]
395395
// CHECK:STDOUT: %bound_method.loc8_13.2: <bound method> = bound_method %addr, %specific_fn [concrete = constants.%bound_method]

0 commit comments

Comments
 (0)