shithub: rgbds

Download patch

ref: 37089ef940a25e04f263233d5436b2d8b7ef3f70
parent: 4ef27a0d239a9752c176031f1665ffa5f4abbba0
author: ISSOtm <[email protected]>
date: Thu Aug 29 13:03:50 EDT 2019

Improve error stack
The old error stack was fairly obtuse and hard to use for debugging.
This improves it notably by ensuring all line numbers are relative
to the file, and not, say, the macro definition.
This is a breaking change if you were parsing the old stack, but
the change should be painless, and the new stack only brings more info.
The syntax is unchanged for files, macros see their name prefixed
with the file they're defined in and a pair of colors, REPT blocks
simply append a '::REPT~n' to the context they're in, where 'n' is
the number of iterations the REPT has done.
This is especially helpful in macro-heavy code such as rgbds-structs.

--- a/include/asm/fstack.h
+++ b/include/asm/fstack.h
@@ -33,6 +33,8 @@
 	char *pREPTBlock;
 	uint32_t nREPTBlockCount;
 	uint32_t nREPTBlockSize;
+	int32_t nREPTBodyFirstLine;
+	int32_t nREPTBodyLastLine;
 };
 
 extern unsigned int nMaxRecursionDepth;
@@ -43,7 +45,7 @@
 void fstk_Dump(void);
 void fstk_AddIncludePath(char *s);
 uint32_t fstk_RunMacro(char *s);
-void fstk_RunRept(uint32_t count);
+void fstk_RunRept(uint32_t count, int32_t nReptLineNo);
 FILE *fstk_FindFile(char *fname, char **incPathUsed);
 int32_t fstk_GetLine(void);
 
--- a/include/asm/symbol.h
+++ b/include/asm/symbol.h
@@ -74,7 +74,7 @@
 void sym_UseCurrentMacroArgs(void);
 void sym_SetMacroArgID(uint32_t nMacroCount);
 uint32_t sym_isString(char *tzSym);
-void sym_AddMacro(char *tzSym);
+void sym_AddMacro(char *tzSym, int32_t nDefLineNo);
 void sym_Ref(char *tzSym);
 void sym_ShiftCurrentMacroArgs(void);
 void sym_AddString(char *tzSym, char *tzValue);
--- a/src/asm/asmy.y
+++ b/src/asm/asmy.y
@@ -816,15 +816,17 @@
 
 rept		: T_POP_REPT uconst
 		{
+			uint32_t nDefinitionLineNo = nLineNo;
 			copyrept();
-			fstk_RunRept($2);
+			fstk_RunRept($2, nDefinitionLineNo);
 		}
 ;
 
 macrodef	: T_LABEL ':' T_POP_MACRO
 		{
+			int32_t nDefinitionLineNo = nLineNo;
 			copymacro();
-			sym_AddMacro($1);
+			sym_AddMacro($1, nDefinitionLineNo);
 		}
 ;
 
--- a/src/asm/fstack.c
+++ b/src/asm/fstack.c
@@ -42,6 +42,8 @@
 static char *pCurrentREPTBlock;
 static uint32_t nCurrentREPTBlockSize;
 static uint32_t nCurrentREPTBlockCount;
+static int32_t nCurrentREPTBodyFirstLine;
+static int32_t nCurrentREPTBodyLastLine;
 
 uint32_t ulMacroReturnValue;
 
@@ -93,6 +95,8 @@
 		(*ppFileStack)->pREPTBlock = pCurrentREPTBlock;
 		(*ppFileStack)->nREPTBlockSize = nCurrentREPTBlockSize;
 		(*ppFileStack)->nREPTBlockCount = nCurrentREPTBlockCount;
+		(*ppFileStack)->nREPTBodyFirstLine = nCurrentREPTBodyFirstLine;
+		(*ppFileStack)->nREPTBodyLastLine = nCurrentREPTBodyLastLine;
 		break;
 	default:
 		fatalerror("%s: Internal error.", __func__);
@@ -107,6 +111,11 @@
 
 	if (nCurrentStatus == STAT_isREPTBlock) {
 		if (--nCurrentREPTBlockCount) {
+			char *pREPTIterationWritePtr;
+			unsigned long nREPTIterationNo;
+			int nNbCharsWritten;
+			int nNbCharsLeft;
+
 			yy_delete_buffer(CurrentFlexHandle);
 			CurrentFlexHandle =
 				yy_scan_bytes(pCurrentREPTBlock,
@@ -115,6 +124,29 @@
 			sym_UseCurrentMacroArgs();
 			sym_SetMacroArgID(nMacroCount++);
 			sym_UseNewMacroArgs();
+
+			/* Increment REPT count in file path */
+			pREPTIterationWritePtr =
+				strrchr(tzCurrentFileName, '~') + 1;
+			nREPTIterationNo =
+				strtoul(pREPTIterationWritePtr, NULL, 10);
+			nNbCharsLeft = sizeof(tzCurrentFileName)
+				- (pREPTIterationWritePtr - tzCurrentFileName);
+			nNbCharsWritten = snprintf(pREPTIterationWritePtr,
+						   nNbCharsLeft, "%lu",
+						   nREPTIterationNo + 1);
+			if (nNbCharsWritten >= nNbCharsLeft) {
+				/*
+				 * The string is probably corrupted somehow,
+				 * revert the change to avoid a bad error
+				 * output.
+				 */
+				sprintf(pREPTIterationWritePtr, "%lu",
+					nREPTIterationNo);
+				fatalerror("Cannot write REPT count to file path");
+			}
+
+			nLineNo = nCurrentREPTBodyFirstLine;
 			return 0;
 		}
 	}
@@ -156,6 +188,9 @@
 		pCurrentREPTBlock = pLastFile->pREPTBlock;
 		nCurrentREPTBlockSize = pLastFile->nREPTBlockSize;
 		nCurrentREPTBlockCount = pLastFile->nREPTBlockCount;
+		nCurrentREPTBodyFirstLine = pLastFile->nREPTBodyFirstLine;
+		/* + 1 to account for the `ENDR` line */
+		nLineNo = pLastFile->nREPTBodyLastLine + 1;
 		break;
 	default:
 		fatalerror("%s: Internal error.", __func__);
@@ -322,6 +357,7 @@
 uint32_t fstk_RunMacro(char *s)
 {
 	struct sSymbol *sym = sym_FindMacro(s);
+	int nPrintedChars;
 
 	if (sym == NULL || sym->pMacro == NULL)
 		return 0;
@@ -328,10 +364,16 @@
 
 	pushcontext();
 	sym_SetMacroArgID(nMacroCount++);
-	nLineNo = -1;
+	/* Minus 1 because there is a newline at the beginning of the buffer */
+	nLineNo = sym->nFileLine - 1;
 	sym_UseNewMacroArgs();
 	nCurrentStatus = STAT_isMacro;
-	strcpy(tzCurrentFileName, s);
+	nPrintedChars = snprintf(tzCurrentFileName, _MAX_PATH + 1,
+				 "%s::%s", sym->tzFileName, s);
+	if (nPrintedChars > _MAX_PATH) {
+		popcontext();
+		fatalerror("File name + macro name is too large to fit into buffer");
+	}
 
 	pCurrentMacro = sym;
 	CurrentFlexHandle = yy_scan_bytes(pCurrentMacro->pMacro,
@@ -387,9 +429,14 @@
 /*
  * Set up a repeat block for parsing
  */
-void fstk_RunRept(uint32_t count)
+void fstk_RunRept(uint32_t count, int32_t nReptLineNo)
 {
 	if (count) {
+		static const char *tzReptStr = "::REPT~1";
+
+		/* For error printing to make sense, fake nLineNo */
+		nCurrentREPTBodyLastLine = nLineNo;
+		nLineNo = nReptLineNo;
 		sym_UseCurrentMacroArgs();
 		pushcontext();
 		sym_SetMacroArgID(nMacroCount++);
@@ -398,6 +445,14 @@
 		nCurrentStatus = STAT_isREPTBlock;
 		nCurrentREPTBlockSize = ulNewMacroSize;
 		pCurrentREPTBlock = tzNewMacro;
+		nCurrentREPTBodyFirstLine = nReptLineNo + 1;
+		nLineNo = nReptLineNo;
+
+		if (strlen(tzCurrentFileName) + strlen(tzReptStr) > _MAX_PATH)
+			fatalerror("Cannot append \"%s\" to file path",
+				   tzReptStr);
+		strcat(tzCurrentFileName, tzReptStr);
+
 		CurrentFlexHandle =
 			yy_scan_bytes(pCurrentREPTBlock, nCurrentREPTBlockSize);
 		yy_switch_to_buffer(CurrentFlexHandle);
--- a/src/asm/symbol.c
+++ b/src/asm/symbol.c
@@ -705,7 +705,7 @@
 /*
  * Add a macro definition
  */
-void sym_AddMacro(char *tzSym)
+void sym_AddMacro(char *tzSym, int32_t nDefLineNo)
 {
 	struct sSymbol *nsym = createNonrelocSymbol(tzSym);
 
@@ -715,6 +715,11 @@
 		nsym->ulMacroSize = ulNewMacroSize;
 		nsym->pMacro = tzNewMacro;
 		updateSymbolFilename(nsym);
+		/*
+		 * The symbol is created at the line after the `endm`,
+		 * override this with the actual definition line
+		 */
+		nsym->nFileLine = nDefLineNo;
 	}
 }
 
--- a/test/asm/label-macro-arg.out
+++ b/test/asm/label-macro-arg.out
@@ -1,4 +1,4 @@
-ERROR: label-macro-arg.asm(45) -> test_char(2):
+ERROR: label-macro-arg.asm(45) -> label-macro-arg.asm::test_char(31):
     Macro 'something' not defined
 $5
 $6
--- a/test/asm/label-macro-arg.out.pipe
+++ b/test/asm/label-macro-arg.out.pipe
@@ -1,4 +1,4 @@
-ERROR: -(45) -> test_char(2):
+ERROR: -(45) -> -::test_char(31):
     Macro 'something' not defined
 $5
 $6
--- a/test/asm/label-redefinition.out
+++ b/test/asm/label-redefinition.out
@@ -1,3 +1,3 @@
 ERROR: label-redefinition.asm(7):
-    'Sym' already defined in m(6)
+    'Sym' already defined in label-redefinition.asm::m(6)
 error: Assembly aborted (1 errors)!
--- a/test/asm/label-redefinition.out.pipe
+++ b/test/asm/label-redefinition.out.pipe
@@ -1,3 +1,3 @@
 ERROR: -(7):
-    'Sym' already defined in m(6)
+    'Sym' already defined in -::m(6)
 error: Assembly aborted (1 errors)!
--- a/test/asm/macro-recursion.out
+++ b/test/asm/macro-recursion.out
@@ -1,2 +1,2 @@
-ERROR: macro-recursion.asm(4) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1):
+ERROR: macro-recursion.asm(4) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2) -> macro-recursion.asm::recurse(2):
     Recursion limit (64) exceeded
--- a/test/asm/macro-recursion.out.pipe
+++ b/test/asm/macro-recursion.out.pipe
@@ -1,2 +1,2 @@
-ERROR: -(4) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1) -> recurse(1):
+ERROR: -(4) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2) -> -::recurse(2):
     Recursion limit (64) exceeded
--- a/test/asm/multiple-charmaps.out
+++ b/test/asm/multiple-charmaps.out
@@ -1,10 +1,10 @@
 warning: multiple-charmaps.asm(75):
     Using 'charmap' within a section when the current charmap is 'main' is deprecated
-ERROR: multiple-charmaps.asm(100) -> new_(6):
+ERROR: multiple-charmaps.asm(100) -> multiple-charmaps.asm::new_(7):
     Charmap 'map1' already exists
-ERROR: multiple-charmaps.asm(102) -> set_(2):
+ERROR: multiple-charmaps.asm(102) -> multiple-charmaps.asm::set_(13):
     Charmap 'map5' doesn't exist
-ERROR: multiple-charmaps.asm(104) -> pop_(2):
+ERROR: multiple-charmaps.asm(104) -> multiple-charmaps.asm::pop_(23):
     No entries in the charmap stack
 main charmap
 $0
--- a/test/asm/multiple-charmaps.out.pipe
+++ b/test/asm/multiple-charmaps.out.pipe
@@ -1,10 +1,10 @@
 warning: -(75):
     Using 'charmap' within a section when the current charmap is 'main' is deprecated
-ERROR: -(100) -> new_(6):
+ERROR: -(100) -> -::new_(7):
     Charmap 'map1' already exists
-ERROR: -(102) -> set_(2):
+ERROR: -(102) -> -::set_(13):
     Charmap 'map5' doesn't exist
-ERROR: -(104) -> pop_(2):
+ERROR: -(104) -> -::pop_(23):
     No entries in the charmap stack
 main charmap
 $0
--- a/test/asm/strsub.out
+++ b/test/asm/strsub.out
@@ -1,18 +1,18 @@
-warning: strsub.asm(13) -> xstrsub(1):
+warning: strsub.asm(13) -> strsub.asm::xstrsub(4):
     STRSUB: Length too big: 32
-warning: strsub.asm(14) -> xstrsub(1):
+warning: strsub.asm(14) -> strsub.asm::xstrsub(4):
     STRSUB: Length too big: 300
-warning: strsub.asm(15) -> xstrsub(1):
+warning: strsub.asm(15) -> strsub.asm::xstrsub(4):
     STRSUB: Position starts at 1
-warning: strsub.asm(15) -> xstrsub(1):
+warning: strsub.asm(15) -> strsub.asm::xstrsub(4):
     STRSUB: Length too big: 300
-warning: strsub.asm(16) -> xstrsub(1):
+warning: strsub.asm(16) -> strsub.asm::xstrsub(4):
     STRSUB: Position 4 is past the end of the string
-warning: strsub.asm(17) -> xstrsub(1):
+warning: strsub.asm(17) -> strsub.asm::xstrsub(4):
     STRSUB: Position 4 is past the end of the string
-warning: strsub.asm(17) -> xstrsub(1):
+warning: strsub.asm(17) -> strsub.asm::xstrsub(4):
     STRSUB: Length too big: 1
-warning: strsub.asm(20) -> xstrsub(1):
+warning: strsub.asm(20) -> strsub.asm::xstrsub(4):
     STRSUB: Length too big: 10
 A
 B
--- a/test/asm/strsub.out.pipe
+++ b/test/asm/strsub.out.pipe
@@ -1,18 +1,18 @@
-warning: -(13) -> xstrsub(1):
+warning: -(13) -> -::xstrsub(4):
     STRSUB: Length too big: 32
-warning: -(14) -> xstrsub(1):
+warning: -(14) -> -::xstrsub(4):
     STRSUB: Length too big: 300
-warning: -(15) -> xstrsub(1):
+warning: -(15) -> -::xstrsub(4):
     STRSUB: Position starts at 1
-warning: -(15) -> xstrsub(1):
+warning: -(15) -> -::xstrsub(4):
     STRSUB: Length too big: 300
-warning: -(16) -> xstrsub(1):
+warning: -(16) -> -::xstrsub(4):
     STRSUB: Position 4 is past the end of the string
-warning: -(17) -> xstrsub(1):
+warning: -(17) -> -::xstrsub(4):
     STRSUB: Position 4 is past the end of the string
-warning: -(17) -> xstrsub(1):
+warning: -(17) -> -::xstrsub(4):
     STRSUB: Length too big: 1
-warning: -(20) -> xstrsub(1):
+warning: -(20) -> -::xstrsub(4):
     STRSUB: Length too big: 10
 A
 B