From dd183d3ba696d6ed186c2ba26309aacdf5a22a6b Mon Sep 17 00:00:00 2001
From: Bill Currie <bill@taniwha.org>
Date: Wed, 20 Dec 2023 22:49:51 +0900
Subject: [PATCH] [qfcc] Handle signed-unsigned int comparison better

This fixes the upostop-- test by auto-casting implicit constants to
unsigned (and it gives a warning for signed-unsigned comparisons
otherwise). The generated code isn't quite the best, but the fix for
that is next.

Also clean up the resulting mess, though not properly. There are a few
bogus warnings, and the legit ones could do with a review.
---
 ruamoko/cl_menu/MenuGroup.h        |  6 ++--
 ruamoko/cl_menu/MenuGroup.r        |  4 +--
 ruamoko/lib/Array+Private.r        |  4 +--
 ruamoko/lib/Array.r                | 22 ++++-----------
 ruamoko/qwaq/debugger/localsdata.r |  4 +--
 ruamoko/qwaq/editor/editor.r       | 22 +++++++--------
 ruamoko/qwaq/ui/garray.r           |  4 +--
 ruamoko/qwaq/ui/group.r            |  2 +-
 ruamoko/scheme/CompiledCode.r      |  2 +-
 ruamoko/scheme/Scope.r             |  4 +--
 tools/qfcc/source/expr_binary.c    | 45 ++++++++++++++++++++++++------
 tools/qfcc/test/arraytypedef.r     |  2 +-
 tools/qfcc/test/ptraliasenc.r      |  3 +-
 tools/qfcc/test/ptrfunc.r          |  2 +-
 14 files changed, 72 insertions(+), 54 deletions(-)

diff --git a/ruamoko/cl_menu/MenuGroup.h b/ruamoko/cl_menu/MenuGroup.h
index 3b4bf473f..da1f3a27d 100644
--- a/ruamoko/cl_menu/MenuGroup.h
+++ b/ruamoko/cl_menu/MenuGroup.h
@@ -10,15 +10,15 @@
 */
 @interface MenuGroup : Group
 {
-	int base;				///< The index of the first menu item.
-	int current;			///< The currently selected menu item.
+	unsigned base;				///< The index of the first menu item.
+	unsigned current;			///< The currently selected menu item.
 }
 
 /**	Set the index of the first menu item.
 
 	\param	b	The index of the first menu item.
 */
--(void) setBase: (int) b;
+-(void) setBase: (unsigned) b;
 
 /**	Select the next menu item.
 
diff --git a/ruamoko/cl_menu/MenuGroup.r b/ruamoko/cl_menu/MenuGroup.r
index 356d9d795..21d06b408 100644
--- a/ruamoko/cl_menu/MenuGroup.r
+++ b/ruamoko/cl_menu/MenuGroup.r
@@ -14,7 +14,7 @@
 	return self;
 }
 
--(void)setBase:(int)b
+-(void)setBase:(unsigned)b
 {
 	if (b >= [views count])
 		b = [views count] - 1;
@@ -51,7 +51,7 @@
 
 -(void) prev
 {
-	if (--current < base)
+	if (current-- == base)
 		current = [views count] - 1;
 	S_LocalSound ("misc/menu1.wav");
 }
diff --git a/ruamoko/lib/Array+Private.r b/ruamoko/lib/Array+Private.r
index 2cdea39cf..1a2e479e0 100644
--- a/ruamoko/lib/Array+Private.r
+++ b/ruamoko/lib/Array+Private.r
@@ -22,8 +22,8 @@
 
 	while (i-- > 0) {
 		if (_objs[i] == anObject) {
-			for (tmp = i; tmp < count - 1; tmp++) {
-				_objs[tmp] = _objs[tmp + 1];
+			for (tmp = i + 1; tmp < count; tmp++) {
+				_objs[tmp + 1] = _objs[tmp];
 			}
 			count--;
 		}
diff --git a/ruamoko/lib/Array.r b/ruamoko/lib/Array.r
index aa0f82082..b556051df 100644
--- a/ruamoko/lib/Array.r
+++ b/ruamoko/lib/Array.r
@@ -52,10 +52,9 @@
 
 + (id) arrayWithObjects: (id *) objs count: (unsigned)cnt
 {
-	local int i;
 	id	newArray = [self array];
 
-	for (i = 0; i < cnt; i++) {
+	for (unsigned i = 0; i < cnt; i++) {
 		[newArray addObject: (id) objs[i]];
 	}
 	return newArray;
@@ -138,12 +137,10 @@
 
 - (id) initWithObjects: (id *) objs count: (unsigned)cnt
 {
-	local int i;
-
 	if (!(self = [self initWithCapacity: cnt]))
 		return nil;
 
-	for (i = 0; i < cnt; i++) {
+	for (unsigned i = 0; i < cnt; i++) {
 		[self addObject: (id) objs[i]];
 	}
 	return self;
@@ -355,7 +352,6 @@
 
 - (void) removeObjectAtIndex: (unsigned)index
 {
-	local int	i;
 	local id		temp;
 
 	if (index >= count) // FIXME: need exceptions
@@ -363,7 +359,7 @@
 
 	temp = _objs[index];
 	count--;
-	for (i = index; i < count; i++) {	// reassign all objs >= index
+	for (unsigned i = index; i < count; i++) {	// reassign all objs >= index
 		_objs[i] = _objs[i+1];
 	}
 
@@ -393,9 +389,7 @@
 
 - (void) makeObjectsPerformSelector: (SEL)selector
 {
-	local int	i;
-
-	for (i = 0; i < [self count]; i++) {
+	for (unsigned i = 0; i < [self count]; i++) {
 		[[self objectAtIndex: i] performSelector: selector];
 	}
 }
@@ -403,9 +397,7 @@
 - (void) makeObjectsPerformSelector: (SEL)selector
                          withObject: (void *)anObject
 {
-	local int	i;
-
-	for (i = 0; i < [self count]; i++) {
+	for (unsigned i = 0; i < [self count]; i++) {
 		[[self objectAtIndex: i] performSelector: selector withObject: anObject];
 	}
 }
@@ -414,9 +406,7 @@
                          withObject: (void *)anObject
                          withObject: (void *)anotherObject
 {
-	local int	i;
-
-	for (i = 0; i < [self count]; i++) {
+	for (unsigned i = 0; i < [self count]; i++) {
 		[[self objectAtIndex: i] performSelector: selector
 									  withObject: anObject
 									  withObject: anotherObject];
diff --git a/ruamoko/qwaq/debugger/localsdata.r b/ruamoko/qwaq/debugger/localsdata.r
index 73b0f51cc..7849a2ced 100644
--- a/ruamoko/qwaq/debugger/localsdata.r
+++ b/ruamoko/qwaq/debugger/localsdata.r
@@ -79,7 +79,7 @@ free_defs (LocalsData *self)
 	num_user_defs = 0;
 	if (aux_func) {
 		defs = qdb_get_local_defs (target, fnum);
-		for (int i = 0; i < aux_func.num_locals; i++) {
+		for (unsigned i = 0; i < aux_func.num_locals; i++) {
 			string def_name = qdb_get_string (target, defs[i].name);
 			if (str_mid (def_name, 0, 1) != ".") {
 				num_user_defs++;
@@ -88,7 +88,7 @@ free_defs (LocalsData *self)
 		def_views = obj_malloc (num_user_defs);
 		def_rows = obj_malloc (num_user_defs + 1);
 		def_rows[0] = 0;
-		for (int i = 0, j = 0; i < aux_func.num_locals; i++) {
+		for (unsigned i = 0, j = 0; i < aux_func.num_locals; i++) {
 			string def_name = qdb_get_string (target, defs[i].name);
 			if (str_mid (def_name, 0, 1) == ".") {
 				continue;
diff --git a/ruamoko/qwaq/editor/editor.r b/ruamoko/qwaq/editor/editor.r
index 317d5ae21..94ba3da78 100644
--- a/ruamoko/qwaq/editor/editor.r
+++ b/ruamoko/qwaq/editor/editor.r
@@ -10,7 +10,7 @@
 static int
 center (unsigned v, int len)
 {
-	return v > len / 2 ? v / 2 : 0;
+	return v > (unsigned) (len / 2) ? v / 2 : 0;
 }
 
 static void
@@ -245,7 +245,7 @@ handleEvent (Editor *self, qwaq_event_t *event)
 
 -scrollLeft:(unsigned) count
 {
-	if (base.x > count) {
+	if ((unsigned) base.x > count) {
 		base.x -= count;
 	} else {
 		base.x = 0;
@@ -257,7 +257,7 @@ handleEvent (Editor *self, qwaq_event_t *event)
 
 -scrollRight:(unsigned) count
 {
-	if (1024 - base.x > count) {
+	if ((unsigned) (1024 - base.x) > count) {
 		base.x += count;
 	} else {
 		base.x = 1024;
@@ -269,9 +269,9 @@ handleEvent (Editor *self, qwaq_event_t *event)
 
 -scrollTo:(unsigned)target
 {
-	if (target > base.y) {
+	if (target > (unsigned) base.y) {
 		base_index = [buffer nextLine:base_index :target - base.y];
-	} else if (target < base.y) {
+	} else if (target < (unsigned) base.y) {
 		base_index = [buffer prevLine:base_index :base.y - target];
 	}
 	base.y = target;
@@ -285,7 +285,7 @@ handleEvent (Editor *self, qwaq_event_t *event)
 	[self recenter:0];
 	unsigned count = cursor.y;
 
-	if (count > ylen) {
+	if (count > (unsigned) ylen) {
 		count = ylen;
 	}
 	if (count) {
@@ -305,7 +305,7 @@ handleEvent (Editor *self, qwaq_event_t *event)
 	[self recenter:0];
 	unsigned count = line_count - cursor.y;
 
-	if (count > ylen) {
+	if (count > (unsigned) ylen) {
 		count = ylen;
 	}
 	if (count) {
@@ -389,7 +389,7 @@ handleEvent (Editor *self, qwaq_event_t *event)
 -charDown
 {
 	[self recenter:0];
-	if (cursor.y >= line_count) {
+	if ((unsigned) cursor.y >= line_count) {
 		return self;
 	}
 	cursor.y++;
@@ -547,7 +547,7 @@ handleEvent (Editor *self, qwaq_event_t *event)
 -moveEOS
 {
 	unsigned count = line_count - base.y;
-	if (count > ylen - 1) {
+	if (count > (unsigned) (ylen - 1)) {
 		count = ylen - 1;
 	}
 	line_index = [buffer nextLine:base_index :count];
@@ -603,9 +603,9 @@ handleEvent (Editor *self, qwaq_event_t *event)
 
 -gotoLine:(unsigned) line
 {
-	if (line > cursor.y) {
+	if (line > (unsigned) cursor.y) {
 		line_index = [buffer nextLine:line_index :line - cursor.y];
-	} else if (line < cursor.y) {
+	} else if (line < (unsigned) cursor.y) {
 		line_index = [buffer prevLine:line_index :cursor.y - line];
 	}
 	cursor.y = line;
diff --git a/ruamoko/qwaq/ui/garray.r b/ruamoko/qwaq/ui/garray.r
index 55aa35e88..5b5155f25 100644
--- a/ruamoko/qwaq/ui/garray.r
+++ b/ruamoko/qwaq/ui/garray.r
@@ -7,7 +7,7 @@
 								 if: (condition_func)condition
 							   with: (void *)data
 {
-	for (int i = 0; i < [self count]; i++) {
+	for (unsigned i = 0; i < [self count]; i++) {
 		if (condition (_objs[i], data)) {
 			[_objs[i] performSelector: selector];
 		}
@@ -19,7 +19,7 @@
 								 if: (condition_func2)condition
 							   with: (void *)data
 {
-	for (int i = 0; i < [self count]; i++) {
+	for (unsigned i = 0; i < [self count]; i++) {
 		if (condition (_objs[i], anObject, data)) {
 			[_objs[i] performSelector: selector withObject: anObject];
 		}
diff --git a/ruamoko/qwaq/ui/group.r b/ruamoko/qwaq/ui/group.r
index 6ba7926ac..9beb11c24 100644
--- a/ruamoko/qwaq/ui/group.r
+++ b/ruamoko/qwaq/ui/group.r
@@ -144,7 +144,7 @@ trySetFocus (Group *self, int viewIndex)
 
 -selectNext
 {
-	for (int i = focused + 1; i < [views count]; i++) {
+	for (unsigned i = focused + 1; i < [views count]; i++) {
 		if (trySetFocus (self, i)) {
 			return self;
 		}
diff --git a/ruamoko/scheme/CompiledCode.r b/ruamoko/scheme/CompiledCode.r
index ac49a8b5b..510d4988b 100644
--- a/ruamoko/scheme/CompiledCode.r
+++ b/ruamoko/scheme/CompiledCode.r
@@ -41,7 +41,7 @@
     
 - (void) compile
 {
-    local int index;
+    local unsigned index;
     local Instruction *inst;
     literals = [Frame newWithSize: [constants count] link: nil];
     code = obj_malloc (@sizeof(instruction_t) * [instructions count]);
diff --git a/ruamoko/scheme/Scope.r b/ruamoko/scheme/Scope.r
index 8df1e5a4b..e49395ae8 100644
--- a/ruamoko/scheme/Scope.r
+++ b/ruamoko/scheme/Scope.r
@@ -18,9 +18,7 @@
 
 - (int) indexLocal: (Symbol *) sym
 {
-    local int index;
-
-    for (index = 0; index < [names count]; index++) {
+    for (unsigned index = 0; index < [names count]; index++) {
             if (sym == [names objectAtIndex: index]) {
                     return index;
             }
diff --git a/tools/qfcc/source/expr_binary.c b/tools/qfcc/source/expr_binary.c
index 6c9d106b3..3d347c676 100644
--- a/tools/qfcc/source/expr_binary.c
+++ b/tools/qfcc/source/expr_binary.c
@@ -60,6 +60,7 @@ static const expr_t *inverse_multiply (int op, const expr_t *e1,
 									   const expr_t *e2);
 static const expr_t *double_compare (int op, const expr_t *e1,
 									 const expr_t *e2);
+static const expr_t *uint_compare (int op, const expr_t *e1, const expr_t *e2);
 static const expr_t *vector_compare (int op, const expr_t *e1, const expr_t *e2);
 static const expr_t *vector_dot (int op, const expr_t *e1, const expr_t *e2);
 static const expr_t *vector_multiply (int op, const expr_t *e1, const expr_t *e2);
@@ -391,10 +392,10 @@ static expr_type_t int_uint[] = {
 	{SHR,	&type_int, 0, &type_int},
 	{EQ,	&type_int, 0, &type_int},
 	{NE,	&type_int, 0, &type_int},
-	{LE,	&type_int, 0, &type_int},
-	{GE,	&type_int, 0, &type_int},
-	{LT,	&type_int, 0, &type_int},
-	{GT,	&type_int, 0, &type_int},
+	{LE,	.process = uint_compare},
+	{GE,	.process = uint_compare},
+	{LT,	.process = uint_compare},
+	{GT,	.process = uint_compare},
 	{0, 0}
 };
 
@@ -469,10 +470,10 @@ static expr_type_t uint_int[] = {
 	{SHR,	&type_uint, 0,        &type_int },
 	{EQ,	&type_int, &type_int, &type_int },
 	{NE,	&type_int, &type_int, &type_int },
-	{LE,	&type_int, &type_int, &type_int },
-	{GE,	&type_int, &type_int, &type_int },
-	{LT,	&type_int, &type_int, &type_int },
-	{GT,	&type_int, &type_int, &type_int },
+	{LE,	.process = uint_compare},
+	{GE,	.process = uint_compare},
+	{LT,	.process = uint_compare},
+	{GT,	.process = uint_compare},
 	{0, 0}
 };
 
@@ -1072,6 +1073,34 @@ double_compare (int op, const expr_t *e1, const expr_t *e2)
 	return e;
 }
 
+static const expr_t *
+uint_compare (int op, const expr_t *e1, const expr_t *e2)
+{
+	type_t     *t1 = get_type (e1);
+	type_t     *t2 = get_type (e2);
+	expr_t     *e;
+
+	if (is_constant (e1) && e1->implicit && is_int (t1)) {
+		t1 = &type_uint;
+		e1 = cast_expr (t1, e1);
+	}
+	if (is_constant (e2) && e2->implicit && is_int (t2)) {
+		t2 = &type_uint;
+		e2 = cast_expr (t2, e2);
+	}
+	if (t1 != t2) {
+		warning (e1, "comparison between signed and unsigned");
+		if (is_int (t1)) {
+			e1 = cast_expr (&type_uint, e2);
+		} else {
+			e2 = cast_expr (&type_uint, e2);
+		}
+	}
+	e = new_binary_expr (op, e1, e2);
+	e->expr.type = &type_int;
+	return e;
+}
+
 static const expr_t *
 entity_compare (int op, const expr_t *e1, const expr_t *e2)
 {
diff --git a/tools/qfcc/test/arraytypedef.r b/tools/qfcc/test/arraytypedef.r
index 288d40896..666cad195 100644
--- a/tools/qfcc/test/arraytypedef.r
+++ b/tools/qfcc/test/arraytypedef.r
@@ -41,7 +41,7 @@ find_xdef (string varname)
 	//FIXME need a simple way to get at a def's meta-data
 	xdefs_t    *xdefs = PR_FindGlobal (".xdefs");
 	xdef_t     *xdef = xdefs.xdefs;
-	while (xdef - xdefs.xdefs < xdefs.num_xdefs && xdef.addr != varaddr) {
+	while (xdef - xdefs.xdefs < (int) xdefs.num_xdefs && xdef.addr != varaddr) {
 		xdef++;
 	}
 	if (xdef.addr != varaddr) {
diff --git a/tools/qfcc/test/ptraliasenc.r b/tools/qfcc/test/ptraliasenc.r
index f42e560e2..02f783406 100644
--- a/tools/qfcc/test/ptraliasenc.r
+++ b/tools/qfcc/test/ptraliasenc.r
@@ -23,7 +23,8 @@ main (void)
 	//FIXME need a simple way to get at a def's meta-data
 	xdefs_t    *xdefs = PR_FindGlobal (".xdefs");
 	xdef_t     *xdef = xdefs.xdefs;
-	while (xdef - xdefs.xdefs < xdefs.num_xdefs && xdef.ofs != &int32_ptr) {
+	while (xdef - xdefs.xdefs < (int) xdefs.num_xdefs
+		   && xdef.ofs != &int32_ptr) {
 		xdef++;
 	}
 	printf ("int32_ptr: %s\n", xdef.type.encoding);
diff --git a/tools/qfcc/test/ptrfunc.r b/tools/qfcc/test/ptrfunc.r
index 12d4a77c7..e85a8b1b7 100644
--- a/tools/qfcc/test/ptrfunc.r
+++ b/tools/qfcc/test/ptrfunc.r
@@ -28,7 +28,7 @@ find_xdef (void *varaddr)
 	//FIXME need a simple way to get at a def's meta-data
 	xdefs_t    *xdefs = PR_FindGlobal (".xdefs");
 	xdef_t     *xdef = xdefs.xdefs;
-	while (xdef - xdefs.xdefs < xdefs.num_xdefs && xdef.ofs != varaddr) {
+	while (xdef - xdefs.xdefs < (int) xdefs.num_xdefs && xdef.ofs != varaddr) {
 		xdef++;
 	}
 	if (xdef.ofs != varaddr) {