Make some error messages more consistent (#1393)

* Update some error messages * Make non-A destination operand syntactically invalid
gbdev · Apr 20, 2024 · 3e9d2ca · 3e9d2ca
1 parent 7aecc00
commit 3e9d2ca
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 61 deletions.
diff --git a/src/asm/parser.y b/src/asm/parser.y
@@ -108,7 +108,8 @@
 %type <int32_t> const_8bit
 %type <int32_t> uconst
 %type <int32_t> rs_uconst
-%type <int32_t> const_3bit
+%type <int32_t> shift_const
+%type <int32_t> bit_const
 %type <Expression> reloc_8bit
 %type <Expression> reloc_8bit_no_str
 %type <Expression> reloc_8bit_offset
@@ -288,6 +289,8 @@
 %token CC_NZ "nz" CC_Z "z" CC_NC "nc" // There is no CC_C, only TOKEN_C
 
 %type <int32_t> reg_r
+%type <int32_t> reg_r_no_a
+%type <int32_t> reg_a
 %type <int32_t> reg_ss
 %type <int32_t> reg_rr
 %type <int32_t> reg_tt
@@ -735,14 +738,7 @@ assert:
 ;
 
 shift:
-	POP_SHIFT {
-		if (MacroArgs *macroArgs = fstk_GetCurrentMacroArgs(); macroArgs) {
-			macroArgs->shiftArgs(1);
-		} else {
-			::error("Cannot shift macro arguments outside of a macro\n");
-		}
-	}
-	| POP_SHIFT const {
+	POP_SHIFT shift_const {
 		if (MacroArgs *macroArgs = fstk_GetCurrentMacroArgs(); macroArgs) {
 			macroArgs->shiftArgs($2);
 		} else {
@@ -751,6 +747,13 @@ shift:
 	}
 ;
 
+shift_const:
+	%empty {
+		$$ = 1;
+	}
+	| const
+;
+
 load:
 	POP_LOAD sect_mod string COMMA sect_type sect_org sect_attrs {
 		sect_SetLoadSection($3, (SectionType)$5, $6, $7, $2);
@@ -1097,15 +1100,12 @@ print_expr:
 	}
 ;
 
-const_3bit:
+bit_const:
 	const {
-		int32_t value = $1;
-
-		if ((value < 0) || (value > 7)) {
-			::error("Immediate value must be 3-bit\n");
+		$$ = $1;
+		if ($$ < 0 || $$ > 7) {
+			::error("Bit number must be between 0 and 7, not %" PRId32 "\n", $$);
 			$$ = 0;
-		} else {
-			$$ = value & 0x7;
 		}
 	}
 ;
@@ -1436,7 +1436,7 @@ opt_q_arg:
 	| COMMA const {
 		$$ = $2;
 		if ($$ < 1 || $$ > 31) {
-			::error("Fixed-point precision must be between 1 and 31\n");
+			::error("Fixed-point precision must be between 1 and 31, not %" PRId32 "\n", $$);
 			$$ = fix_Precision();
 		}
 	}
@@ -1698,7 +1698,7 @@ z80_and:
 ;
 
 z80_bit:
-	Z80_BIT const_3bit COMMA reg_r {
+	Z80_BIT bit_const COMMA reg_r {
 		sect_AbsByte(0xCB);
 		sect_AbsByte(0x40 | ($2 << 3) | $4);
 	}
@@ -1846,19 +1846,20 @@ z80_ldio:
 c_ind:
 	  LBRACK MODE_C RBRACK
 	| LBRACK relocexpr OP_ADD MODE_C RBRACK {
-		if (!$2.isKnown() || $2.value() != 0xFF00)
-			::error("Expected constant expression equal to $FF00 for \"$ff00+c\"\n");
+		// This has to use `relocexpr`, not `const`, to avoid a shift/reduce conflict
+		if ($2.getConstVal() != 0xFF00)
+			::error("Base value must be equal to $FF00 for $FF00+C\n");
 	}
 ;
 
 z80_ld:
 	  z80_ld_mem
-	| z80_ld_cind
+	| z80_ld_c_ind
 	| z80_ld_rr
 	| z80_ld_ss
 	| z80_ld_hl
 	| z80_ld_sp
-	| z80_ld_r
+	| z80_ld_r_no_a
 	| z80_ld_a
 ;
 
@@ -1894,7 +1895,7 @@ z80_ld_mem:
 	}
 ;
 
-z80_ld_cind:
+z80_ld_c_ind:
 	Z80_LD c_ind COMMA MODE_A {
 		sect_AbsByte(0xE2);
 	}
@@ -1906,39 +1907,36 @@ z80_ld_rr:
 	}
 ;
 
-z80_ld_r:
-	Z80_LD reg_r COMMA reloc_8bit {
+z80_ld_r_no_a:
+	Z80_LD reg_r_no_a COMMA reloc_8bit {
 		sect_AbsByte(0x06 | ($2 << 3));
 		sect_RelByte($4, 1);
 	}
-	| Z80_LD reg_r COMMA reg_r {
-		if (($2 == REG_HL_IND) && ($4 == REG_HL_IND))
-			::error("LD [HL],[HL] not a valid instruction\n");
+	| Z80_LD reg_r_no_a COMMA reg_r {
+		if ($2 == REG_HL_IND && $4 == REG_HL_IND)
+			::error("LD [HL], [HL] is not a valid instruction\n");
 		else
 			sect_AbsByte(0x40 | ($2 << 3) | $4);
 	}
 ;
 
 z80_ld_a:
-	Z80_LD reg_r COMMA c_ind {
-		if ($2 == REG_A)
-			sect_AbsByte(0xF2);
-		else
-			::error("Destination operand must be A\n");
+	Z80_LD reg_a COMMA reloc_8bit {
+		sect_AbsByte(0x06 | ($2 << 3));
+		sect_RelByte($4, 1);
 	}
-	| Z80_LD reg_r COMMA reg_rr {
-		if ($2 == REG_A)
-			sect_AbsByte(0x0A | ($4 << 4));
-		else
-			::error("Destination operand must be A\n");
+	| Z80_LD reg_a COMMA reg_r {
+		sect_AbsByte(0x40 | ($2 << 3) | $4);
 	}
-	| Z80_LD reg_r COMMA op_mem_ind {
-		if ($2 == REG_A) {
-			sect_AbsByte(0xFA);
-			sect_RelWord($4, 1);
-		} else {
-			::error("Destination operand must be A\n");
-		}
+	| Z80_LD reg_a COMMA c_ind {
+		sect_AbsByte(0xF2);
+	}
+	| Z80_LD reg_a COMMA reg_rr {
+		sect_AbsByte(0x0A | ($4 << 4));
+	}
+	| Z80_LD reg_a COMMA op_mem_ind {
+		sect_AbsByte(0xFA);
+		sect_RelWord($4, 1);
 	}
 ;
 
@@ -1984,7 +1982,7 @@ z80_push:
 ;
 
 z80_res:
-	Z80_RES const_3bit COMMA reg_r {
+	Z80_RES bit_const COMMA reg_r {
 		sect_AbsByte(0xCB);
 		sect_AbsByte(0x80 | ($2 << 3) | $4);
 	}
@@ -2084,7 +2082,7 @@ z80_scf:
 ;
 
 z80_set:
-	Z80_SET const_3bit COMMA reg_r {
+	Z80_SET bit_const COMMA reg_r {
 		sect_AbsByte(0xCB);
 		sect_AbsByte(0xC0 | ($2 << 3) | $4);
 	}
@@ -2232,7 +2230,9 @@ ccode:
 	}
 ;
 
-reg_r:
+reg_r: reg_r_no_a | reg_a;
+
+reg_r_no_a:
 	MODE_B {
 		$$ = REG_B;
 	}
@@ -2254,7 +2254,10 @@ reg_r:
 	| LBRACK MODE_HL RBRACK {
 		$$ = REG_HL_IND;
 	}
-	| MODE_A {
+;
+
+reg_a:
+	MODE_A {
 		$$ = REG_A;
 	}
 ;

diff --git a/src/asm/section.cpp b/src/asm/section.cpp
@@ -817,7 +817,11 @@ void sect_PCRelByte(Expression &expr, uint32_t pcShift) {
 			offset = sym->getValue() - (pc->getValue() + 1);
 
 		if (offset < -128 || offset > 127) {
-			error("jr target out of reach (expected -129 < %" PRId16 " < 128)\n", offset);
+			error(
+			    "jr target must be between -128 and 127 bytes away, not %" PRId16
+			    "; use jp instead\n",
+			    offset
+			);
 			writebyte(0);
 		} else {
 			writebyte(offset);

diff --git a/src/link/patch.cpp b/src/link/patch.cpp
@@ -461,7 +461,8 @@ static void applyFilePatches(Section &section, Section &dataSection) {
 				error(
 				    patch.src,
 				    patch.lineNo,
-				    "jr target out of reach (expected -129 < %" PRId16 " < 128)",
+				    "jr target must be between -128 and 127 bytes away, not %" PRId16
+				    "; use jp instead\n",
 				    jumpOffset
 				);
 			dataSection.data[offset] = jumpOffset & 0xFF;

diff --git a/test/asm/ff00+c-bad.err b/test/asm/ff00+c-bad.err
@@ -1,5 +1,7 @@
 error: ff00+c-bad.asm(8):
-    Expected constant expression equal to $FF00 for "$ff00+c"
+    Base value must be equal to $FF00 for $FF00+C
 error: ff00+c-bad.asm(9):
-    Expected constant expression equal to $FF00 for "$ff00+c"
-error: Assembly aborted (2 errors)!
+    Expected constant expression: 'xyz' is not constant at assembly time
+error: ff00+c-bad.asm(9):
+    Base value must be equal to $FF00 for $FF00+C
+error: Assembly aborted (3 errors)!
diff --git a/test/asm/invalid-instructions.err b/test/asm/invalid-instructions.err
@@ -1,17 +1,17 @@
 error: invalid-instructions.asm(1):
     Address $10000 is not 16-bit
 error: invalid-instructions.asm(2):
-    LD [HL],[HL] not a valid instruction
+    LD [HL], [HL] is not a valid instruction
 error: invalid-instructions.asm(3):
-    Expected constant expression equal to $FF00 for "$ff00+c"
+    Base value must be equal to $FF00 for $FF00+C
 error: invalid-instructions.asm(4):
-    Destination operand must be A
+    syntax error, unexpected c, expecting hl
 error: invalid-instructions.asm(5):
-    Destination operand must be A
+    syntax error, unexpected bc, expecting hl
 error: invalid-instructions.asm(6):
-    Destination operand must be A
+    syntax error, unexpected number, expecting hl
 error: invalid-instructions.asm(7):
-    Immediate value must be 3-bit
+    Bit number must be between 0 and 7, not 8
 error: invalid-instructions.asm(8):
     Invalid address $40 for RST
 error: Assembly aborted (8 errors)!
diff --git a/test/asm/invalid-jr.err b/test/asm/invalid-jr.err
@@ -1,3 +1,3 @@
 error: invalid-jr.asm(3):
-    jr target out of reach (expected -129 < -258 < 128)
+    jr target must be between -128 and 127 bytes away, not -258; use jp instead
 error: Assembly aborted (1 error)!