Ver Fonte

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 <danakj@orodu.net>
Jon Ross-Perkins há 5 meses atrás
pai
commit
01a7c79c41
1 ficheiros alterados com 71 adições e 56 exclusões
  1. 71 56
      toolchain/check/import_ref.cpp

+ 71 - 56
toolchain/check/import_ref.cpp

@@ -2965,37 +2965,83 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
                  .facet_value_inst_id = facet_value_inst_id});
 }
 
-static auto TryResolveTypedInst(ImportRefResolver& resolver,
-                                SemIR::FacetType inst) -> ResolveResult {
-  CARBON_CHECK(inst.type_id == SemIR::TypeType::TypeId);
-
-  const SemIR::FacetTypeInfo& import_facet_type_info =
-      resolver.import_facet_types().Get(inst.facet_type_id);
+// Collects and assigns constants for a `FacetTypeInfo`. Discards constants when
+// `local_facet_type_info` is null.
+static auto ResolveFacetTypeInfo(
+    ImportRefResolver& resolver,
+    const SemIR::FacetTypeInfo& import_facet_type_info,
+    SemIR::FacetTypeInfo* local_facet_type_info) -> void {
+  if (local_facet_type_info) {
+    local_facet_type_info->extend_constraints.reserve(
+        import_facet_type_info.extend_constraints.size());
+  }
   for (auto interface : import_facet_type_info.extend_constraints) {
-    // We discard this here and recompute it below instead of saving it to avoid
-    // allocations.
-    GetLocalSpecificInterfaceData(resolver, interface);
+    auto data = GetLocalSpecificInterfaceData(resolver, interface);
+    if (local_facet_type_info) {
+      local_facet_type_info->extend_constraints.push_back(
+          GetLocalSpecificInterface(resolver, interface, data));
+    }
+  }
+
+  if (local_facet_type_info) {
+    local_facet_type_info->self_impls_constraints.reserve(
+        import_facet_type_info.self_impls_constraints.size());
   }
   for (auto interface : import_facet_type_info.self_impls_constraints) {
-    // We discard this here and recompute it below instead of saving it to avoid
-    // allocations.
-    GetLocalSpecificInterfaceData(resolver, interface);
+    auto data = GetLocalSpecificInterfaceData(resolver, interface);
+    if (local_facet_type_info) {
+      local_facet_type_info->self_impls_constraints.push_back(
+          GetLocalSpecificInterface(resolver, interface, data));
+    }
+  }
+
+  if (local_facet_type_info) {
+    local_facet_type_info->extend_named_constraints.reserve(
+        import_facet_type_info.extend_named_constraints.size());
   }
   for (auto constraint : import_facet_type_info.extend_named_constraints) {
-    // We discard this here and recompute it below instead of saving it to avoid
-    // allocations.
-    GetLocalSpecificNamedConstraintData(resolver, constraint);
+    auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
+    if (local_facet_type_info) {
+      local_facet_type_info->extend_named_constraints.push_back(
+          GetLocalSpecificNamedConstraint(resolver, constraint, data));
+    }
+  }
+
+  if (local_facet_type_info) {
+    local_facet_type_info->self_impls_named_constraints.reserve(
+        import_facet_type_info.self_impls_named_constraints.size());
   }
   for (auto constraint : import_facet_type_info.self_impls_named_constraints) {
-    // We discard this here and recompute it below instead of saving it to avoid
-    // allocations.
-    GetLocalSpecificNamedConstraintData(resolver, constraint);
+    auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
+    if (local_facet_type_info) {
+      local_facet_type_info->self_impls_named_constraints.push_back(
+          GetLocalSpecificNamedConstraint(resolver, constraint, data));
+    }
+  }
+
+  if (local_facet_type_info) {
+    local_facet_type_info->rewrite_constraints.reserve(
+        import_facet_type_info.rewrite_constraints.size());
   }
   for (auto rewrite : import_facet_type_info.rewrite_constraints) {
-    GetLocalConstantInstId(resolver, rewrite.lhs_id);
-    GetLocalConstantInstId(resolver, rewrite.rhs_id);
+    auto lhs_id = GetLocalConstantInstId(resolver, rewrite.lhs_id);
+    auto rhs_id = GetLocalConstantInstId(resolver, rewrite.rhs_id);
+    if (local_facet_type_info) {
+      local_facet_type_info->rewrite_constraints.push_back(
+          {.lhs_id = lhs_id, .rhs_id = rhs_id});
+    }
   }
-  // TODO: Import named constraints in the facet type.
+}
+
+static auto TryResolveTypedInst(ImportRefResolver& resolver,
+                                SemIR::FacetType inst) -> ResolveResult {
+  CARBON_CHECK(inst.type_id == SemIR::TypeType::TypeId);
+
+  const SemIR::FacetTypeInfo& import_facet_type_info =
+      resolver.import_facet_types().Get(inst.facet_type_id);
+  // Ensure values are imported, but discard them to avoid allocations.
+  ResolveFacetTypeInfo(resolver, import_facet_type_info,
+                       /*local_facet_type_info=*/nullptr);
   if (resolver.HasNewWork()) {
     return ResolveResult::Retry();
   }
@@ -3004,41 +3050,10 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
       .builtin_constraint_mask = import_facet_type_info.builtin_constraint_mask,
       // TODO: Also process the other requirements.
       .other_requirements = import_facet_type_info.other_requirements};
-  local_facet_type_info.extend_constraints.reserve(
-      import_facet_type_info.extend_constraints.size());
-  for (auto interface : import_facet_type_info.extend_constraints) {
-    auto data = GetLocalSpecificInterfaceData(resolver, interface);
-    local_facet_type_info.extend_constraints.push_back(
-        GetLocalSpecificInterface(resolver, interface, data));
-  }
-  local_facet_type_info.self_impls_constraints.reserve(
-      import_facet_type_info.self_impls_constraints.size());
-  for (auto interface : import_facet_type_info.self_impls_constraints) {
-    auto data = GetLocalSpecificInterfaceData(resolver, interface);
-    local_facet_type_info.self_impls_constraints.push_back(
-        GetLocalSpecificInterface(resolver, interface, data));
-  }
-  local_facet_type_info.extend_named_constraints.reserve(
-      import_facet_type_info.extend_named_constraints.size());
-  for (auto constraint : import_facet_type_info.extend_named_constraints) {
-    auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
-    local_facet_type_info.extend_named_constraints.push_back(
-        GetLocalSpecificNamedConstraint(resolver, constraint, data));
-  }
-  local_facet_type_info.self_impls_named_constraints.reserve(
-      import_facet_type_info.self_impls_named_constraints.size());
-  for (auto constraint : import_facet_type_info.self_impls_named_constraints) {
-    auto data = GetLocalSpecificNamedConstraintData(resolver, constraint);
-    local_facet_type_info.self_impls_named_constraints.push_back(
-        GetLocalSpecificNamedConstraint(resolver, constraint, data));
-  }
-  local_facet_type_info.rewrite_constraints.reserve(
-      import_facet_type_info.rewrite_constraints.size());
-  for (auto rewrite : import_facet_type_info.rewrite_constraints) {
-    local_facet_type_info.rewrite_constraints.push_back(
-        {.lhs_id = GetLocalConstantInstId(resolver, rewrite.lhs_id),
-         .rhs_id = GetLocalConstantInstId(resolver, rewrite.rhs_id)});
-  }
+  // Re-resolve and add values to the local `FacetTypeInfo`.
+  ResolveFacetTypeInfo(resolver, import_facet_type_info,
+                       &local_facet_type_info);
+
   SemIR::FacetTypeId facet_type_id =
       resolver.local_facet_types().Add(std::move(local_facet_type_info));
   return ResolveResult::Deduplicated<SemIR::FacetType>(