Repository: carbon-language/carbon-lang
File: toolchain/check/eval.cpp (C++)

Code before:
          eval_context.entity_names().MakeCanonical(bind.entity_name_id);
      bind.value_id = SemIR::InstId::None;
      if (!ReplaceFieldWithConstantValue(
              eval_context, &bind, &SemIR::BindSymbolicName::type_id, &phase)) {
        return MakeNonConstantResult(phase);
      }
      return MakeConstantResult(eval_context.context(), bind, phase);
    }

    // AsCompatible changes the type of the source instruction; its constant
    // value, if there is one, needs to be modified to be of the same type.
    case CARBON_KIND(SemIR::AsCompatible inst): {
      auto value = eval_context.GetConstantValue(inst.source_id);
      if (!value.is_constant()) {
        return value;
      }

      auto type_id = inst.type_id;
      auto value_inst = eval_context.insts().Get(
          eval_context.constant_values().GetInstId(value));
      value_inst.SetType(type_id);

      auto from_phase = value.is_symbolic() ? Phase::Symbolic : Phase::Template;
      auto to_phase = eval_context.types().GetConstantId(type_id).is_symbolic()
                          ? Phase::Symbolic
                          : Phase::Template;
      if (to_phase >= from_phase) {
        // If moving from a template constant value to a symbolic type, the new
        // constant value takes on the phase of the new type. We're adding the
        // symbolic bit to the new constant value due to the presence of a
        // symbolic type.
        return MakeConstantResult(eval_context.context(), value_inst, to_phase);
      } else {
        // If moving from a symbolic constant value to a template type, the new
        // constant value has a phase that depends on what is in the value. If
        // there is anything symbolic within the value, then it's symbolic. We
        // can't easily determine that here without evaluating a new constant
        // value. See
        // https://github.com/carbon-language/carbon-lang/pull/4881#discussion_r1939961372
        //
        // TODO: Find a way to avoid recursion for this. Make this function into
        // a while loop? Track the phase of the constant value separately from
        // the phase of the value's type?
        return TryEvalInstInContext(eval_context, SemIR::InstId::None,
                                    value_inst);
      }
    }

    // These semantic wrappers don't change the constant value.
    case CARBON_KIND(SemIR::BindAlias typed_inst): {
      return eval_context.GetConstantValue(typed_inst.value_id);

Review comment (suggestion):
The phase of the constant or the type could also be `PeriodSelfSymbolic` (or `UnknownDueToError`?). Instead, use `::GetConstantValue` to get the phase at the same time you get the source value and type from `inst`:

```c++
auto from_phase = Phase::Template;
auto value_id = GetConstantValue(eval_context, inst.source_id, &from_phase);

auto to_phase = Phase::Template;
auto type_id = GetConstantValue(eval_context, inst.type_id, &to_phase);
```

(Using `GetConstantValue` rather than directly calling `types().GetConstantId` also means you'll get the proper substituted type for the constant in the case where you're evaluating a generic eval block and the type depends on a generic parameter.)

Code after:
      // The constant form of a symbolic binding is an idealized form of the
      // original, with no equivalent value.
      bind.entity_name_id =
          eval_context.entity_names().MakeCanonical(bind.entity_name_id);
      bind.value_id = SemIR::InstId::None;
      if (!ReplaceFieldWithConstantValue(
              eval_context, &bind, &SemIR::BindSymbolicName::type_id, &phase)) {
        return MakeNonConstantResult(phase);
      }
      return MakeConstantResult(eval_context.context(), bind, phase);
    }

    // AsCompatible changes the type of the source instruction; its constant
    // value, if there is one, needs to be modified to be of the same type.
    case CARBON_KIND(SemIR::AsCompatible inst): {
      auto value = eval_context.GetConstantValue(inst.source_id);
      if (!value.is_constant()) {
        return value;
      }

      auto from_phase = Phase::Template;
      auto value_inst_id =
          GetConstantValue(eval_context, inst.source_id, &from_phase);

      auto to_phase = Phase::Template;
      auto type_id = GetConstantValue(eval_context, inst.type_id, &to_phase);

      auto value_inst = eval_context.insts().Get(value_inst_id);
      value_inst.SetType(type_id);

      if (to_phase >= from_phase) {
        // If moving from a template constant value to a symbolic type, the new
        // constant value takes on the phase of the new type. We're adding the
        // symbolic bit to the new constant value due to the presence of a
        // symbolic type.
        return MakeConstantResult(eval_context.context(), value_inst, to_phase);
      } else {
        // If moving from a symbolic constant value to a template type, the new
        // constant value has a phase that depends on what is in the value. If
        // there is anything symbolic within the value, then it's symbolic. We
        // can't easily determine that here without evaluating a new constant
        // value. See
        // https://github.com/carbon-language/carbon-lang/pull/4881#discussion_r1939961372
        [[clang::musttail]] return TryEvalInstInContext(
            eval_context, SemIR::InstId::None, value_inst);
      }
    }

    // These semantic wrappers don't change the constant value.
    case CARBON_KIND(SemIR::BindAlias typed_inst): {
      return eval_context.GetConstantValue(typed_inst.value_id);