Merge pull request #7884 from wintermi/fix-switch-range-bound-types

Give switch range bounds the type of the switch tag

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");

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);
 

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");

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")..<cstring16("9"): result += 1
+	}
+
+	S16 :: string16("3")
+	switch S16 {
+	case "0"..="9": result += 1
+	}
+
+	testing.expect_value(t, result, 5)
+}
+
+@(test)
+test_integer_ranges :: proc(t: ^testing.T) {
+	// rune bounds on a `u8` tag now take the type `u8`
+	letter :: proc(c: u8) -> 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))
+}

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"