Address feedback

Author: ammaralassal

core/prelude/types/string.carbon

@@ -8,6 +8,13 @@ import library "prelude/copy";
 import library "prelude/destroy";
 import library "prelude/types/char";
 import library "prelude/types/uint";
+import library "prelude/operators/index";
+import library "prelude/types/int";
+
+class String;
+
+// Forward declaration for builtin function
+fn StringAt(s: String, index: i32) -> Char;
 
 class String {
   fn Size[self: Self]() -> u64 { return self.size; }
@@ -16,8 +23,15 @@ class String {
     fn Op[self: Self]() -> Self { return {.ptr = self.ptr, .size = self.size}; }
   }
 
+  impl as IndexWith(i32) where .ElementType = Char {
+    fn At[self: Self](subscript: i32) -> Char {
+      return StringAt(self, subscript);
+    }
+  }
   // TODO: This should be an array iterator.
   private var ptr: Char*;
   // TODO: This should be a word-sized integer.
   private var size: u64;
 }
+
+fn StringAt(s: String, index: i32) -> Char = "string.at";

toolchain/check/eval.cpp

@@ -1687,6 +1687,63 @@ static auto MakeConstantForBuiltinCall(EvalContext& eval_context,
       return context.constant_values().Get(arg_ids[0]);
     }
 
+    case SemIR::BuiltinFunctionKind::StringAt: {
+      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);
+      CARBON_CHECK(elements.size() == 2, "String struct should have 2 fields.");
+
+      auto ptr_const_id = eval_context.constant_values().Get(elements[0]);
+      auto string_literal = eval_context.insts().GetAs<SemIR::StringLiteral>(
+          eval_context.constant_values().GetInstId(ptr_const_id));
+
+      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);
+
+      // 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(StringAtIndexOutOfBounds, Error,
+                          "string index `{0}` is past the end of the string.",
+                          TypedInt);
+        eval_context.emitter().Emit(
+            eval_context.GetDiagnosticLoc(index_id), StringAtIndexOutOfBounds,
+            {.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::PrintChar:
     case SemIR::BuiltinFunctionKind::PrintInt:
     case SemIR::BuiltinFunctionKind::ReadChar:

toolchain/check/testdata/operators/overloaded/string_indexing.carbon

@@ -0,0 +1,21 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/full.carbon
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/operators/overloaded/string_indexing.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/operators/overloaded/string_indexing.carbon
+
+// --- fail_wrong_type.carbon
+
+fn TestWrongType() {
+  var x: i32 = 42;
+  // CHECK:STDERR: fail_wrong_type.carbon:[[@LINE+4]]:22: error: cannot access member of interface `Core.IndexWith(Core.IntLiteral)` in type `i32` that does not implement that interface [MissingImplInMemberAccess]
+  // CHECK:STDERR:   var c: Core.Char = x[0];
+  // CHECK:STDERR:                      ^~~~
+  // CHECK:STDERR:
+  var c: Core.Char = x[0];
+}
+

toolchain/diagnostics/diagnostic_kind.def

@@ -397,6 +397,7 @@ CARBON_DIAGNOSTIC_KIND(AddrOnNonSelfParam)
 CARBON_DIAGNOSTIC_KIND(AddrOnNonPointerType)
 CARBON_DIAGNOSTIC_KIND(ArrayBoundTooLarge)
 CARBON_DIAGNOSTIC_KIND(ArrayBoundNegative)
+CARBON_DIAGNOSTIC_KIND(ArrayIndexNegative)
 CARBON_DIAGNOSTIC_KIND(ArrayIndexOutOfBounds)
 CARBON_DIAGNOSTIC_KIND(ArrayInitFromLiteralArgCountMismatch)
 CARBON_DIAGNOSTIC_KIND(ArrayInitFromExprArgCountMismatch)
@@ -450,6 +451,7 @@ CARBON_DIAGNOSTIC_KIND(NegativeIntInUnsignedType)
 CARBON_DIAGNOSTIC_KIND(NonConstantCallToCompTimeOnlyFunction)
 CARBON_DIAGNOSTIC_KIND(CompTimeOnlyFunctionHere)
 CARBON_DIAGNOSTIC_KIND(SelfOutsideImplicitParamList)
+CARBON_DIAGNOSTIC_KIND(StringAtIndexOutOfBounds)
 CARBON_DIAGNOSTIC_KIND(StringLiteralTooLong)
 CARBON_DIAGNOSTIC_KIND(StringLiteralTypeIncomplete)
 CARBON_DIAGNOSTIC_KIND(StringLiteralTypeUnexpected)

toolchain/lower/handle_call.cpp

@@ -318,6 +318,34 @@ static auto HandleBuiltinCall(FunctionContext& context, SemIR::InstId inst_id,
       return;
     }
 
+    case SemIR::BuiltinFunctionKind::StringAt: {
+      auto string_inst_id = arg_ids[0];
+      auto* string_arg = context.GetValue(string_inst_id);
+
+      auto string_type_id = context.GetTypeIdOfInst(string_inst_id);
+      auto* string_type = context.GetType(string_type_id);
+      auto* string_value =
+          context.builder().CreateLoad(string_type, string_arg, "string.load");
+
+      auto* string_ptr_field =
+          context.builder().CreateExtractValue(string_value, {0}, "string.ptr");
+
+      auto* index_value = context.GetValue(arg_ids[1]);
+
+      auto* char_ptr = context.builder().CreateInBoundsGEP(
+          llvm::Type::getInt8Ty(context.llvm_context()), string_ptr_field,
+          index_value, "string.char_ptr");
+
+      auto* char_i8 = context.builder().CreateLoad(
+          llvm::Type::getInt8Ty(context.llvm_context()), char_ptr,
+          "string.char");
+
+      context.SetLocal(inst_id, context.builder().CreateZExt(
+                                    char_i8, context.GetTypeOfInst(inst_id),
+                                    "string.char.zext"));
+      return;
+    }
+
     case SemIR::BuiltinFunctionKind::TypeAnd: {
       context.SetLocal(inst_id, context.GetTypeAsValue());
       return;

toolchain/sem_ir/builtin_function_kind.cpp

@@ -177,6 +177,22 @@ struct AnyType {
     return true;
   }
 };
+// Constraint that checks if a type is Core.String.
+struct CoreStringType {
+  static auto Check(const File& sem_ir, ValidateState& /*state*/,
+                    TypeId type_id) -> bool {
+    auto type_inst_id = sem_ir.types().GetInstId(type_id);
+    auto class_type = sem_ir.insts().TryGetAs<ClassType>(type_inst_id);
+    if (!class_type) {
+      // Not a string.
+      return false;
+    }
+
+    const auto& class_info = sem_ir.classes().Get(class_type->class_id);
+
+    return sem_ir.names().GetFormatted(class_info.name_id).str() == "String";
+  }
+};
 
 // Constraint that requires the type to be the type type.
 using Type = BuiltinType<TypeType::TypeInstId>;
@@ -322,6 +338,11 @@ constexpr BuiltinInfo PrintInt = {
 constexpr BuiltinInfo ReadChar = {"read.char",
                                   ValidateSignature<auto()->AnySizedInt>};
 
+// Gets a character from a string at the given index.
+constexpr BuiltinInfo StringAt = {
+    "string.at",
+    ValidateSignature<auto(CoreStringType, AnySizedInt)->AnySizedInt>};
+
 // Returns the `Core.CharLiteral` type.
 constexpr BuiltinInfo CharLiteralMakeType = {"char_literal.make_type",
                                              ValidateSignature<auto()->Type>};

toolchain/sem_ir/builtin_function_kind.def

@@ -30,6 +30,7 @@ CARBON_SEM_IR_BUILTIN_FUNCTION_KIND(PrimitiveCopy)
 CARBON_SEM_IR_BUILTIN_FUNCTION_KIND(PrintChar)
 CARBON_SEM_IR_BUILTIN_FUNCTION_KIND(PrintInt)
 CARBON_SEM_IR_BUILTIN_FUNCTION_KIND(ReadChar)
+CARBON_SEM_IR_BUILTIN_FUNCTION_KIND(StringAt)
 
 // Type factories.
 CARBON_SEM_IR_BUILTIN_FUNCTION_KIND(CharLiteralMakeType)