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

Code before:
      auto string_value = eval_context.sem_ir().string_literal_values().Get(
        string_literal->string_literal_id);
      //Get index value
      auto index_inst = eval_context.insts().TryGetAs<SemIR::IntValue>(index_id);
      if (!index_inst) {
        CARBON_CHECK(phase != Phase::Concrete, "Concrete constant integer should be a literal");
        return MakeNonConstantResult(phase);
      }

      const auto& index_val = eval_context.ints().Get(index_inst->int_id);

      //Check bounds
      if (index_val.isNegative()){
        CARBON_DIAGNOSTIC(ArrayIndexNegative, Error, "index `{0}` is negative",
                          TypedInt);
        eval_context.emitter().Emit(
          eval_context.GetDiagnosticLoc(index_id), ArrayIndexNegative,
          {.type = eval_context.insts().Get(index_id).type_id(),
          .value = index_val});
        return SemIR::ErrorInst::ConstantId;
      }

      //Check for out of bounds
      if (index_val.getActiveBits() > 64 ||
          index_val.getZExtValue() >= string_value.size()) {
        CARBON_DIAGNOSTIC(ArrayIndexOutOfBounds, Error,
                          "string index `{0}` is past the end of the string",
                          TypedInt);
        eval_context.emitter().Emit(
            eval_context.GetDiagnosticLoc(index_id), ArrayIndexOutOfBounds,
            {.type = eval_context.insts().Get(index_id).type_id(),
             .value = index_val});
        return SemIR::ErrorInst::ConstantId;
      }

      auto char_value = static_cast<uint8_t>(string_value[index_val.getZExtValue()]);

      auto int_id = eval_context.ints().Add(llvm::APSInt(llvm::APInt(32, char_value), /*isUnsigned=*/false));
      return MakeConstantResult(eval_context.context(),
        SemIR::IntValue{.type_id = call.type_id, .int_id = int_id},
        phase);

    }
    case SemIR::BuiltinFunctionKind::ReadChar:
    case SemIR::BuiltinFunctionKind::FloatAddAssign:
    case SemIR::BuiltinFunctionKind::FloatSubAssign:
    case SemIR::BuiltinFunctionKind::FloatMulAssign:
    case SemIR::BuiltinFunctionKind::FloatDivAssign:
    case SemIR::BuiltinFunctionKind::IntSAddAssign:
    case SemIR::BuiltinFunctionKind::IntSSubAssign:
    case SemIR::BuiltinFunctionKind::IntSMulAssign:

Review comment (suggestion):
A diagnostic name can only be used once, so maybe we want to add `StringAtIndexOutOfBounds`. Could we use that for the negative case too though?

We should add tests for both negative and too large as well.

Code after:
      Phase phase = Phase::Concrete;
      auto str_id = GetConstantValue(eval_context, arg_ids[0], &phase);
      auto index_id = GetConstantValue(eval_context, arg_ids[1], &phase);

      if (phase != Phase::Concrete) {
        return MakeNonConstantResult(phase);
      }

      auto str_struct = eval_context.insts().GetAs<SemIR::StructValue>(str_id);
      auto elements = eval_context.inst_blocks().Get(str_struct.elements_id);
      // String struct has two fields: a pointer to the string data and the
      // length.
      CARBON_CHECK(elements.size() == 2, "String struct should have 2 fields.");

      auto string_literal = eval_context.insts().GetAs<SemIR::StringLiteral>(
          eval_context.constant_values().GetConstantInstId(elements[0]));

      const auto& string_value =
          eval_context.sem_ir().string_literal_values().Get(
              string_literal.string_literal_id);

      auto index_inst = eval_context.insts().GetAs<SemIR::IntValue>(index_id);
      const auto& index_val = eval_context.ints().Get(index_inst.int_id);

      if (index_val.isNegative()) {
        CARBON_DIAGNOSTIC(StringAtIndexNegative, Error,
                          "index `{0}` is negative.", TypedInt);
        context.emitter().Emit(
            loc_id, StringAtIndexNegative,
            {.type = eval_context.insts().Get(index_id).type_id(),
             .value = index_val});
        return SemIR::ConstantId::NotConstant;
      }

      if (index_val.getZExtValue() >= string_value.size()) {
        CARBON_DIAGNOSTIC(
            StringAtIndexOutOfBounds, Error,
            "string index `{0}` is out of bounds; string has length {1}.",
            TypedInt, size_t);
        context.emitter().Emit(
            loc_id, StringAtIndexOutOfBounds,
            {.type = eval_context.insts().Get(index_id).type_id(),
             .value = index_val},
            string_value.size());
        return SemIR::ConstantId::NotConstant;
      }

      auto char_value =
          static_cast<uint8_t>(string_value[index_val.getZExtValue()]);

      auto int_id = eval_context.ints().Add(