From 3fdf01c0f53297e4e8d2d7a19040abc594d75abf Mon Sep 17 00:00:00 2001 From: Rangi Date: Mon, 26 Apr 2021 15:52:30 -0400 Subject: [PATCH] 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. --- include/asm/lexer.h | 2 +- include/asm/main.h | 8 -------- include/asm/symbol.h | 6 +++--- src/asm/fstack.c | 4 ++-- src/asm/lexer.c | 6 ++++-- src/asm/section.c | 1 - src/asm/symbol.c | 5 +---- src/asm/warning.c | 5 ++--- src/link/object.c | 5 +---- 9 files changed, 14 insertions(+), 28 deletions(-) diff --git a/include/asm/lexer.h b/include/asm/lexer.h index 706e92e9..0ad6de48 100644 --- a/include/asm/lexer.h +++ b/include/asm/lexer.h @@ -53,7 +53,7 @@ static inline void lexer_SetGfxDigits(char const digits[4]) * `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); diff --git a/include/asm/main.h b/include/asm/main.h index 173427bf..b97080f6 100644 --- a/include/asm/main.h +++ b/include/asm/main.h @@ -26,12 +26,4 @@ extern bool generatedMissingIncludes; 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 */ diff --git a/include/asm/symbol.h b/include/asm/symbol.h index ed49fc78..b52b0fb6 100644 --- a/include/asm/symbol.h +++ b/include/asm/symbol.h @@ -45,13 +45,13 @@ struct Symbol { /* 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) */ diff --git a/src/asm/fstack.c b/src/asm/fstack.c index 02836d8e..75b2f2f3 100644 --- a/src/asm/fstack.c +++ b/src/asm/fstack.c @@ -417,7 +417,7 @@ void fstk_RunMacro(char const *macroName, struct MacroArgs *args) 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 @@ static bool newReptContext(int32_t reptLineNo, char *body, size_t size) /* 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); diff --git a/src/asm/lexer.c b/src/asm/lexer.c index 7a84b112..673778cb 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -304,6 +304,8 @@ static bool isWhitespace(int c) } #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 @@ struct LexerState *lexer_OpenFile(char const *path) 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 @@ struct LexerState *lexer_OpenFileView(char *buf, size_t size, uint32_t lineNo) 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; diff --git a/src/asm/section.c b/src/asm/section.c index dfd5269c..b9fc281c 100644 --- a/src/asm/section.c +++ b/src/asm/section.c @@ -902,7 +902,6 @@ void out_PushSection(void) sect->offset = curOffset; sect->next = sectionStack; sectionStack = sect; - /* TODO: maybe set current section to NULL? */ } void out_PopSection(void) diff --git a/src/asm/symbol.c b/src/asm/symbol.c index dac156da..7c3f894c 100644 --- a/src/asm/symbol.c +++ b/src/asm/symbol.c @@ -96,8 +96,6 @@ static char const *Callback__FILE__(void) 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 @@ static void updateSymbolFilename(struct Symbol *sym) /* 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 @@ static void assignStringSymbol(struct Symbol *sym, char const *value) 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); } diff --git a/src/asm/warning.c b/src/asm/warning.c index 4e709b2c..dc233866 100644 --- a/src/asm/warning.c +++ b/src/asm/warning.c @@ -152,8 +152,7 @@ void processWarningFlag(char const *flag) 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 @@ void processWarningFlag(char const *flag) 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++) { diff --git a/src/link/object.c b/src/link/object.c index 9e8cfe6c..26d0de27 100644 --- a/src/link/object.c +++ b/src/link/object.c @@ -48,6 +48,7 @@ static struct Assertion *assertions; 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 @@ static int64_t readlong(FILE *file) /** * 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 @@ static int64_t readlong(FILE *file) * 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 @@ static int64_t readlong(FILE *file) * 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 @@ static char *readstr(FILE *file) /** * 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