Skip to content

Commit 01a7c79

Browse files
jonmeowdanakj
andauthored
Proposing helpers to reduce some facet type boilerplate (#6412)
About the same # of LOC, but maybe less work to analyze correctness? Versus the template, could also stamp that out in the helper function and still avoid the duplication of calls before/after HasNewWork. Similar to how I've left `rewrite_constraints`. Alternately I'm also kind of tempted to rename GetLocalSpecificInterface and GetLocalSpecificNamedConstraint to instead be overloaded functions (or to provide overloaded versions), which would allow this to drop the function type parameters. But, naming is hard. --------- Co-authored-by: Dana Jansens <[email protected]>
1 parent 972854e commit 01a7c79

File tree

1 file changed

+71
-56
lines changed

1 file changed

+71
-56
lines changed

toolchain/check/import_ref.cpp

Lines changed: 71 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2965,37 +2965,83 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
29652965
.facet_value_inst_id = facet_value_inst_id});
29662966
}
29672967

2968-
static auto TryResolveTypedInst(ImportRefResolver& resolver,
2969-
SemIR::FacetType inst) -> ResolveResult {
2970-
CARBON_CHECK(inst.type_id == SemIR::TypeType::TypeId);
2971-
2972-
const SemIR::FacetTypeInfo& import_facet_type_info =
2973-
resolver.import_facet_types().Get(inst.facet_type_id);
2968+
// Collects and assigns constants for a `FacetTypeInfo`. Discards constants when
2969+
// `local_facet_type_info` is null.
2970+
static auto ResolveFacetTypeInfo(
2971+
ImportRefResolver& resolver,
2972+
const SemIR::FacetTypeInfo& import_facet_type_info,
2973+
SemIR::FacetTypeInfo* local_facet_type_info) -> void {
2974+
if (local_facet_type_info) {
2975+
local_facet_type_info->extend_constraints.reserve(
2976+
import_facet_type_info.extend_constraints.size());
2977+
}
29742978
for (auto interface : import_facet_type_info.extend_constraints) {
2975-
// We discard this here and recompute it below instead of saving it to avoid
2976-
// allocations.
2977-
GetLocalSpecificInterfaceData(resolver, interface);
2979+
auto data = GetLocalSpecificInterfaceData(resolver, interface);
2980+
if (local_facet_type_info) {
2981+
local_facet_type_info->extend_constraints.push_back(
2982+
GetLocalSpecificInterface(resolver, interface, data));
2983+
}
2984+
}
2985+
2986+
if (local_facet_type_info) {
2987+
local_facet_type_info->self_impls_constraints.reserve(
2988+
import_facet_type_info.self_impls_constraints.size());
29782989
}
29792990
for (auto interface : import_facet_type_info.self_impls_constraints) {
2980-
// We discard this here and recompute it below instead of saving it to avoid
2981-
// allocations.
2982-
GetLocalSpecificInterfaceData(resolver, interface);
2991+
auto data = GetLocalSpecificInterfaceData(resolver, interface);
2992+
if (local_facet_type_info) {
2993+
local_facet_type_info->self_impls_constraints.push_back(
2994+
GetLocalSpecificInterface(resolver, interface, data));
2995+
}
2996+
}
2997+
2998+
if (local_facet_type_info) {
2999+
local_facet_type_info->extend_named_constraints.reserve(
3000+
import_facet_type_info.extend_named_constraints.size());
29833001
}
29843002
for (auto constraint : import_facet_type_info.extend_named_constraints) {
2985-
// We discard this here and recompute it below instead of saving it to avoid
2986-
// allocations.
2987-
GetLocalSpecificNamedConstraintData(resolver, constraint);
3003+
auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
3004+
if (local_facet_type_info) {
3005+
local_facet_type_info->extend_named_constraints.push_back(
3006+
GetLocalSpecificNamedConstraint(resolver, constraint, data));
3007+
}
3008+
}
3009+
3010+
if (local_facet_type_info) {
3011+
local_facet_type_info->self_impls_named_constraints.reserve(
3012+
import_facet_type_info.self_impls_named_constraints.size());
29883013
}
29893014
for (auto constraint : import_facet_type_info.self_impls_named_constraints) {
2990-
// We discard this here and recompute it below instead of saving it to avoid
2991-
// allocations.
2992-
GetLocalSpecificNamedConstraintData(resolver, constraint);
3015+
auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
3016+
if (local_facet_type_info) {
3017+
local_facet_type_info->self_impls_named_constraints.push_back(
3018+
GetLocalSpecificNamedConstraint(resolver, constraint, data));
3019+
}
3020+
}
3021+
3022+
if (local_facet_type_info) {
3023+
local_facet_type_info->rewrite_constraints.reserve(
3024+
import_facet_type_info.rewrite_constraints.size());
29933025
}
29943026
for (auto rewrite : import_facet_type_info.rewrite_constraints) {
2995-
GetLocalConstantInstId(resolver, rewrite.lhs_id);
2996-
GetLocalConstantInstId(resolver, rewrite.rhs_id);
3027+
auto lhs_id = GetLocalConstantInstId(resolver, rewrite.lhs_id);
3028+
auto rhs_id = GetLocalConstantInstId(resolver, rewrite.rhs_id);
3029+
if (local_facet_type_info) {
3030+
local_facet_type_info->rewrite_constraints.push_back(
3031+
{.lhs_id = lhs_id, .rhs_id = rhs_id});
3032+
}
29973033
}
2998-
// TODO: Import named constraints in the facet type.
3034+
}
3035+
3036+
static auto TryResolveTypedInst(ImportRefResolver& resolver,
3037+
SemIR::FacetType inst) -> ResolveResult {
3038+
CARBON_CHECK(inst.type_id == SemIR::TypeType::TypeId);
3039+
3040+
const SemIR::FacetTypeInfo& import_facet_type_info =
3041+
resolver.import_facet_types().Get(inst.facet_type_id);
3042+
// Ensure values are imported, but discard them to avoid allocations.
3043+
ResolveFacetTypeInfo(resolver, import_facet_type_info,
3044+
/*local_facet_type_info=*/nullptr);
29993045
if (resolver.HasNewWork()) {
30003046
return ResolveResult::Retry();
30013047
}
@@ -3004,41 +3050,10 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
30043050
.builtin_constraint_mask = import_facet_type_info.builtin_constraint_mask,
30053051
// TODO: Also process the other requirements.
30063052
.other_requirements = import_facet_type_info.other_requirements};
3007-
local_facet_type_info.extend_constraints.reserve(
3008-
import_facet_type_info.extend_constraints.size());
3009-
for (auto interface : import_facet_type_info.extend_constraints) {
3010-
auto data = GetLocalSpecificInterfaceData(resolver, interface);
3011-
local_facet_type_info.extend_constraints.push_back(
3012-
GetLocalSpecificInterface(resolver, interface, data));
3013-
}
3014-
local_facet_type_info.self_impls_constraints.reserve(
3015-
import_facet_type_info.self_impls_constraints.size());
3016-
for (auto interface : import_facet_type_info.self_impls_constraints) {
3017-
auto data = GetLocalSpecificInterfaceData(resolver, interface);
3018-
local_facet_type_info.self_impls_constraints.push_back(
3019-
GetLocalSpecificInterface(resolver, interface, data));
3020-
}
3021-
local_facet_type_info.extend_named_constraints.reserve(
3022-
import_facet_type_info.extend_named_constraints.size());
3023-
for (auto constraint : import_facet_type_info.extend_named_constraints) {
3024-
auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
3025-
local_facet_type_info.extend_named_constraints.push_back(
3026-
GetLocalSpecificNamedConstraint(resolver, constraint, data));
3027-
}
3028-
local_facet_type_info.self_impls_named_constraints.reserve(
3029-
import_facet_type_info.self_impls_named_constraints.size());
3030-
for (auto constraint : import_facet_type_info.self_impls_named_constraints) {
3031-
auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
3032-
local_facet_type_info.self_impls_named_constraints.push_back(
3033-
GetLocalSpecificNamedConstraint(resolver, constraint, data));
3034-
}
3035-
local_facet_type_info.rewrite_constraints.reserve(
3036-
import_facet_type_info.rewrite_constraints.size());
3037-
for (auto rewrite : import_facet_type_info.rewrite_constraints) {
3038-
local_facet_type_info.rewrite_constraints.push_back(
3039-
{.lhs_id = GetLocalConstantInstId(resolver, rewrite.lhs_id),
3040-
.rhs_id = GetLocalConstantInstId(resolver, rewrite.rhs_id)});
3041-
}
3053+
// Re-resolve and add values to the local `FacetTypeInfo`.
3054+
ResolveFacetTypeInfo(resolver, import_facet_type_info,
3055+
&local_facet_type_info);
3056+
30423057
SemIR::FacetTypeId facet_type_id =
30433058
resolver.local_facet_types().Add(std::move(local_facet_type_info));
30443059
return ResolveResult::Deduplicated<SemIR::FacetType>(

0 commit comments

Comments
 (0)