Fix constant string16 carrying UTF-8, and inverted slice indices reaching substring (#7268)

* str16 fix

* remove stale call

src/check_expr.cpp

@@ -2306,7 +2306,15 @@ gb_internal bool check_representable_as_constant(CheckerContext *c, ExactValue i
 		if (in_value.kind == ExactValue_String16) {
 			return is_type_string16(type) || is_type_cstring16(type);
 		}
-		return in_value.kind == ExactValue_String;
+		if (in_value.kind != ExactValue_String) {
+			return false;
+		}
+		// NOTE: a UTF-8 constant has to be re-expressed in UTF-16, otherwise its length and
+		// indices stay those of the UTF-8 encoding
+		if (is_type_string16(type) || is_type_cstring16(type)) {
+			if (out_value) *out_value = exact_value_string16(string_to_string16(permanent_allocator(), in_value.value_string));
+		}
+		return true;
 	} else if (is_type_integer(type) || is_type_rune(type)) {
 		ExactValue v = exact_value_to_integer(in_value);
 		if (v.kind != ExactValue_Integer) {
@@ -4005,14 +4013,15 @@ gb_internal void check_cast(CheckerContext *c, Operand *x, Type *type, bool forb
 		Type *dst = core_type(type);
 
 		if (is_type_string(src) && is_type_string(dst)) {
-			bool src_utf16 = is_type_string16(src) || is_type_cstring16(src);
 			bool dst_utf16 = is_type_string16(dst) || is_type_cstring16(dst);
 
-			if (!src_utf16 && dst_utf16) {
+			// NOTE: keyed off the value's encoding rather than the source type; it may have been re-expressed 
+			// when it was checked against the target type
+			if (dst_utf16 && x->value.kind == ExactValue_String) {
 				x->value = exact_value_string16(string_to_string16(permanent_allocator(), x->value.value_string));
 			}
 
-			if (src_utf16 && !dst_utf16) {
+			if (!dst_utf16 && x->value.kind == ExactValue_String16) {
 				x->value = exact_value_string(string16_to_string(permanent_allocator(), x->value.value_string16));
 			}
 		}
@@ -12218,12 +12227,14 @@ gb_internal ExprKind check_slice_expr(CheckerContext *c, Operand *o, Ast *node,
 		indices[i] = index;
 	}
 
+	bool invalid_indices = false;
 	for (isize i = 0; i < gb_count_of(indices); i++) {
 		i64 a = indices[i];
 		for (isize j = i+1; j < gb_count_of(indices); j++) {
 			i64 b = indices[j];
 			if (a > b && b >= 0) {
 				error(se->close, "Invalid slice indices: [%td > %td]", a, b);
+				invalid_indices = true;
 			}
 		}
 	}
@@ -12249,7 +12260,7 @@ gb_internal ExprKind check_slice_expr(CheckerContext *c, Operand *o, Ast *node,
 
 	o->mode = Addressing_Value;
 
-	if (is_type_string(t) && max_count >= 0) {
+	if (is_type_string(t) && max_count >= 0 && !invalid_indices) {
 		bool all_constant = true;
 		for (isize i = 0; i < gb_count_of(nodes); i++) {
 			if (nodes[i] != nullptr) {

tests/internal/test_string16.odin

@@ -0,0 +1,136 @@
+package test_internal
+
+import "core:testing"
+
+// The compiler folds constant `string16` operations at check time and emits the same
+// operations at runtime. Every case below computes a value both ways and compares them,
+// because the interesting failures are the ones where the two disagree silently.
+
+// Related previous issue #6101
+
+@(private="file")
+opaque :: proc(v: $T) -> T { return v }
+
+@(private="file")
+Ascii   : string16 : "hello"          // 5 bytes utf-8, 5 units utf-16
+@(private="file")
+Latin   : string16 : "héllo"          // 6 bytes utf-8, 5 units utf-16
+@(private="file")
+Cjk     : string16 : "日本語"          // 9 bytes utf-8, 3 units utf-16
+@(private="file")
+NonBmp  : string16 : "\U0001F63A"     // 4 bytes utf-8, 2 units utf-16 (surrogate pair)
+@(private="file")
+Mixed   : string16 : "a日\U0001F63A"   // 8 bytes utf-8, 4 units utf-16
+@(private="file")
+Empty   : string16 : ""
+
+@test
+string16_constant_length :: proc(t: ^testing.T) {
+	// lengths are in utf-16 code units, not utf-8 bytes
+	testing.expect_value(t, len(Ascii),  5)
+	testing.expect_value(t, len(Latin),  5)
+	testing.expect_value(t, len(Cjk),    3)
+	testing.expect_value(t, len(NonBmp), 2)
+	testing.expect_value(t, len(Mixed),  4)
+	testing.expect_value(t, len(Empty),  0)
+
+	// the constant length must match the length of the same value at runtime
+	testing.expect_value(t, len(Ascii),  len(opaque(Ascii)))
+	testing.expect_value(t, len(Latin),  len(opaque(Latin)))
+	testing.expect_value(t, len(Cjk),    len(opaque(Cjk)))
+	testing.expect_value(t, len(NonBmp), len(opaque(NonBmp)))
+	testing.expect_value(t, len(Mixed),  len(opaque(Mixed)))
+	testing.expect_value(t, len(Empty),  len(opaque(Empty)))
+}
+
+@test
+string16_constant_index :: proc(t: ^testing.T) {
+	testing.expect_value(t, Latin[0], 'h')
+	testing.expect_value(t, Latin[1], 0x00E9) // é stays one unit
+	testing.expect_value(t, Cjk[0],   0x65E5)
+	testing.expect_value(t, Cjk[2],   0x8A9E)
+	testing.expect_value(t, NonBmp[0], 0xD83D) // high surrogate
+	testing.expect_value(t, NonBmp[1], 0xDE3A) // low surrogate
+
+	// each constant-folded unit must match the same unit read at runtime
+	m := opaque(Mixed)
+	testing.expect_value(t, Mixed[0], m[0])
+	testing.expect_value(t, Mixed[1], m[1])
+	testing.expect_value(t, Mixed[2], m[2])
+	testing.expect_value(t, Mixed[3], m[3])
+}
+
+@test
+string16_constant_slice :: proc(t: ^testing.T) {
+	A :: Latin[0:2]
+	B :: Cjk[1:3]
+	C :: NonBmp[0:2]
+	D :: Mixed[1:2]
+	E :: Ascii[2:2]
+
+	testing.expect_value(t, len(A), 2)
+	testing.expect_value(t, len(B), 2)
+	testing.expect_value(t, len(C), 2)
+	testing.expect_value(t, len(D), 1)
+	testing.expect_value(t, len(E), 0)
+
+	testing.expect_value(t, A[0], 'h')
+	testing.expect_value(t, A[1], 0x00E9)
+	testing.expect_value(t, B[0], 0x672C)
+	testing.expect_value(t, C[1], 0xDE3A)
+	testing.expect_value(t, D[0], 0x65E5)
+
+	// open-ended and full slices
+	F :: Cjk[:]
+	G :: Cjk[1:]
+	H :: Cjk[:2]
+	testing.expect_value(t, len(F), 3)
+	testing.expect_value(t, len(G), 2)
+	testing.expect_value(t, len(H), 2)
+
+	// folded slice must equal the same slice taken at runtime
+	l := opaque(Latin)
+	rt := l[0:2]
+	testing.expect_value(t, len(A), len(rt))
+	testing.expect_value(t, A[0], rt[0])
+	testing.expect_value(t, A[1], rt[1])
+}
+
+@test
+string16_from_cast_and_assignment :: proc(t: ^testing.T) {
+	// the three ways a constant acquires a string16 type must agree
+	Typed  : string16 : "日本語"
+	Casted :: string16("日本語")
+	testing.expect_value(t, len(Typed), len(Casted))
+	testing.expect_value(t, Typed[0], Casted[0])
+	testing.expect_value(t, Typed[2], Casted[2])
+
+	assigned: string16 = "日本語"
+	testing.expect_value(t, len(assigned), len(Typed))
+	testing.expect_value(t, assigned[0], Typed[0])
+}
+
+@test
+string16_underlying_units :: proc(t: ^testing.T) {
+	// transmute exposes the utf-16 code units directly
+	u := transmute([]u16)opaque(NonBmp)
+	testing.expect_value(t, len(u), 2)
+	testing.expect_value(t, u[0], 0xD83D)
+	testing.expect_value(t, u[1], 0xDE3A)
+
+	c := transmute([]u16)opaque(Cjk)
+	testing.expect_value(t, len(c), 3)
+	testing.expect_value(t, c[0], 0x65E5)
+
+	// a utf-8 string of the same text keeps its byte length
+	testing.expect_value(t, len("日本語"), 9)
+}
+
+@test
+string16_comparison :: proc(t: ^testing.T) {
+	X : string16 : "日本語"
+	testing.expect(t, X == Cjk)
+	testing.expect(t, X != Ascii)
+	testing.expect(t, opaque(X) == Cjk)
+	testing.expect(t, Empty == "")
+}

tests/issues/run.bat

@@ -29,7 +29,6 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style -ignore-unused
 ..\..\..\odin build ..\test_issue_5573.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "2" || exit /b
 ..\..\..\odin test ..\test_issue_5699.odin %COMMON%  || exit /b
 ..\..\..\odin test ..\test_issue_6068.odin %COMMON%  || exit /b
-..\..\..\odin test ..\test_issue_6101.odin %COMMON%  || exit /b
 ..\..\..\odin test ..\test_issue_6165.odin %COMMON%  || exit /b
 ..\..\..\odin build ..\test_issue_6240.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b
 ..\..\..\odin build ..\test_issue_6401.odin %COMMON% 2>&1 | find /c "Error:" | findstr /x "3" || exit /b

tests/issues/run.sh

@@ -41,7 +41,6 @@ else
 fi
 $ODIN test ../test_issue_5699.odin $COMMON
 $ODIN test ../test_issue_6068.odin $COMMON
-$ODIN test ../test_issue_6101.odin $COMMON
 $ODIN test ../test_issue_6165.odin $COMMON
 $ODIN test ../test_issue_6344.odin $COMMON
 $ODIN test ../test_issue_6344.odin $COMMON -o:speed

tests/issues/test_issue_6101.odin

@@ -1,23 +0,0 @@
-// Tests issue #6101 https://github.com/odin-lang/Odin/issues/6101
-package test_issues
-
-import "core:testing"
-
-@(test)
-test_issue_6101_bmp :: proc(t: ^testing.T) {
-	s := string16("\u732b")
-	testing.expect_value(t, len(s), 1)
-
-	u := transmute([]u16)s
-	testing.expect_value(t, u[0], 0x732b)
-}
-
-@(test)
-test_issue_6101_non_bmp :: proc(t: ^testing.T) {
-	s := string16("\U0001F63A")
-	testing.expect_value(t, len(s), 2)
-
-	u := transmute([]u16)s
-	testing.expect_value(t, u[0], 0xD83D)
-	testing.expect_value(t, u[1], 0xDE3A)
-}