From cc59730c5be4ed9efc164238fb58e15108a5b2ff Mon Sep 17 00:00:00 2001
From: ISSOtm <eldredhabert0@gmail.com>
Date: Mon, 20 Jan 2020 14:04:14 +0100
Subject: [PATCH] Cleanup the RPN evaluator somewhat Make the bool field an
 actual bool Rename `iReloc` to a more exact `isKnown` (as was already pointed
 out by some comments) Make the value of `BANK(symbol)` consistent when the
 argument is invalid

---
 include/asm/rpn.h |  5 ++--
 src/asm/asmy.y    | 19 ++++++++-------
 src/asm/output.c  |  6 ++---
 src/asm/rpn.c     | 60 ++++++++++++++++++++++-------------------------
 4 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/include/asm/rpn.h b/include/asm/rpn.h
index 1b0a75b6..6bff26a9 100644
--- a/include/asm/rpn.h
+++ b/include/asm/rpn.h
@@ -10,6 +10,7 @@
 #define RGBDS_ASM_RPN_H
 
 #include <stdint.h>
+#include <stdbool.h>
 
 #include "linkdefs.h"
 
@@ -22,13 +23,13 @@ struct Expression {
 	uint32_t nRPNLength;
 	uint32_t nRPNPatchSize;
 	uint32_t nRPNOut;
-	uint32_t isReloc;
+	bool isKnown;
 };
 
 /* FIXME: Should be defined in `asmy.h`, but impossible with POSIX Yacc */
 extern int32_t nPCOffset;
 
-uint32_t rpn_isReloc(const struct Expression *expr);
+bool rpn_isKnown(const struct Expression *expr);
 void rpn_Symbol(struct Expression *expr, char *tzSym);
 void rpn_Number(struct Expression *expr, uint32_t i);
 void rpn_LOGNOT(struct Expression *expr, const struct Expression *src);
diff --git a/src/asm/asmy.y b/src/asm/asmy.y
index 3f417eab..8d24e441 100644
--- a/src/asm/asmy.y
+++ b/src/asm/asmy.y
@@ -1190,7 +1190,7 @@ constlist_32bit_entry : /* empty */
 
 const_8bit	: relocconst
 		{
-			if( (!rpn_isReloc(&$1)) && (($1.nVal < -128) || ($1.nVal > 255)) )
+			if( (rpn_isKnown(&$1)) && (($1.nVal < -128) || ($1.nVal > 255)) )
 				warning(WARNING_TRUNCATION, "Expression must be 8-bit");
 			$$ = $1;
 		}
@@ -1198,7 +1198,7 @@ const_8bit	: relocconst
 
 const_16bit	: relocconst
 		{
-			if ((!rpn_isReloc(&$1)) && (($1.nVal < -32768) || ($1.nVal > 65535)))
+			if ((rpn_isKnown(&$1)) && (($1.nVal < -32768) || ($1.nVal > 65535)))
 				warning(WARNING_TRUNCATION, "Expression must be 16-bit");
 			$$ = $1;
 		}
@@ -1769,7 +1769,7 @@ z80_ldio	: T_Z80_LDIO T_MODE_A comma op_mem_ind
 		{
 			rpn_CheckHRAM(&$4, &$4);
 
-			if ((!rpn_isReloc(&$4)) && ($4.nVal < 0 || ($4.nVal > 0xFF && $4.nVal < 0xFF00) || $4.nVal > 0xFFFF))
+			if ((rpn_isKnown(&$4)) && ($4.nVal < 0 || ($4.nVal > 0xFF && $4.nVal < 0xFF00) || $4.nVal > 0xFFFF))
 				yyerror("Source address $%x not in $FF00 to $FFFF", $4.nVal);
 
 			out_AbsByte(0xF0);
@@ -1780,7 +1780,7 @@ z80_ldio	: T_Z80_LDIO T_MODE_A comma op_mem_ind
 		{
 			rpn_CheckHRAM(&$2, &$2);
 
-			if ((!rpn_isReloc(&$2)) && ($2.nVal < 0 || ($2.nVal > 0xFF && $2.nVal < 0xFF00) || $2.nVal > 0xFFFF))
+			if ((rpn_isKnown(&$2)) && ($2.nVal < 0 || ($2.nVal > 0xFF && $2.nVal < 0xFF00) || $2.nVal > 0xFFFF))
 				yyerror("Destination address $%x not in $FF00 to $FFFF", $2.nVal);
 
 			out_AbsByte(0xE0);
@@ -1844,7 +1844,7 @@ z80_ld_mem	: T_Z80_LD op_mem_ind comma T_MODE_SP
 		| T_Z80_LD op_mem_ind comma T_MODE_A
 		{
 			if (CurrentOptions.optimizeloads &&
-			    (!rpn_isReloc(&$2)) && ($2.nVal >= 0xFF00)) {
+			    (rpn_isKnown(&$2)) && ($2.nVal >= 0xFF00)) {
 				out_AbsByte(0xE0);
 				out_AbsByte($2.nVal & 0xFF);
 				rpn_Free(&$2);
@@ -1899,7 +1899,7 @@ z80_ld_a	: T_Z80_LD reg_r comma T_MODE_C_IND
 		{
 			if ($2 == REG_A) {
 				if (CurrentOptions.optimizeloads &&
-				    (!rpn_isReloc(&$4)) && ($4.nVal >= 0xFF00)) {
+				    (rpn_isKnown(&$4)) && ($4.nVal >= 0xFF00)) {
 					out_AbsByte(0xF0);
 					out_AbsByte($4.nVal & 0xFF);
 					rpn_Free(&$4);
@@ -2036,13 +2036,14 @@ z80_rrca	: T_Z80_RRCA
 
 z80_rst		: T_Z80_RST const_8bit
 		{
-			if (rpn_isReloc(&$2)) {
+			if (!rpn_isKnown(&$2)) {
 				rpn_CheckRST(&$2, &$2);
 				out_RelByte(&$2);
-			} else if (($2.nVal & 0x38) != $2.nVal)
+			} else if (($2.nVal & 0x38) != $2.nVal) {
 				yyerror("Invalid address $%x for RST", $2.nVal);
-			else
+			} else {
 				out_AbsByte(0xC7 | $2.nVal);
+			}
 			rpn_Free(&$2);
 		}
 ;
diff --git a/src/asm/output.c b/src/asm/output.c
index e1cfd87d..0b7f71b8 100644
--- a/src/asm/output.c
+++ b/src/asm/output.c
@@ -766,7 +766,7 @@ void out_RelByte(struct Expression *expr)
 {
 	checkcodesection();
 	checksectionoverflow(1);
-	if (rpn_isReloc(expr)) {
+	if (!rpn_isKnown(expr)) {
 		pCurrentSection->tData[nPC] = 0;
 		createpatch(PATCHTYPE_BYTE, expr);
 		pCurrentSection->nPC++;
@@ -801,7 +801,7 @@ void out_RelWord(struct Expression *expr)
 {
 	checkcodesection();
 	checksectionoverflow(2);
-	if (rpn_isReloc(expr)) {
+	if (!rpn_isKnown(expr)) {
 		pCurrentSection->tData[nPC] = 0;
 		pCurrentSection->tData[nPC + 1] = 0;
 		createpatch(PATCHTYPE_WORD, expr);
@@ -838,7 +838,7 @@ void out_RelLong(struct Expression *expr)
 {
 	checkcodesection();
 	checksectionoverflow(4);
-	if (rpn_isReloc(expr)) {
+	if (!rpn_isKnown(expr)) {
 		pCurrentSection->tData[nPC] = 0;
 		pCurrentSection->tData[nPC + 1] = 0;
 		pCurrentSection->tData[nPC + 2] = 0;
diff --git a/src/asm/rpn.c b/src/asm/rpn.c
index 2c93515a..7f1a2644 100644
--- a/src/asm/rpn.c
+++ b/src/asm/rpn.c
@@ -25,7 +25,7 @@
 /*
  * Add a byte to the RPN expression
  */
-void pushbyte(struct Expression *expr, uint8_t b)
+static void pushbyte(struct Expression *expr, uint8_t b)
 {
 	if (expr->nRPNLength == expr->nRPNCapacity) {
 		if (expr->nRPNCapacity == 0)
@@ -55,7 +55,7 @@ void rpn_Init(struct Expression *expr)
 	expr->nRPNLength = 0;
 	expr->nRPNPatchSize = 0;
 	expr->nRPNOut = 0;
-	expr->isReloc = 0;
+	expr->isKnown = true;
 }
 
 /*
@@ -79,11 +79,11 @@ uint16_t rpn_PopByte(struct Expression *expr)
 }
 
 /*
- * Determine if the current expression is relocatable
+ * Determine if the current expression is known at assembly time
  */
-uint32_t rpn_isReloc(const struct Expression *expr)
+bool rpn_isKnown(const struct Expression *expr)
 {
-	return expr->isReloc;
+	return expr->isKnown;
 }
 
 /*
@@ -108,7 +108,7 @@ void rpn_Symbol(struct Expression *expr, char *tzSym)
 	if (!sym || !sym_IsConstant(sym)) {
 		rpn_Init(expr);
 		sym_Ref(tzSym);
-		expr->isReloc = 1;
+		expr->isKnown = false;
 		pushbyte(expr, RPN_SYM);
 		while (*tzSym)
 			pushbyte(expr, *tzSym++);
@@ -132,11 +132,7 @@ void rpn_BankSelf(struct Expression *expr)
 	rpn_Init(expr);
 
 	if (pCurrentSection->nBank == -1)
-		/*
-		 * This is not really relocatable, but this makes the assembler
-		 * write this expression as a RPN patch to the object file.
-		 */
-		expr->isReloc = 1;
+		expr->isKnown = false;
 	else
 		expr->nVal = pCurrentSection->nBank;
 
@@ -154,10 +150,10 @@ void rpn_BankSymbol(struct Expression *expr, char *tzSym)
 		return;
 	}
 
+	rpn_Init(expr);
 	if (sym && sym_IsConstant(sym)) {
 		yyerror("BANK argument must be a relocatable identifier");
 	} else {
-		rpn_Init(expr);
 		sym_Ref(tzSym);
 		pushbyte(expr, RPN_BANK_SYM);
 		for (unsigned int i = 0; tzSym[i]; i++)
@@ -172,7 +168,7 @@ void rpn_BankSymbol(struct Expression *expr, char *tzSym)
 			/* Symbol's section is known and bank's fixed */
 			expr->nVal = pSymbol->pSection->nBank;
 		else
-			expr->isReloc = 1;
+			expr->isKnown = false;
 	}
 }
 
@@ -185,11 +181,7 @@ void rpn_BankSection(struct Expression *expr, char *tzSectionName)
 	if (pSection && pSection->nBank != -1)
 		expr->nVal = pSection->nBank;
 	else
-		/*
-		 * This is not really relocatable, but this makes the assembler
-		 * write this expression as a RPN patch to the object file.
-		 */
-		expr->isReloc = 1;
+		expr->isKnown = false;
 
 	pushbyte(expr, RPN_BANK_SECT);
 	expr->nRPNPatchSize++;
@@ -272,11 +264,11 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 {
 	assert(src1->tRPN != NULL && src2->tRPN != NULL);
 
-	if (src1->nRPNLength + src2->nRPNLength > MAXRPNLEN)
-		fatalerror("RPN expression is too large");
-
 	uint32_t len = src1->nRPNLength + src2->nRPNLength;
 
+	if (len > MAXRPNLEN)
+		fatalerror("RPN expression is too large");
+
 	expr->nVal = 0;
 	expr->tRPN = src1->tRPN;
 
@@ -302,7 +294,7 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 	expr->nRPNLength = len;
 	expr->nRPNPatchSize = src1->nRPNPatchSize + src2->nRPNPatchSize;
 	expr->nRPNOut = 0;
-	expr->isReloc = src1->isReloc || src2->isReloc;
+	expr->isKnown = src1->isKnown && src2->isKnown;
 
 	switch (op) {
 	case RPN_LOGOR:
@@ -345,24 +337,28 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 		expr->nVal = src1->nVal & src2->nVal;
 		break;
 	case RPN_SHL:
-		if (src2->nVal < 0)
-			warning(WARNING_SHIFT_AMOUNT, "Shifting left by negative value: %d",
-				src2->nVal);
+		if (expr->isKnown) {
+			if (src2->nVal < 0)
+				warning(WARNING_SHIFT_AMOUNT, "Shifting left by negative value: %d",
+					src2->nVal);
 
-		expr->nVal = shift(src1->nVal, src2->nVal);
+			expr->nVal = shift(src1->nVal, src2->nVal);
+		}
 		break;
 	case RPN_SHR:
-		if (src2->nVal < 0)
-			warning(WARNING_SHIFT_AMOUNT, "Shifting right by negative value: %d",
-				src2->nVal);
+		if (expr->isKnown) {
+			if (src2->nVal < 0)
+				warning(WARNING_SHIFT_AMOUNT, "Shifting right by negative value: %d",
+					src2->nVal);
 
-		expr->nVal = shift(src1->nVal, -src2->nVal);
+			expr->nVal = shift(src1->nVal, -src2->nVal);
+		}
 		break;
 	case RPN_MUL:
 		expr->nVal = (uint32_t)src1->nVal * (uint32_t)src2->nVal;
 		break;
 	case RPN_DIV:
-		if (!expr->isReloc) {
+		if (expr->isKnown) {
 			if (src2->nVal == 0)
 				fatalerror("Division by zero");
 
@@ -375,7 +371,7 @@ void rpn_BinaryOp(enum RPNCommand op, struct Expression *expr,
 		}
 		break;
 	case RPN_MOD:
-		if (!expr->isReloc) {
+		if (expr->isKnown) {
 			if (src2->nVal == 0)
 				fatalerror("Division by zero");