Make FacetAccessType evaluate to SymbolicBindingType for type-of a BindSymbolicName

toolchain/check/convert.cpp

@@ -1208,8 +1208,10 @@ static auto PerformBuiltinConversion(
     } else {
       type_inst_id = context.types().GetAsTypeInstId(value_id);
 
-      // Shortcut for lossless round trips through a FacetAccessType when
-      // converting back to the type of its original facet value.
+      // Shortcut for lossless round trips through a FacetAccessType (which
+      // evaluates to SymbolicBindingType when wrapping a symbolic binding) when
+      // converting back to the type of the original symbolic binding facet
+      // value.
       //
       // In the case where the FacetAccessType wraps a BindSymbolicName with the
       // exact facet type that we are converting to, the resulting FacetValue
@@ -1219,16 +1221,10 @@ static auto PerformBuiltinConversion(
       //
       // TODO: This instruction is going to become a `SymbolicBindingType`, so
       // we'll need to handle that instead.
-      auto const_type_inst_id =
-          sem_ir.constant_values().GetConstantTypeInstId(type_inst_id);
-      if (auto facet_access_type_inst =
-              sem_ir.insts().TryGetAs<SemIR::FacetAccessType>(
-                  const_type_inst_id)) {
-        auto facet_value_inst_id = facet_access_type_inst->facet_value_inst_id;
-        if (sem_ir.insts().Get(facet_value_inst_id).type_id() ==
-            target.type_id) {
-          return facet_value_inst_id;
-        }
+      auto facet_value_inst_id =
+          GetCanonicalFacetOrTypeValue(context, type_inst_id);
+      if (sem_ir.insts().Get(facet_value_inst_id).type_id() == target.type_id) {
+        return facet_value_inst_id;
       }
     }
 

toolchain/check/eval.cpp

@@ -2171,6 +2171,82 @@ auto TryEvalTypedInst<SemIR::BindSymbolicName>(EvalContext& eval_context,
   return MakeConstantResult(eval_context.context(), bind, phase);
 }
 
+template <>
+auto TryEvalTypedInst<SemIR::SymbolicBindingType>(EvalContext& eval_context,
+                                                  SemIR::InstId inst_id,
+                                                  SemIR::Inst inst)
+    -> SemIR::ConstantId {
+  auto bind = inst.As<SemIR::SymbolicBindingType>();
+
+  Phase phase = Phase::Concrete;
+  bool updated_constants = false;
+
+  // If we know which specific we're evaluating within and this is the type
+  // component of a facet parameter of the generic, its constant value refers to
+  // the type component of the corresponding argument value of the specific.
+  const auto& bind_name = eval_context.entity_names().Get(bind.entity_name_id);
+  if (bind_name.bind_index().has_value()) {
+    // SymbolicBindingType comes from the evaluation of FacetAccessType when the
+    // facet value is symbolic. This block is effectively the deferred
+    // evaluation of that FacetAccessType now that a new value for the symbolic
+    // facet value has become known. The result is equivalent to creating a new
+    // FacetAccessType here with the `value_inst_id` and evaluating it.
+    if (auto value =
+            eval_context.GetCompileTimeBindValue(bind_name.bind_index());
+        value.has_value()) {
+      auto value_inst_id = eval_context.constant_values().GetInstId(value);
+      if (auto facet =
+              eval_context.insts().TryGetAs<SemIR::FacetValue>(value_inst_id)) {
+        return eval_context.constant_values().Get(facet->type_inst_id);
+      }
+
+      // Replace the fields with constant values as usual, except we get the
+      // EntityNameId from the BindSymbolicName in the specific, which
+      // ReplaceFieldWithConstantValue doesn't know how to do.
+      if (!ReplaceTypeWithConstantValue(eval_context, inst_id, &bind, &phase) ||
+          !ReplaceFieldWithConstantValue(
+              eval_context, &bind,
+              &SemIR::SymbolicBindingType::facet_value_inst_id, &phase)) {
+        return SemIR::ConstantId::NotConstant;
+      }
+
+      if (value_inst_id == SemIR::ErrorInst::InstId) {
+        phase = Phase::UnknownDueToError;
+      } else {
+        auto value_bind =
+            eval_context.insts().GetAs<SemIR::BindSymbolicName>(value_inst_id);
+        bind.entity_name_id =
+            GetConstantValue(eval_context, value_bind.entity_name_id, &phase);
+      }
+
+      updated_constants = true;
+    }
+  }
+
+  if (!updated_constants) {
+    if (!ReplaceTypeWithConstantValue(eval_context, inst_id, &inst, &phase) ||
+        !ReplaceAllFieldsWithConstantValues(eval_context, &inst, &phase)) {
+      return SemIR::ConstantId::NotConstant;
+    }
+    // Copy the updated constant field values into `bind`.
+    bind = inst.As<SemIR::SymbolicBindingType>();
+  }
+  // Propagate error phase after getting the constant value for all fields.
+  if (phase == Phase::UnknownDueToError) {
+    return SemIR::ErrorInst::ConstantId;
+  }
+
+  // TODO: Look in ScopeStack with the entity_name_id to find the facet value
+  // and get its constant value in the current specific context. The
+  // facet_value_inst_id will go away.
+  if (auto facet_value = eval_context.insts().TryGetAs<SemIR::FacetValue>(
+          bind.facet_value_inst_id)) {
+    return eval_context.constant_values().Get(facet_value->type_inst_id);
+  }
+
+  return MakeConstantResult(eval_context.context(), bind, phase);
+}
+
 // Returns whether `const_id` is the same constant facet value as
 // `facet_value_inst_id`.
 //

toolchain/check/eval_inst.cpp

@@ -200,20 +200,46 @@ auto EvalConstantInst(Context& context, SemIR::FacetAccessType inst)
     return ConstantEvalResult::Existing(
         context.constant_values().Get(facet_value->type_inst_id));
   }
+
+  if (auto bind_name = context.insts().TryGetAs<SemIR::BindSymbolicName>(
+          inst.facet_value_inst_id)) {
+    return ConstantEvalResult::NewSamePhase(SemIR::SymbolicBindingType{
+        .type_id = SemIR::TypeType::TypeId,
+        .entity_name_id = bind_name->entity_name_id,
+        // TODO: This is to be removed, at which point explore if we should
+        // replace NewSamePhase with NewAnyPhase (to make the constant value
+        // concrete). This is still a symbolic type though even if the inst
+        // doesn't contain a symbolic constant. Previously we crashed in CHECKs
+        // when we had a symbolic instruction with only an EntityNameId, due to
+        // it not changing in a generic eval block. Maybe that has improved in
+        // the latest version of this instruction. If it's not symbolic, then
+        // SubstConstantCallbacks and other Subst callers may need to handle
+        // looking through concrete instructions which would be unfortunate.
+        .facet_value_inst_id = inst.facet_value_inst_id});
+  }
+
+  // The `facet_value_inst_id` is always a facet value (has type facet type).
+  CARBON_CHECK(context.types().Is<SemIR::FacetType>(
+      context.insts().Get(inst.facet_value_inst_id).type_id()));
+
+  // Other instructions (e.g. ImplWitnessAccess) of type FacetType can appear
+  // here, in which case the constant inst is a FacetAccessType until those
+  // instructions resolve to one of the above.
   return ConstantEvalResult::NewSamePhase(inst);
 }
 
 auto EvalConstantInst(Context& context, SemIR::FacetValue inst)
     -> ConstantEvalResult {
   // A FacetValue that just wraps a BindSymbolicName without adding/removing any
   // witnesses is evaluated back to the BindSymbolicName itself.
-  if (auto access =
-          context.insts().TryGetAs<SemIR::FacetAccessType>(inst.type_inst_id)) {
-    auto bind_id = access->facet_value_inst_id;
-    auto bind = context.insts().TryGetAs<SemIR::BindSymbolicName>(bind_id);
+  if (auto bind_as_type = context.insts().TryGetAs<SemIR::SymbolicBindingType>(
+          inst.type_inst_id)) {
+    // TODO: Look in ScopeStack with the entity_name_id to find the facet value.
+    auto bind_id = bind_as_type->facet_value_inst_id;
+    auto bind = context.insts().GetAs<SemIR::BindSymbolicName>(bind_id);
     // If the FacetTypes are the same, then the FacetValue didn't add/remove
     // any witnesses.
-    if (bind.has_value() && bind->type_id == inst.type_id) {
+    if (bind.type_id == inst.type_id) {
       return ConstantEvalResult::Existing(
           context.constant_values().Get(bind_id));
     }

toolchain/check/import_ref.cpp

@@ -752,6 +752,8 @@ static auto GetLocalConstantId(ImportRefResolver& resolver,
 }
 
 // Translates a NameId from the import IR to a local NameId.
+//
+// No new work is generated by calling this function.
 static auto GetLocalNameId(ImportContext& context, SemIR::NameId import_name_id)
     -> SemIR::NameId {
   if (auto ident_id = import_name_id.AsIdentifierId(); ident_id.has_value()) {
@@ -761,6 +763,23 @@ static auto GetLocalNameId(ImportContext& context, SemIR::NameId import_name_id)
   return import_name_id;
 }
 
+// Returns the id for a local symbolic EntityName from an imported one,
+// preserving only the `NameId`, the `CompileTimeBindIndex`, and whether it is a
+// template. Other parts of the EntityName are not kept and are not considered
+// part of the canonical EntityName (even if they are present there).
+//
+// No new work is generated by calling this function.
+static auto GetLocalSymbolicEntityNameId(
+    ImportContext& context, SemIR::EntityNameId import_entity_name_id)
+    -> SemIR::EntityNameId {
+  const auto& import_entity_name =
+      context.import_entity_names().Get(import_entity_name_id);
+  auto name_id = GetLocalNameId(context, import_entity_name.name_id);
+  return context.local_entity_names().AddSymbolicBindingName(
+      name_id, SemIR::NameScopeId::None, import_entity_name.bind_index(),
+      import_entity_name.is_template);
+}
+
 // Gets the local constant values corresponding to an imported inst block.
 static auto GetLocalInstBlockContents(ImportRefResolver& resolver,
                                       SemIR::InstBlockId import_block_id)
@@ -1501,12 +1520,8 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     return ResolveResult::Retry();
   }
 
-  const auto& import_entity_name =
-      resolver.import_entity_names().Get(inst.entity_name_id);
-  auto name_id = GetLocalNameId(resolver, import_entity_name.name_id);
-  auto entity_name_id = resolver.local_entity_names().AddSymbolicBindingName(
-      name_id, SemIR::NameScopeId::None, import_entity_name.bind_index(),
-      import_entity_name.is_template);
+  auto entity_name_id =
+      GetLocalSymbolicEntityNameId(resolver, inst.entity_name_id);
   return ResolveAsDeduplicated<SemIR::BindSymbolicName>(
       resolver,
       {.type_id =
@@ -2960,6 +2975,23 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
            GetLocalCanonicalInstBlockId(resolver, inst.elements_id, elems)});
 }
 
+static auto TryResolveTypedInst(ImportRefResolver& resolver,
+                                SemIR::SymbolicBindingType inst)
+    -> ResolveResult {
+  auto facet_value_inst_id =
+      GetLocalConstantInstId(resolver, inst.facet_value_inst_id);
+  if (resolver.HasNewWork()) {
+    return ResolveResult::Retry();
+  }
+
+  auto entity_name_id =
+      GetLocalSymbolicEntityNameId(resolver, inst.entity_name_id);
+  return ResolveAsDeduplicated<SemIR::SymbolicBindingType>(
+      resolver, {.type_id = SemIR::TypeType::TypeId,
+                 .entity_name_id = entity_name_id,
+                 .facet_value_inst_id = facet_value_inst_id});
+}
+
 static auto TryResolveTypedInst(ImportRefResolver& resolver,
                                 SemIR::TupleAccess inst) -> ResolveResult {
   auto type_id = GetLocalConstantId(resolver, inst.type_id);
@@ -3311,6 +3343,9 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
     case CARBON_KIND(SemIR::SymbolicBindingPattern inst): {
       return TryResolveTypedInst(resolver, inst, constant_inst_id);
     }
+    case CARBON_KIND(SemIR::SymbolicBindingType inst): {
+      return TryResolveTypedInst(resolver, inst);
+    }
     case CARBON_KIND(SemIR::TupleAccess inst): {
       return TryResolveTypedInst(resolver, inst);
     }