shithub: rgbds

Info  •  Files  •  Log  •  Branches

Resolve some TODO comments

- `out_PushSection` should not set `currentSection` to NULL because
  PUSHS, PUSHC, and PUSHO consistently keep the current section,
  charmap, and options, even though the stack has been pushed.

- `Callback__FILE__` does not need to assert that `fileName` is not
  empty because `__FILE__`'s value is quoted, and can safely be empty.

- `YY_FATAL_ERROR` and `YYLMAX` are not needed since the lexer is
  not generated with flex.

--- a/include/asm/lexer.h

+++ b/include/asm/lexer.h

@@ -53,7 +53,7 @@

  * `path` is referenced, but not held onto..!

  */

 struct LexerState *lexer_OpenFile(char const *path);

-struct LexerState *lexer_OpenFileView(char *buf, size_t size, uint32_t lineNo);

+struct LexerState *lexer_OpenFileView(char const *path, char *buf, size_t size, uint32_t lineNo);

 void lexer_RestartRept(uint32_t lineNo);

 void lexer_DeleteState(struct LexerState *state);

 void lexer_Init(void);

--- a/include/asm/main.h

+++ b/include/asm/main.h

@@ -26,12 +26,4 @@

 extern bool failedOnMissingInclude;

 extern bool generatePhonyDeps;

-/* TODO: are these really needed? */

-#define YY_FATAL_ERROR fatalerror

-

-#ifdef YYLMAX

-#undef YYLMAX

-#endif

-#define YYLMAX 65536

-

 #endif /* RGBDS_MAIN_H */

--- a/include/asm/symbol.h

+++ b/include/asm/symbol.h

@@ -45,13 +45,13 @@

 		/* If sym_IsNumeric */

 		int32_t value;

 		int32_t (*numCallback)(void);

-		/* For SYM_MACRO */

+		/* For SYM_MACRO and SYM_EQUS; TODO: have separate fields */

 		struct {

 			size_t macroSize;

 			char *macro;

 		};

-		/* For SYM_EQUS, TODO: separate "base" fields from SYM_MACRO */

-		char const *(*strCallback)(void); /* For SYM_EQUS */

+		/* For SYM_EQUS */

+		char const *(*strCallback)(void);

 	};

 	uint32_t ID; /* ID of the symbol in the object file (-1 if none) */

--- a/src/asm/fstack.c

+++ b/src/asm/fstack.c

@@ -417,7 +417,7 @@

 	memcpy(dest, macro->name, macroNameLen + 1);

 	newContext((struct FileStackNode *)fileInfo);

-	contextStack->lexerState = lexer_OpenFileView(macro->macro, macro->macroSize,

+	contextStack->lexerState = lexer_OpenFileView("MACRO", macro->macro, macro->macroSize,

 						      macro->fileLine);

 	if (!contextStack->lexerState)

 		fatalerror("Failed to set up lexer for macro invocation\n");

@@ -451,7 +451,7 @@

 	/* Correct our line number, which currently points to the `ENDR` line */

 	contextStack->fileInfo->lineNo = reptLineNo;

-	contextStack->lexerState = lexer_OpenFileView(body, size, reptLineNo);

+	contextStack->lexerState = lexer_OpenFileView("REPT", body, size, reptLineNo);

 	if (!contextStack->lexerState)

 		fatalerror("Failed to set up lexer for REPT block\n");

 	lexer_SetStateAtEOL(contextStack->lexerState);

--- a/src/asm/lexer.c

+++ b/src/asm/lexer.c

@@ -304,6 +304,8 @@

 }

 #define LEXER_BUF_SIZE 42 /* TODO: determine a sane value for this */

+/* The buffer needs to be large enough for the maximum `peekInternal` lookahead distance */

+static_assert(LEXER_BUF_SIZE > 1, "Lexer buffer size is too small");

 /* This caps the size of buffer reads, and according to POSIX, passing more than SSIZE_MAX is UB */

 static_assert(LEXER_BUF_SIZE <= SSIZE_MAX, "Lexer buffer size is too large");

@@ -531,7 +533,7 @@

 	return state;

 }

-struct LexerState *lexer_OpenFileView(char *buf, size_t size, uint32_t lineNo)

+struct LexerState *lexer_OpenFileView(char const *path, char *buf, size_t size, uint32_t lineNo)

 {

 	dbgPrint("Opening view on buffer \"%.*s\"[...]\n", size < 16 ? (int)size : 16, buf);

@@ -541,8 +543,8 @@

 		error("Failed to allocate memory for lexer state: %s\n", strerror(errno));

 		return NULL;

 	}

-	// TODO: init `path`

+	state->path = path; /* Used to report read errors in `peekInternal` */

 	state->isFile = false;

 	state->isMmapped = true; /* It's not *really* mmap()ed, but it behaves the same */

 	state->ptr = buf;

--- a/src/asm/section.c

+++ b/src/asm/section.c

@@ -902,7 +902,6 @@

 	sect->offset = curOffset;

 	sect->next = sectionStack;

 	sectionStack = sect;

-	/* TODO: maybe set current section to NULL? */

 }

 void out_PopSection(void)

--- a/src/asm/symbol.c

+++ b/src/asm/symbol.c

@@ -96,8 +96,6 @@

 	char const *fileName = fstk_GetFileName();

 	size_t j = 1;

-	/* TODO: is there a way for a file name to be empty? */

-	assert(fileName[0]);

 	/* The assertion above ensures the loop runs at least once */

 	for (size_t i = 0; fileName[i]; i++, j++) {

 		/* Account for the extra backslash inserted below */

@@ -175,7 +173,7 @@

 	/* If the old node was referenced, ensure the new one is */

 	if (oldSrc && oldSrc->referenced && oldSrc->ID != (uint32_t)-1)

 		out_RegisterNode(sym->src);

-	/* TODO: unref the old node, and use `out_ReplaceNode` instead if deleting it */

+	/* TODO: unref the old node, and use `out_ReplaceNode` instead of deleting it */

 }

 /*

@@ -226,7 +224,6 @@

 		fatalerror("No memory for string equate: %s\n", strerror(errno));

 	sym->type = SYM_EQUS;

-	/* TODO: use other fields */

 	sym->macro = string;

 	sym->macroSize = strlen(string);

 }

--- a/src/asm/warning.c

+++ b/src/asm/warning.c

@@ -152,8 +152,7 @@

 				errx(1, "Cannot make meta warning \"%s\" into an error",

 				     flag);

-			uint8_t const *ptr =

-					metaWarningCommands[id - NB_WARNINGS];

+			uint8_t const *ptr = metaWarningCommands[id - NB_WARNINGS];

 			for (;;) {

 				if (*ptr == META_WARNING_DONE)

@@ -194,7 +193,7 @@

 	bool isNegation = !strncmp(flag, "no-", strlen("no-")) && !setError;

 	char const *rootFlag = isNegation ? flag + strlen("no-") : flag;

 	enum WarningState state = setError ? WARNING_ERROR :

-			isNegation ? WARNING_DISABLED : WARNING_ENABLED;

+				  isNegation ? WARNING_DISABLED : WARNING_ENABLED;

 	/* Try to match the flag against a "normal" flag */

 	for (enum WarningID id = 0; id < NB_WARNINGS; id++) {

--- a/src/link/object.c

+++ b/src/link/object.c

@@ -48,6 +48,7 @@

 	do { \

 		FILE *tmpFile = file; \

 		type tmpVal = func(tmpFile); \

+		/* TODO: maybe mark the condition as `unlikely`; how to do that portably? */ \

 		if (tmpVal == (errval)) { \

 			errx(1, __VA_ARGS__, feof(tmpFile) \

 						? "Unexpected end of file" \

@@ -86,7 +87,6 @@

 /**

  * Helper macro for reading longs from a file, and errors out if it fails to.

  * Not as a function to avoid overhead in the general case.

- * TODO: maybe mark the condition as `unlikely`; how to do that portably?

  * @param var The variable to stash the number into

  * @param file The file to read from. Its position will be advanced

  * @param ... A format string and related arguments; note that an extra string

@@ -101,7 +101,6 @@

  * Helper macro for reading bytes from a file, and errors out if it fails to.

  * Differs from `tryGetc` in that the backing function is fgetc(1).

  * Not as a function to avoid overhead in the general case.

- * TODO: maybe mark the condition as `unlikely`; how to do that portably?

  * @param var The variable to stash the number into

  * @param file The file to read from. Its position will be advanced

  * @param ... A format string and related arguments; note that an extra string

@@ -114,7 +113,6 @@

  * Helper macro for reading bytes from a file, and errors out if it fails to.

  * Differs from `tryGetc` in that the backing function is fgetc(1).

  * Not as a function to avoid overhead in the general case.

- * TODO: maybe mark the condition as `unlikely`; how to do that portably?

  * @param var The variable to stash the number into

  * @param file The file to read from. Its position will be advanced

  * @param ... A format string and related arguments; note that an extra string

@@ -163,7 +161,6 @@

 /**

  * Helper macro for reading bytes from a file, and errors out if it fails to.

  * Not as a function to avoid overhead in the general case.

- * TODO: maybe mark the condition as `unlikely`; how to do that portably?

  * @param var The variable to stash the string into

  * @param file The file to read from. Its position will be advanced

  * @param ... A format string and related arguments; note that an extra string