Make FacetAccessType evaluate to SymbolicBindingType for type-of a BindSymbolicName (#6115)

The SymbolicBindingType refers to the type value that will be
substituted in for the BindSymbolicName, but holds onto the EntityNameId
from the BindSymbolicName instead of (or in addition to, for now) the
instruction.

The EntityNameId will be used to look in the ScopeStack to find the
witnesses either from the BindSymbolicName instruction, or other
instructions that specify `impls` constraints against the EntityName.

This will allow us to have the `T` in `I(T)` resolve to a `.Self`
reference in the type so that we get type equality with the binding's
type: `T:! I(.Self)`.

Author: danakj

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);
     }