Handle a specific providing ImplWitnessAccess for a symbolic binding used as a type

toolchain/check/eval.cpp

@@ -2178,64 +2178,48 @@ auto TryEvalTypedInst<SemIR::SymbolicBindingType>(EvalContext& eval_context,
     -> 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.
+  // 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(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;
+      // 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;
   }
 
+  // Copy the updated constant field values into `bind`.
+  bind = inst.As<SemIR::SymbolicBindingType>();
+
+  // 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.

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 {