Port some cleanup from PR #847

- Update some whitespace formatting
- Factor out some functions
- Free data after outputting to an object file

include/asm/rpn.h

@@ -30,7 +30,7 @@ struct Expression {
 /*
  * Determines if an expression is known at assembly time
  */
-static inline bool rpn_isKnown(const struct Expression *expr)
+static inline bool rpn_isKnown(struct Expression const *expr)
 {
 	return expr->isKnown;
 }
@@ -64,5 +64,7 @@ void rpn_StartOfSection(struct Expression *expr, char const *sectionName);
 void rpn_Free(struct Expression *expr);
 void rpn_CheckHRAM(struct Expression *expr, const struct Expression *src);
 void rpn_CheckRST(struct Expression *expr, const struct Expression *src);
+void rpn_CheckNBit(struct Expression const *expr, uint8_t n);
+int32_t rpn_GetConstVal(struct Expression const *expr);
 
 #endif /* RGBDS_ASM_RPN_H */

include/linkdefs.h

@@ -28,17 +28,17 @@ enum RPNCommand {
 	RPN_MUL			= 0x02,
 	RPN_DIV			= 0x03,
 	RPN_MOD			= 0x04,
-	RPN_UNSUB		= 0x05,
+	RPN_UNSUB		= 0x05, // FIXME: should be renamed to "NEG" for consistency
 	RPN_EXP			= 0x06,
 
 	RPN_OR			= 0x10,
 	RPN_AND			= 0x11,
 	RPN_XOR			= 0x12,
-	RPN_UNNOT		= 0x13,
+	RPN_UNNOT		= 0x13, // FIXME: should be renamed to "NOT" for consistency
 
 	RPN_LOGAND		= 0x21,
 	RPN_LOGOR		= 0x22,
-	RPN_LOGUNNOT		= 0x23,
+	RPN_LOGUNNOT		= 0x23, // FIXME: should be renamed to "LOGNOT" for consistency
 
 	RPN_LOGEQ		= 0x30,
 	RPN_LOGNE		= 0x31,

src/asm/output.c

@@ -242,6 +242,21 @@ static void writesection(struct Section const *sect, FILE *f)
 	}
 }
 
+static void freesection(struct Section const *sect)
+{
+	if (sect_HasData(sect->type)) {
+		struct Patch *patch = sect->patches;
+
+		while (patch != NULL) {
+			struct Patch *next = patch->next;
+
+			free(patch->rpn);
+			free(patch);
+			patch = next;
+		}
+	}
+}
+
 /*
  * Write a symbol to a file
  */
@@ -468,6 +483,13 @@ static void writeassert(struct Assertion *assert, FILE *f)
 	putstring(assert->message, f);
 }
 
+static void freeassert(struct Assertion *assert)
+{
+	free(assert->patch->rpn);
+	free(assert->patch);
+	free(assert);
+}
+
 static void writeFileStackNode(struct FileStackNode const *node, FILE *f)
 {
 	putlong(node->parent ? node->parent->ID : -1, f);
@@ -530,13 +552,21 @@ void out_WriteObject(void)
 	for (struct Symbol const *sym = objectSymbols; sym; sym = sym->next)
 		writesymbol(sym, f);
 
-	for (struct Section *sect = sectionList; sect; sect = sect->next)
+	for (struct Section *sect = sectionList; sect; sect = sect->next) {
 		writesection(sect, f);
+		freesection(sect);
+	}
 
 	putlong(countAsserts(), f);
-	for (struct Assertion *assert = assertions; assert;
-	     assert = assert->next)
+	struct Assertion *assert = assertions;
+
+	while (assert != NULL) {
+		struct Assertion *next = assert->next;
+
 		writeassert(assert, f);
+		freeassert(assert);
+		assert = next;
+	}
 
 	fclose(f);
 }

src/asm/parser.y

@@ -481,6 +481,7 @@ enum {
 %type	<expr>		relocexpr_no_str
 %type	<constValue>	const
 %type	<constValue>	const_no_str
+%type	<constValue>	const_8bit
 %type	<constValue>	uconst
 %type	<constValue>	rs_uconst
 %type	<constValue>	const_3bit
@@ -952,8 +953,7 @@ assert_type	: %empty		{ $$ = ASSERT_ERROR; }
 		| T_POP_FATAL T_COMMA { $$ = ASSERT_FATAL; }
 ;
 
-assert		: T_POP_ASSERT assert_type relocexpr
-		{
+assert		: T_POP_ASSERT assert_type relocexpr {
 			if (!rpn_isKnown(&$3)) {
 				if (!out_CreateAssert($2, &$3, "",
 						      sect_GetOutputOffset()))
@@ -964,8 +964,7 @@ assert		: T_POP_ASSERT assert_type relocexpr
 			}
 			rpn_Free(&$3);
 		}
-		| T_POP_ASSERT assert_type relocexpr T_COMMA string
-		{
+		| T_POP_ASSERT assert_type relocexpr T_COMMA string {
 			if (!rpn_isKnown(&$3)) {
 				if (!out_CreateAssert($2, &$3, $5,
 						      sect_GetOutputOffset()))
@@ -976,13 +975,11 @@ assert		: T_POP_ASSERT assert_type relocexpr
 			}
 			rpn_Free(&$3);
 		}
-		| T_POP_STATIC_ASSERT assert_type const
-		{
+		| T_POP_STATIC_ASSERT assert_type const {
 			if ($3 == 0)
 				failAssert($2);
 		}
-		| T_POP_STATIC_ASSERT assert_type const T_COMMA string
-		{
+		| T_POP_STATIC_ASSERT assert_type const T_COMMA string {
 			if ($3 == 0)
 				failAssertMsg($2, $5);
 		}
@@ -1205,9 +1202,7 @@ incbin		: T_POP_INCBIN string {
 		}
 ;
 
-charmap		: T_POP_CHARMAP string T_COMMA const {
-			if ($4 < INT8_MIN || $4 > UINT8_MAX)
-				warning(WARNING_TRUNCATION, "Expression must be 8-bit\n");
+charmap		: T_POP_CHARMAP string T_COMMA const_8bit {
 			charmap_Add($2, (uint8_t)$4);
 		}
 ;
@@ -1318,11 +1313,12 @@ constlist_32bit : constlist_32bit_entry
 		| constlist_32bit T_COMMA constlist_32bit_entry
 ;
 
-constlist_32bit_entry :relocexpr_no_str	{
+constlist_32bit_entry : relocexpr_no_str {
 			out_RelLong(&$1, 0);
 		}
 		| string {
-			uint8_t *output = malloc(strlen($1)); /* Cannot be larger than that */
+			// Charmaps cannot increase the length of a string
+			uint8_t *output = malloc(strlen($1));
 			int32_t length = charmap_Convert($1, output);
 
 			out_AbsLongGroup(output, length);
@@ -1331,33 +1327,25 @@ constlist_32bit_entry :relocexpr_no_str	{
 ;
 
 reloc_8bit	: relocexpr {
-			if(rpn_isKnown(&$1)
-			 && ($1.val < -128 || $1.val > 255))
-				warning(WARNING_TRUNCATION, "Expression must be 8-bit\n");
+			rpn_CheckNBit(&$1, 8);
 			$$ = $1;
 		}
 ;
 
 reloc_8bit_no_str : relocexpr_no_str {
-			if(rpn_isKnown(&$1)
-			 && ($1.val < -128 || $1.val > 255))
-				warning(WARNING_TRUNCATION, "Expression must be 8-bit\n");
+			rpn_CheckNBit(&$1, 8);
 			$$ = $1;
 		}
 ;
 
 reloc_16bit	: relocexpr {
-			if (rpn_isKnown(&$1)
-			 && ($1.val < -32768 || $1.val > 65535))
-				warning(WARNING_TRUNCATION, "Expression must be 16-bit\n");
+			rpn_CheckNBit(&$1, 16);
 			$$ = $1;
 		}
 ;
 
 reloc_16bit_no_str : relocexpr_no_str {
-			if (rpn_isKnown(&$1)
-			 && ($1.val < -32768 || $1.val > 65535))
-				warning(WARNING_TRUNCATION, "Expression must be 16-bit\n");
+			rpn_CheckNBit(&$1, 16);
 			$$ = $1;
 		}
 ;
@@ -1365,7 +1353,8 @@ reloc_16bit_no_str : relocexpr_no_str {
 
 relocexpr	: relocexpr_no_str
 		| string {
-			uint8_t *output = malloc(strlen($1)); /* Cannot be longer than that */
+			// Charmaps cannot increase the length of a string
+			uint8_t *output = malloc(strlen($1));
 			int32_t length = charmap_Convert($1, output);
 			uint32_t r = str2int2(output, length);
 
@@ -1441,9 +1430,9 @@ relocexpr_no_str : scoped_anon_id	{ rpn_Symbol(&$$, $1); }
 		| T_OP_NOT relocexpr %prec NEG { rpn_UNNOT(&$$, &$2); }
 		| T_OP_HIGH T_LPAREN relocexpr T_RPAREN { rpn_HIGH(&$$, &$3); }
 		| T_OP_LOW T_LPAREN relocexpr T_RPAREN { rpn_LOW(&$$, &$3); }
-		| T_OP_ISCONST T_LPAREN relocexpr T_RPAREN{ rpn_ISCONST(&$$, &$3); }
+		| T_OP_ISCONST T_LPAREN relocexpr T_RPAREN { rpn_ISCONST(&$$, &$3); }
 		| T_OP_BANK T_LPAREN scoped_anon_id T_RPAREN {
-			/* '@' is also a T_ID, it is handled here. */
+			// '@' is also a T_ID; it is handled here
 			rpn_BankSymbol(&$$, $3);
 		}
 		| T_OP_BANK T_LPAREN string T_RPAREN { rpn_BankSection(&$$, $3); }
@@ -1525,31 +1514,17 @@ relocexpr_no_str : scoped_anon_id	{ rpn_Symbol(&$$, $1); }
 uconst		: const {
 			$$ = $1;
 			if ($$ < 0)
-				fatalerror("Constant mustn't be negative: %d\n",
-					   $1);
+				fatalerror("Constant mustn't be negative: %d\n", $1);
 		}
 ;
 
-const		: relocexpr {
-			if (!rpn_isKnown(&$1)) {
-				error("Expected constant expression: %s\n",
-					$1.reason);
-				$$ = 0;
-			} else {
-				$$ = $1.val;
-			}
-		}
+const		: relocexpr { $$ = rpn_GetConstVal(&$1); }
 ;
 
-const_no_str	: relocexpr_no_str {
-			if (!rpn_isKnown(&$1)) {
-				error("Expected constant expression: %s\n",
-					$1.reason);
-				$$ = 0;
-			} else {
-				$$ = $1.val;
-			}
-		}
+const_no_str	: relocexpr_no_str { $$ = rpn_GetConstVal(&$1); }
+;
+
+const_8bit	: reloc_8bit { $$ = rpn_GetConstVal(&$1); }
 ;
 
 string		: T_STRING
@@ -1615,20 +1590,10 @@ strfmt_args	: string strfmt_va_args {
 strfmt_va_args	: %empty {
 			initStrFmtArgList(&$$);
 		}
-		| strfmt_va_args T_COMMA relocexpr_no_str {
-			int32_t value;
-
-			if (!rpn_isKnown(&$3)) {
-				error("Expected constant expression: %s\n",
-					$3.reason);
-				value = 0;
-			} else {
-				value = $3.val;
-			}
-
+		| strfmt_va_args T_COMMA const_no_str {
 			size_t i = nextStrFmtArgListIndex(&$1);
 
-			$1.args[i].number = value;
+			$1.args[i].number = $3;
 			$1.args[i].isNumeric = true;
 			$$ = $1;
 		}
@@ -1648,7 +1613,7 @@ section		: T_POP_SECTION sectmod string T_COMMA sectiontype sectorg sectattrs {
 
 sectmod		: %empty { $$ = SECTION_NORMAL; }
 		| T_POP_UNION { $$ = SECTION_UNION; }
-		| T_POP_FRAGMENT{ $$ = SECTION_FRAGMENT; }
+		| T_POP_FRAGMENT { $$ = SECTION_FRAGMENT; }
 ;
 
 sectiontype	: T_SECT_WRAM0 { $$ = SECTTYPE_WRAM0; }
@@ -2014,8 +1979,7 @@ z80_res		: T_Z80_RES const_3bit T_COMMA reg_r {
 		}
 ;
 
-z80_ret		: T_Z80_RET	{ out_AbsByte(0xC9);
-		}
+z80_ret		: T_Z80_RET { out_AbsByte(0xC9); }
 		| T_Z80_RET ccode { out_AbsByte(0xC0 | ($2 << 3)); }
 ;
 
@@ -2116,8 +2080,7 @@ z80_sub		: T_Z80_SUB op_a_n {
 			out_AbsByte(0xD6);
 			out_RelByte(&$2, 1);
 		}
-		| T_Z80_SUB op_a_r	{ out_AbsByte(0x90 | $2);
-		}
+		| T_Z80_SUB op_a_r { out_AbsByte(0x90 | $2); }
 ;
 
 z80_swap	: T_Z80_SWAP reg_r {

src/asm/rpn.c

@@ -13,6 +13,7 @@
 #include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -75,7 +76,7 @@ static uint8_t *reserveSpace(struct Expression *expr, uint32_t size)
 }
 
 /*
- * Init the RPN expression
+ * Init a RPN expression
  */
 static void rpn_Init(struct Expression *expr)
 {
@@ -260,6 +261,31 @@ void rpn_CheckRST(struct Expression *expr, const struct Expression *src)
 	}
 }
 
+/*
+ * Checks that an RPN expression's value fits within N bits (signed or unsigned)
+ */
+void rpn_CheckNBit(struct Expression const *expr, uint8_t n)
+{
+	assert(n != 0); // That doesn't make sense
+	assert(n < CHAR_BIT * sizeof(int)); // Otherwise `1 << n` is UB
+
+	if (rpn_isKnown(expr)) {
+		int32_t val = expr->val;
+
+		if (val < -(1 << (n - 1)) || val >= 1 << n)
+			warning(WARNING_TRUNCATION, "Expression must be %u-bit\n", n);
+	}
+}
+
+int32_t rpn_GetConstVal(struct Expression const *expr)
+{
+	if (!rpn_isKnown(expr)) {
+		error("Expected constant expression: %s\n", expr->reason);
+		return 0;
+	}
+	return expr->val;
+}
+
 void rpn_LOGNOT(struct Expression *expr, const struct Expression *src)
 {
 	*expr = *src;