From 1a631b2f3fe2bb0f791850987f5e2b87b72b66a5 Mon Sep 17 00:00:00 2001 From: kalsprite <52809771+kalsprite@users.noreply.github.com> Date: Sun, 9 Aug 2026 12:22:13 -0700 Subject: [PATCH] Fix constant `string16` carrying UTF-8, and inverted slice indices reaching `substring` (#7268) * str16 fix * remove stale call --- src/check_expr.cpp | 21 +++-- tests/internal/test_string16.odin | 136 ++++++++++++++++++++++++++++++ tests/issues/run.bat | 1 - tests/issues/run.sh | 1 - tests/issues/test_issue_6101.odin | 23 ----- 5 files changed, 152 insertions(+), 30 deletions(-) create mode 100644 tests/internal/test_string16.odin delete mode 100644 tests/issues/test_issue_6101.odin diff --git a/src/check_expr.cpp b/src/check_expr.cpp index e73ec65c4..332f15836 100644 --- a/src/check_expr.cpp +++ b/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) { diff --git a/tests/internal/test_string16.odin b/tests/internal/test_string16.odin new file mode 100644 index 000000000..c67739168 --- /dev/null +++ b/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 == "") +} diff --git a/tests/issues/run.bat b/tests/issues/run.bat index 4bd301ac1..ff02105d7 100644 --- a/tests/issues/run.bat +++ b/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 diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 9c3cfecc3..fe4db8a4b 100755 --- a/tests/issues/run.sh +++ 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 diff --git a/tests/issues/test_issue_6101.odin b/tests/issues/test_issue_6101.odin deleted file mode 100644 index 9f24ade52..000000000 --- a/tests/issues/test_issue_6101.odin +++ /dev/null @@ -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) -}