From 4dad4ef64cfd495970b8edd9567ea310c6a490ec Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:31:42 +1100 Subject: [PATCH] Give switch range bounds the type of the switch tag The bounds of a `switch` range case were checked against the tag but never converted to its type, unlike a single case value, so untyped bounds took their own default type. Against a `cstring` tag `"0"..="9"` became `string` bounds, and converting a constant `string` to `cstring` in the backend crashed the compiler. A `cstring16` or `string16` tag failed with an invalid conversion from `string` instead. Convert the bounds to the tag's type, which also gives an invalid bound the same error as a single case value. A constant `cstring` or `cstring16` tag forced the `string` comparison procedures rather than its own, so it failed with a missing `cstring_le` or `cstring16_le` even with typed bounds. `lb_get_const_string` expected the data pointer of a constant string to be a zero index GEP, which LLVM folds to the global itself, and it is null for an empty string, so it read an invalid constant expression. Handle both, and have a converted empty string point to its terminator like a constant `cstring` rather than be nil. Fixes #7853 --- src/check_stmt.cpp | 19 ++- src/llvm_backend_const.cpp | 11 +- src/llvm_backend_expr.cpp | 3 +- .../test_issue_7853/test_issue_7853.odin | 151 ++++++++++++++++++ tests/issues/test_simple.odin | 1 + 5 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 tests/issues/test_issue_7853/test_issue_7853.odin diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index 5ec566104..2a1fc1f7a 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -1275,6 +1275,17 @@ gb_internal void check_switch_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags continue; } + // NOTE: like a single case value, the bounds take the type of the switch tag, otherwise + // untyped bounds would default to their own type (e.g. 'string' for a 'cstring' tag) (#7853) + convert_to_typed(ctx, &lhs, x.type); + if (lhs.mode == Addressing_Invalid) { + continue; + } + convert_to_typed(ctx, &rhs, x.type); + if (rhs.mode == Addressing_Invalid) { + continue; + } + TokenKind upper_op = Token_Invalid; switch (be->op.kind) { case Token_Ellipsis: upper_op = Token_LtEq; break; @@ -1304,7 +1315,13 @@ gb_internal void check_switch_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags add_to_seen_map(ctx, &seen, upper_op, x, lhs, rhs); } - if (is_type_string16(x.type)) { + if (is_type_cstring16(x.type)) { + add_package_dependency(ctx, "runtime", "cstring16_le"); + add_package_dependency(ctx, "runtime", "cstring16_lt"); + } else if (is_type_cstring(x.type)) { + add_package_dependency(ctx, "runtime", "cstring_le"); + add_package_dependency(ctx, "runtime", "cstring_lt"); + } else if (is_type_string16(x.type)) { // NOTE(bill): Force dependency for strings here add_package_dependency(ctx, "runtime", "string16_le"); add_package_dependency(ctx, "runtime", "string16_lt"); diff --git a/src/llvm_backend_const.cpp b/src/llvm_backend_const.cpp index 56b38e904..e566052b6 100644 --- a/src/llvm_backend_const.cpp +++ b/src/llvm_backend_const.cpp @@ -76,8 +76,15 @@ gb_internal String lb_get_const_string(lbModule *m, lbValue value) { LLVMValueRef underlying_ptr = llvm_const_extract_value(m, value.value, ptr_indices, gb_count_of(ptr_indices)); LLVMValueRef underlying_len = llvm_const_extract_value(m, value.value, len_indices, gb_count_of(len_indices)); - GB_ASSERT(LLVMGetConstOpcode(underlying_ptr) == LLVMGetElementPtr); - underlying_ptr = LLVMGetOperand(underlying_ptr, 0); + if (LLVMIsNull(underlying_ptr)) { + // NOTE: an empty string has no data + return {}; + } + // NOTE: LLVM folds the zero index GEP into the string's data to the global itself + if (LLVMIsAConstantExpr(underlying_ptr)) { + GB_ASSERT(LLVMGetConstOpcode(underlying_ptr) == LLVMGetElementPtr); + underlying_ptr = LLVMGetOperand(underlying_ptr, 0); + } GB_ASSERT(LLVMIsAGlobalVariable(underlying_ptr)); underlying_ptr = LLVMGetInitializer(underlying_ptr); diff --git a/src/llvm_backend_expr.cpp b/src/llvm_backend_expr.cpp index b2f8b184d..c2aadb909 100644 --- a/src/llvm_backend_expr.cpp +++ b/src/llvm_backend_expr.cpp @@ -2392,7 +2392,8 @@ gb_internal lbValue lb_emit_conv(lbProcedure *p, lbValue value, Type *t) { String str = lb_get_const_string(m, value); lbValue res = {}; res.type = t; - res.value = llvm_cstring(m, str); + // NOTE: as with a constant 'cstring', even "" points to its terminator rather than being nil + res.value = lb_find_or_add_entity_string_ptr(m, str, false); return res; } else if (src->kind == Type_Basic && src->Basic.kind == Basic_string16 && dst->Basic.kind == Basic_cstring16) { GB_PANIC("TODO(bill): UTF-16 string"); diff --git a/tests/issues/test_issue_7853/test_issue_7853.odin b/tests/issues/test_issue_7853/test_issue_7853.odin new file mode 100644 index 000000000..70a3a8859 --- /dev/null +++ b/tests/issues/test_issue_7853/test_issue_7853.odin @@ -0,0 +1,151 @@ +package test_issue_7853 + +import "core:testing" + +// The bounds of a `switch` range case were not converted to the type of the switch tag, unlike a single case +// value, so untyped bounds took their own default type. Against a `cstring` tag they became `string`, and +// converting a constant `string` to `cstring` in the backend crashed the compiler. A `cstring16` or `string16` +// tag failed with an invalid conversion from `string`, and a constant `cstring` or `cstring16` tag missed the +// runtime comparison procedures it needs. + +classify :: proc(text: cstring) -> int { + switch text { + case "0"..="9": + return 1 + case "a"..<"n": + return 2 + case: + return 0 + } +} + +@(test) +test_cstring_range :: proc(t: ^testing.T) { + testing.expect_value(t, classify("0"), 1) + testing.expect_value(t, classify("3"), 1) + testing.expect_value(t, classify("9"), 1) + testing.expect_value(t, classify("10"), 1) // compared as strings, "0" <= "10" <= "9" + testing.expect_value(t, classify("a"), 2) + testing.expect_value(t, classify("m"), 2) + testing.expect_value(t, classify("n"), 0) + testing.expect_value(t, classify(""), 0) + testing.expect_value(t, classify("~"), 0) +} + +@(test) +test_cstring_range_bounds :: proc(t: ^testing.T) { + // one bound typed, the other untyped + one_typed :: proc(text: cstring) -> bool { + switch text { + case cstring("0")..="9": + return true + } + return false + } + testing.expect(t, one_typed("5")) + testing.expect(t, !one_typed("a")) + + // an empty bound + from_empty :: proc(text: cstring) -> bool { + switch text { + case ""..="9": + return true + } + return false + } + testing.expect(t, from_empty("")) + testing.expect(t, from_empty("9")) + testing.expect(t, !from_empty("a")) +} + +@(test) +test_distinct_cstring_range :: proc(t: ^testing.T) { + Distinct :: distinct cstring + in_range :: proc(text: Distinct) -> bool { + switch text { + case "0"..="9": + return true + } + return false + } + testing.expect(t, in_range("3")) + testing.expect(t, !in_range("a")) +} + +@(test) +test_utf16_ranges :: proc(t: ^testing.T) { + c16 :: proc(text: cstring16) -> bool { + switch text { + case "0"..="9": + return true + } + return false + } + testing.expect(t, c16("3")) + testing.expect(t, !c16("a")) + + s16 :: proc(text: string16) -> bool { + switch text { + case "0"..<"9": + return true + } + return false + } + testing.expect(t, s16("3")) + testing.expect(t, !s16("9")) +} + +@(test) +test_constant_tags :: proc(t: ^testing.T) { + result := 0 + + C :: cstring("3") + switch C { + case "0"..="9": result += 1 + } + switch C { + case cstring("0")..=cstring("9"): result += 1 + } + + C16 :: cstring16("3") + switch C16 { + case "0"..="9": result += 1 + } + switch C16 { + case cstring16("0").. bool { + switch c { + case 'a'..='z', 'A'..='Z': + return true + } + return false + } + testing.expect(t, letter('q')) + testing.expect(t, letter('Q')) + testing.expect(t, !letter('0')) + testing.expect(t, !letter(255)) + + high :: proc(c: u8) -> bool { + switch c { + case 128..=255: + return true + } + return false + } + testing.expect(t, high(200)) + testing.expect(t, high(255)) + testing.expect(t, !high(127)) +} diff --git a/tests/issues/test_simple.odin b/tests/issues/test_simple.odin index 9fcffd282..154f215bd 100644 --- a/tests/issues/test_simple.odin +++ b/tests/issues/test_simple.odin @@ -47,6 +47,7 @@ package tests_issues @(require) import "test_issue_7708" @(require) import "test_issue_7763" @(require) import "test_issue_7779" +@(require) import "test_issue_7853" @(require) import "test_issue_fast_isel_lowering" @(require) import "test_issue_omitted_field_union" @(require) import "test_issue_equal_proc_dependencies"