From c780de628219006cb322215662356448256796fb Mon Sep 17 00:00:00 2001
From: Nick Treleaven <n@trelsoft.com>
Date: Tue, 12 Apr 2022 17:37:30 +0100
Subject: [PATCH] checker: disallow 'small_unsigned == signed' (#13967)

---
 vlib/bitfield/bitfield_test.v                 |  2 +-
 vlib/builtin/js/map_test.js.v                 |  4 +-
 vlib/builtin/map_test.v                       |  4 +-
 vlib/builtin/string.v                         |  2 +-
 vlib/szip/szip.v                              |  2 +-
 vlib/term/ui/input_windows.c.v                |  4 +-
 vlib/toml/scanner/scanner.v                   |  2 +-
 vlib/v/ast/types.v                            | 20 ++++++++++
 vlib/v/ast/types_test.v                       |  6 +++
 vlib/v/checker/checker.v                      | 23 +++++++++++
 .../checker/tests/compare_unsigned_signed.out | 40 +++++++++++++++++++
 .../checker/tests/compare_unsigned_signed.vv  | 16 ++++++++
 .../infix_unsigned_and_signed_int_err.out     | 13 ------
 .../infix_unsigned_and_signed_int_err.vv      |  9 -----
 vlib/x/ttf/render_bmp.v                       |  2 +-
 15 files changed, 116 insertions(+), 33 deletions(-)
 create mode 100644 vlib/v/checker/tests/compare_unsigned_signed.out
 create mode 100644 vlib/v/checker/tests/compare_unsigned_signed.vv
 delete mode 100644 vlib/v/checker/tests/infix_unsigned_and_signed_int_err.out
 delete mode 100644 vlib/v/checker/tests/infix_unsigned_and_signed_int_err.vv

diff --git a/vlib/bitfield/bitfield_test.v b/vlib/bitfield/bitfield_test.v
index 08240903a0..365e583b27 100644
--- a/vlib/bitfield/bitfield_test.v
+++ b/vlib/bitfield/bitfield_test.v
@@ -161,7 +161,7 @@ fn test_bf_from_str() {
 	output := bitfield.from_str(input)
 	mut result := 1
 	for i in 0 .. len {
-		if input[i] != output.get_bit(i) + 48 {
+		if input[i] != byte(output.get_bit(i)) + 48 {
 			result = 0
 		}
 	}
diff --git a/vlib/builtin/js/map_test.js.v b/vlib/builtin/js/map_test.js.v
index a70747423c..ef6e013893 100644
--- a/vlib/builtin/js/map_test.js.v
+++ b/vlib/builtin/js/map_test.js.v
@@ -775,7 +775,7 @@ fn test_byte_keys() {
 		m[i]++
 		assert m[i] == i + 1
 	}
-	assert m.len == byte_max
+	assert m.len == int(byte_max)
 	keys := m.keys()
 	for i in byte(0) .. byte_max {
 		assert keys[i] == i
@@ -827,7 +827,7 @@ fn test_u16_keys() {
 		m[i]++
 		assert m[i] == i + 1
 	}
-	assert m.len == end
+	assert m.len == int(end)
 	keys := m.keys()
 	for i in u16(0) .. end {
 		assert keys[i] == i
diff --git a/vlib/builtin/map_test.v b/vlib/builtin/map_test.v
index 740c6f1cd0..a064d7f6bb 100644
--- a/vlib/builtin/map_test.v
+++ b/vlib/builtin/map_test.v
@@ -763,7 +763,7 @@ fn test_byte_keys() {
 		m[i]++
 		assert m[i] == i + 1
 	}
-	assert m.len == byte_max
+	assert m.len == int(byte_max)
 	keys := m.keys()
 	for i in byte(0) .. byte_max {
 		assert keys[i] == i
@@ -815,7 +815,7 @@ fn test_u16_keys() {
 		m[i]++
 		assert m[i] == i + 1
 	}
-	assert m.len == end
+	assert m.len == int(end)
 	keys := m.keys()
 	for i in u16(0) .. end {
 		assert keys[i] == i
diff --git a/vlib/builtin/string.v b/vlib/builtin/string.v
index f1252149f3..7f38bc878a 100644
--- a/vlib/builtin/string.v
+++ b/vlib/builtin/string.v
@@ -1937,7 +1937,7 @@ pub fn (name string) match_glob(pattern string) bool {
 						mut is_inverted := false
 						mut inner_match := false
 						mut inner_idx := bstart + 1
-						mut inner_c := 0
+						mut inner_c := byte(0)
 						if inner_idx < plen {
 							inner_c = pattern[inner_idx]
 							if inner_c == `^` {
diff --git a/vlib/szip/szip.v b/vlib/szip/szip.v
index 3982582a75..0e96c0242c 100644
--- a/vlib/szip/szip.v
+++ b/vlib/szip/szip.v
@@ -173,7 +173,7 @@ pub fn (mut zentry Zip) crc32() u32 {
 
 // write_entry compresses an input buffer for the current zip entry.
 pub fn (mut zentry Zip) write_entry(data []byte) ? {
-	if (data[0] & 0xff) == -1 {
+	if int(data[0] & 0xff) == -1 {
 		return error('szip: cannot write entry')
 	}
 	res := C.zip_entry_write(zentry, data.data, data.len)
diff --git a/vlib/term/ui/input_windows.c.v b/vlib/term/ui/input_windows.c.v
index 6f08f2a934..09a3913d4e 100644
--- a/vlib/term/ui/input_windows.c.v
+++ b/vlib/term/ui/input_windows.c.v
@@ -288,8 +288,8 @@ fn (mut ctx Context) parse_events() {
 				if !C.GetConsoleScreenBufferInfo(ctx.stdout_handle, &sb) {
 					panic('could not get screenbuffer info')
 				}
-				w := sb.srWindow.Right - sb.srWindow.Left + 1
-				h := sb.srWindow.Bottom - sb.srWindow.Top + 1
+				w := int(sb.srWindow.Right - sb.srWindow.Left + 1)
+				h := int(sb.srWindow.Bottom - sb.srWindow.Top + 1)
 				utf8 := '($ctx.window_width, $ctx.window_height) -> ($w, $h)'
 				if w != ctx.window_width || h != ctx.window_height {
 					ctx.window_width, ctx.window_height = w, h
diff --git a/vlib/toml/scanner/scanner.v b/vlib/toml/scanner/scanner.v
index 7f421227c9..1c61fa8bfe 100644
--- a/vlib/toml/scanner/scanner.v
+++ b/vlib/toml/scanner/scanner.v
@@ -9,7 +9,7 @@ import toml.util
 
 pub const (
 	digit_extras = [`_`, `.`, `x`, `o`, `b`, `e`, `E`]
-	end_of_text  = 4294967295
+	end_of_text  = u32(~0)
 )
 
 // Scanner contains the necessary fields for the state of the scan process.
diff --git a/vlib/v/ast/types.v b/vlib/v/ast/types.v
index 7bc360d1c8..4b032c3c38 100644
--- a/vlib/v/ast/types.v
+++ b/vlib/v/ast/types.v
@@ -374,6 +374,23 @@ pub fn (typ Type) is_unsigned() bool {
 	return typ.idx() in ast.unsigned_integer_type_idxs
 }
 
+pub fn (typ Type) flip_signedness() Type {
+	r := match typ {
+		ast.i8_type { ast.byte_type }
+		ast.i16_type { ast.u16_type }
+		ast.int_type { ast.u32_type }
+		ast.isize_type { ast.usize_type }
+		ast.i64_type { ast.u64_type }
+		ast.byte_type { ast.i8_type }
+		ast.u16_type { ast.i16_type }
+		ast.u32_type { ast.int_type }
+		ast.usize_type { ast.isize_type }
+		ast.u64_type { ast.i64_type }
+		else { typ }
+	}
+	return r
+}
+
 [inline]
 pub fn (typ Type) is_int_literal() bool {
 	return int(typ) == ast.int_literal_type_idx
@@ -442,6 +459,9 @@ pub const (
 		i64_type_idx, isize_type_idx]
 	unsigned_integer_type_idxs = [byte_type_idx, u8_type_idx, u16_type_idx, u32_type_idx,
 		u64_type_idx, usize_type_idx]
+	// C will promote any type smaller than int to int in an expression
+	int_promoted_type_idxs     = [char_type_idx, i8_type_idx, i16_type_idx, byte_type_idx,
+		u8_type_idx, u16_type_idx]
 	float_type_idxs            = [f32_type_idx, f64_type_idx, float_literal_type_idx]
 	number_type_idxs           = [i8_type_idx, i16_type_idx, int_type_idx, i64_type_idx,
 		byte_type_idx, char_type_idx, u16_type_idx, u32_type_idx, u64_type_idx, isize_type_idx,
diff --git a/vlib/v/ast/types_test.v b/vlib/v/ast/types_test.v
index fd98e33c8c..67e29459b4 100644
--- a/vlib/v/ast/types_test.v
+++ b/vlib/v/ast/types_test.v
@@ -79,3 +79,9 @@ fn test_derive() {
 	assert t2.has_flag(ast.TypeFlag.generic) == true
 	assert t2.nr_muls() == 10
 }
+
+fn test_flip_signedness() {
+	assert ast.i8_type.flip_signedness() == ast.byte_type
+	assert ast.u16_type.flip_signedness() == ast.i16_type
+	assert ast.isize_type.flip_signedness() == ast.usize_type
+}
diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 44c413cf44..d0b032c45d 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -626,6 +626,29 @@ pub fn (mut c Checker) infix_expr(mut node ast.InfixExpr) ast.Type {
 			if is_mismatch {
 				c.error('possible type mismatch of compared values of `$node.op` operation',
 					left_right_pos)
+			} else if left_type in ast.integer_type_idxs && right_type in ast.integer_type_idxs {
+				is_left_type_signed := left_type in ast.signed_integer_type_idxs
+				is_right_type_signed := right_type in ast.signed_integer_type_idxs
+				if !is_left_type_signed && mut node.right is ast.IntegerLiteral {
+					if node.right.val.int() < 0 && left_type in ast.int_promoted_type_idxs {
+						lt := c.table.sym(left_type).name
+						c.error('`$lt` cannot be compared with negative value', node.right.pos)
+					}
+				} else if !is_right_type_signed && mut node.left is ast.IntegerLiteral {
+					if node.left.val.int() < 0 && right_type in ast.int_promoted_type_idxs {
+						rt := c.table.sym(right_type).name
+						c.error('negative value cannot be compared with `$rt`', node.left.pos)
+					}
+				} else if is_left_type_signed != is_right_type_signed
+					&& left_type.flip_signedness() != right_type {
+					// prevent e.g. `u16(-1) == int(-1)` which is false in C
+					if (is_right_type_signed && left_type in ast.int_promoted_type_idxs)
+						|| (is_left_type_signed && right_type in ast.int_promoted_type_idxs) {
+						lt := c.table.sym(left_type).name
+						rt := c.table.sym(right_type).name
+						c.error('`$lt` cannot be compared with `$rt`', node.pos)
+					}
+				}
 			}
 		}
 		.key_in, .not_in {
diff --git a/vlib/v/checker/tests/compare_unsigned_signed.out b/vlib/v/checker/tests/compare_unsigned_signed.out
new file mode 100644
index 0000000000..7a2dd31db1
--- /dev/null
+++ b/vlib/v/checker/tests/compare_unsigned_signed.out
@@ -0,0 +1,40 @@
+vlib/v/checker/tests/compare_unsigned_signed.vv:2:14: error: unsigned integer cannot be compared with negative value
+    1 | fn main() {
+    2 |     if u32(1) < -1 {
+      |                 ~~
+    3 |         println('unexpected')
+    4 |     }
+vlib/v/checker/tests/compare_unsigned_signed.vv:6:5: error: unsigned integer cannot be compared with negative value
+    4 |     }
+    5 | 
+    6 |     if -1 > u32(1) {
+      |        ~~
+    7 |         println('unexpected')
+    8 |     }
+vlib/v/checker/tests/compare_unsigned_signed.vv:10:18: error: `byte` cannot be compared with negative value
+    8 |     }
+    9 |     // unsigned == literal
+   10 |     _ = byte(-1) == -1 // false!
+      |                     ~~
+   11 |     _ = -1 == u16(-1) // false!
+   12 |
+vlib/v/checker/tests/compare_unsigned_signed.vv:11:6: error: negative value cannot be compared with `u16`
+    9 |     // unsigned == literal
+   10 |     _ = byte(-1) == -1 // false!
+   11 |     _ = -1 == u16(-1) // false!
+      |         ~~
+   12 |     
+   13 |     // unsigned == signed
+vlib/v/checker/tests/compare_unsigned_signed.vv:14:14: error: `u16` cannot be compared with `int`
+   12 |     
+   13 |     // unsigned == signed
+   14 |     _ = u16(-1) == int(-1)
+      |                 ~~
+   15 |     _ = int(-1) != byte(-1)
+   16 | }
+vlib/v/checker/tests/compare_unsigned_signed.vv:15:14: error: `int` cannot be compared with `byte`
+   13 |     // unsigned == signed
+   14 |     _ = u16(-1) == int(-1)
+   15 |     _ = int(-1) != byte(-1)
+      |                 ~~
+   16 | }
diff --git a/vlib/v/checker/tests/compare_unsigned_signed.vv b/vlib/v/checker/tests/compare_unsigned_signed.vv
new file mode 100644
index 0000000000..ed6fc040ed
--- /dev/null
+++ b/vlib/v/checker/tests/compare_unsigned_signed.vv
@@ -0,0 +1,16 @@
+fn main() {
+	if u32(1) < -1 {
+		println('unexpected')
+	}
+
+	if -1 > u32(1) {
+		println('unexpected')
+	}
+	// unsigned == literal
+	_ = byte(-1) == -1 // false!
+	_ = -1 == u16(-1) // false!
+	
+	// unsigned == signed
+	_ = u16(-1) == int(-1)
+	_ = int(-1) != byte(-1)
+}
diff --git a/vlib/v/checker/tests/infix_unsigned_and_signed_int_err.out b/vlib/v/checker/tests/infix_unsigned_and_signed_int_err.out
deleted file mode 100644
index 1895d686ff..0000000000
--- a/vlib/v/checker/tests/infix_unsigned_and_signed_int_err.out
+++ /dev/null
@@ -1,13 +0,0 @@
-vlib/v/checker/tests/infix_unsigned_and_signed_int_err.vv:2:14: error: unsigned integer cannot be compared with negative value
-    1 | fn main() {
-    2 |     if u32(1) < -1 {
-      |                 ~~
-    3 |         println('unexpected')
-    4 |     }
-vlib/v/checker/tests/infix_unsigned_and_signed_int_err.vv:6:5: error: unsigned integer cannot be compared with negative value
-    4 |     }
-    5 |
-    6 |     if -1 > u32(1) {
-      |        ~~
-    7 |         println('unexpected')
-    8 |     }
diff --git a/vlib/v/checker/tests/infix_unsigned_and_signed_int_err.vv b/vlib/v/checker/tests/infix_unsigned_and_signed_int_err.vv
deleted file mode 100644
index e3404eb33a..0000000000
--- a/vlib/v/checker/tests/infix_unsigned_and_signed_int_err.vv
+++ /dev/null
@@ -1,9 +0,0 @@
-fn main() {
-	if u32(1) < -1 {
-		println('unexpected')
-	}
-
-	if -1 > u32(1) {
-		println('unexpected')
-	}
-}
diff --git a/vlib/x/ttf/render_bmp.v b/vlib/x/ttf/render_bmp.v
index d7a74f79d3..4539617ca2 100644
--- a/vlib/x/ttf/render_bmp.v
+++ b/vlib/x/ttf/render_bmp.v
@@ -778,7 +778,7 @@ pub fn (mut bmp BitMap) draw_glyph(index u16) (int, int) {
 			}
 		}
 
-		if count == glyph.contour_ends[c] {
+		if count == int(glyph.contour_ends[c]) {
 			// dprintln("count == glyph.contour_ends[count]")
 			if s == 2 { // final point was off-curve. connect to start