Handle a specific providing ImplWitnessAccess for a symbolic binding used as a type (#6201)

SymbolicBindingType evaluates to the type component of a symbolic facet
value (a type/witnesses pair), and that symbolic facet value has its
constant value replaced by a specific. That specific can provide a
FacetValue, in which case it just evaluates to that FacetValue's type
component. It can provide a BindSymbolicName of another binding, in
which case it points to that entity instead and awaits a further
specific. Currently the code only handles these two cases, and they
match the behaviour of the evaluation of FacetAccessType itself.

However FacetAccessType evaluation also handles cases beyond these, as
there are other instructions that occur as facet values, such as
ImplWitnessAccess, when accessing an associated constant of an interface
that has a facet type as its type.

Currently eval then crashes in this scenario. Instead of furthering to
reproduce the contents of FacetAccessType's evaluation, defer to calling
the `EvalConstantInst()` overload for it when evaluating
SymbolicBindingType against a new value from a specific. This means
SymbolicBindingType can evaluate back into a FacetAccessType, when it
was originally a FacetAccessType(BindSymbolicName) and becomes
FacetAccessType(ImplWitnessAccess) through a specific.

This comes with a test that crashed in eval before this change.

Author: danakj

toolchain/check/eval.cpp

@@ -2178,75 +2178,55 @@ 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 a specific provides a new value for the binding with `entity_name_id`,
+  // the SymbolicBindingType is evaluated for that new value.
+  const auto& bind_name = eval_context.entity_names().Get(
+      inst.As<SemIR::SymbolicBindingType>().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;
+      // A SymbolicBindingType can evaluate to a FacetAccessType if the new
+      // value of the entity is a facet value that that does not have a concrete
+      // type (a FacetType) and does not have a new EntityName to point to (a
+      // BindSymbolicName).
+      auto access = SemIR::FacetAccessType{
+          .type_id = SemIR::TypeType::TypeId,
+          .facet_value_inst_id = value_inst_id,
+      };
+      return ConvertEvalResultToConstantId(
+          eval_context.context(),
+          EvalConstantInst(eval_context.context(), access),
+          ComputeInstPhase(eval_context.context(), access));
     }
   }
 
-  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>();
+  Phase phase = Phase::Concrete;
+  if (!ReplaceTypeWithConstantValue(eval_context, inst_id, &inst, &phase) ||
+      !ReplaceAllFieldsWithConstantValues(eval_context, &inst, &phase)) {
+    return SemIR::ConstantId::NotConstant;
   }
   // Propagate error phase after getting the constant value for all fields.
   if (phase == Phase::UnknownDueToError) {
     return SemIR::ErrorInst::ConstantId;
   }
 
+  // Evaluation of SymbolicBindingType.
+  //
+  // Like FacetAccessType, a SymbolicBindingType of a FacetValue just evaluates
+  // to the type inside.
+  //
   // 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)) {
+          inst.As<SemIR::SymbolicBindingType>().facet_value_inst_id)) {
     return eval_context.constant_values().Get(facet_value->type_inst_id);
   }
 
-  return MakeConstantResult(eval_context.context(), bind, phase);
+  return MakeConstantResult(eval_context.context(), inst, phase);
 }
 
 // Returns whether `const_id` is the same constant facet value as

toolchain/check/testdata/facet/access.carbon

@@ -383,6 +383,33 @@ fn G(AB:! A & B where .X = () and .Y = {}) -> AB.Y {
 }
 //@dump-sem-ir-end
 
+// --- symbolic_binding_type_of_impl_witness_access.carbon
+
+interface Y {}
+impl () as Y {}
+
+interface Z {
+  let Y1:! Y;
+  fn G() -> Y1*;
+}
+
+// The type of `T` matches exactly the type of `Z.Y1`, which prevents the
+// specific in the call from F2 from deducing a `FacetValue`. Instead it just
+// passes in the `ImplWitnessAccess` instruction as is.
+//
+// The `t: T` creates a `T as type`, or a `SymbolicBindingType` that gets
+// evaluated against the specific. The facet value that it evaluates against is
+// the `ImplWitnessAccess` from the call in F2.
+//
+// This requires that `SymbolicBindingType` evaluation correctly handles
+// arbitrary facet value instructions. Not just the common case of `FacetValue`
+// or `BindSymbolicName`.
+fn F1(T:! Y, t: T*) {}
+
+fn F2(U:! Z) {
+  F1(U.Y1, U.G());
+}
+
 // CHECK:STDOUT: --- access_assoc_fn.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: constants {