diff --git a/include/asm/lexer.h b/include/asm/lexer.h index 73057d88..23513105 100644 --- a/include/asm/lexer.h +++ b/include/asm/lexer.h @@ -67,6 +67,7 @@ enum LexerMode { }; void lexer_SetMode(enum LexerMode mode); +bool lexer_IsRawMode(void); void lexer_ToggleStringExpansion(bool enable); uint32_t lexer_GetIFDepth(void); diff --git a/include/asm/macro.h b/include/asm/macro.h index df48ebf6..b3b06f84 100644 --- a/include/asm/macro.h +++ b/include/asm/macro.h @@ -10,6 +10,7 @@ #define RGBDS_MACRO_H #include +#include #include #include "asm/warning.h" @@ -20,7 +21,7 @@ struct MacroArgs; struct MacroArgs *macro_GetCurrentArgs(void); struct MacroArgs *macro_NewArgs(void); -void macro_AppendArg(struct MacroArgs **args, char *s); +void macro_AppendArg(struct MacroArgs **args, char *s, bool isLastArg); void macro_UseNewArgs(struct MacroArgs *args); void macro_FreeArgs(struct MacroArgs *args); char const *macro_GetArg(uint32_t i); diff --git a/include/asm/warning.h b/include/asm/warning.h index 98d273d4..26539286 100644 --- a/include/asm/warning.h +++ b/include/asm/warning.h @@ -20,6 +20,7 @@ enum WarningID { WARNING_DIV, /* Division undefined behavior */ WARNING_EMPTY_DATA_DIRECTIVE, /* `db`, `dw` or `dl` directive without data in ROM */ WARNING_EMPTY_ENTRY, /* Empty entry in `db`, `dw` or `dl` */ + WARNING_EMPTY_MACRO_ARG, /* Empty macro argument */ WARNING_EMPTY_STRRPL, /* Empty second argument in `STRRPL` */ WARNING_LARGE_CONSTANT, /* Constants too large */ WARNING_LONG_STR, /* String too long for internal buffers */ diff --git a/src/asm/lexer.c b/src/asm/lexer.c index 5f74c0eb..e996cbbe 100644 --- a/src/asm/lexer.c +++ b/src/asm/lexer.c @@ -360,6 +360,7 @@ struct LexerState { bool disableMacroArgs; bool disableInterpolation; size_t macroArgScanDistance; /* Max distance already scanned for macro args */ + bool injectNewline; /* Whether to inject a newline at EOF */ bool expandStrings; struct Expansion *expansions; size_t expansionOfs; /* Offset into the current top-level expansion (negative = before) */ @@ -381,6 +382,7 @@ static void initState(struct LexerState *state) state->disableMacroArgs = false; state->disableInterpolation = false; state->macroArgScanDistance = 0; + state->injectNewline = false; state->expandStrings = true; state->expansions = NULL; state->expansionOfs = 0; @@ -638,6 +640,11 @@ void lexer_SetMode(enum LexerMode mode) lexerState->mode = mode; } +bool lexer_IsRawMode(void) +{ + return lexerState->mode == LEXER_RAW; +} + void lexer_ToggleStringExpansion(bool enable) { lexerState->expandStrings = enable; @@ -2047,6 +2054,10 @@ static int yylex_NORMAL(void) return T_NEWLINE; case EOF: + if (lexerState->injectNewline) { + lexerState->injectNewline = false; + return T_NEWLINE; + } return T_EOF; /* Handle escapes */ @@ -2140,6 +2151,19 @@ static int yylex_RAW(void) case '\r': case '\n': case EOF: + // Returning T_COMMAs to the parser would mean that two consecutive commas + // (i.e. an empty argument) need to return two different tokens (T_STRING + // then T_COMMA) without advancing the read. To avoid this, commas in raw + // mode end the current macro argument but are not tokenized themselves. + if (c == ',') + shiftChars(1); + else + lexer_SetMode(LEXER_NORMAL); + // If a macro is invoked on the last line of a file, with no blank + // line afterwards, returning EOF afterwards will cause Bison to + // stop parsing, despite the lexer being ready to output more. + if (c == EOF) + lexerState->injectNewline = true; /* Trim right whitespace */ while (i && isWhitespace(yylval.tzString[i - 1])) i--; @@ -2147,20 +2171,6 @@ static int yylex_RAW(void) i--; warning(WARNING_LONG_STR, "Macro argument too long\n"); } - /* Empty macro args break their expansion, so prevent that */ - if (i == 0) { - // If at EOF, don't shift a non-existent char. - // However, don't return EOF, as this might cause a bug... - // If a macro is invoked on the last line of a file, with no blank - // line afterwards, returning EOF here will cause Bison to stop - // parsing, despite the lexer being ready to output more. - if (c == EOF) - return T_NEWLINE; - shiftChars(1); - if (c == '\r' && peek(0) == '\n') - shiftChars(1); - return c == ',' ? T_COMMA : T_NEWLINE; - } yylval.tzString[i] = '\0'; dbgPrint("Read raw string \"%s\"\n", yylval.tzString); return T_STRING; diff --git a/src/asm/macro.c b/src/asm/macro.c index f47d55d8..23a12f67 100644 --- a/src/asm/macro.c +++ b/src/asm/macro.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -58,9 +59,15 @@ struct MacroArgs *macro_NewArgs(void) return args; } -void macro_AppendArg(struct MacroArgs **argPtr, char *s) +void macro_AppendArg(struct MacroArgs **argPtr, char *s, bool isLastArg) { #define macArgs (*argPtr) + if (s[0] == '\0') { + /* Zero arguments are parsed as a spurious empty argument; do not append it */ + if (isLastArg && !macArgs->nbArgs) + return; + warning(WARNING_EMPTY_MACRO_ARG, "Empty macro argument\n"); + } if (macArgs->nbArgs == MAXMACROARGS) error("A maximum of " EXPAND_AND_STR(MAXMACROARGS) " arguments is allowed\n"); if (macArgs->nbArgs >= macArgs->capacity) { diff --git a/src/asm/parser.y b/src/asm/parser.y index 6aadafad..841cb55a 100644 --- a/src/asm/parser.y +++ b/src/asm/parser.y @@ -730,7 +730,6 @@ label : %empty macro : T_ID { lexer_SetMode(LEXER_RAW); } macroargs { - lexer_SetMode(LEXER_NORMAL); fstk_RunMacro($1, $3); } ; @@ -738,12 +737,8 @@ macro : T_ID { macroargs : %empty { $$ = macro_NewArgs(); } - | T_STRING { - $$ = macro_NewArgs(); - macro_AppendArg(&($$), strdup($1)); - } - | macroargs T_COMMA T_STRING { - macro_AppendArg(&($$), strdup($3)); + | macroargs T_STRING { + macro_AppendArg(&($$), strdup($2), !lexer_IsRawMode()); } ; diff --git a/src/asm/rgbasm.1 b/src/asm/rgbasm.1 index ca173c2a..fe4a776d 100644 --- a/src/asm/rgbasm.1 +++ b/src/asm/rgbasm.1 @@ -215,6 +215,10 @@ Warn when an empty entry is encountered in a list. This warning is enabled by .Fl Wextra . +.It Fl Wempty-macro-arg +Warn when a macro argument is empty. +This warning is enabled by +.Fl Wextra . .It Fl Wempty-strrpl Warn when .Fn STRRPL diff --git a/src/asm/warning.c b/src/asm/warning.c index c7ceda5b..2c834d60 100644 --- a/src/asm/warning.c +++ b/src/asm/warning.c @@ -35,6 +35,7 @@ static enum WarningState const defaultWarnings[NB_WARNINGS] = { [WARNING_DIV] = WARNING_DISABLED, [WARNING_EMPTY_DATA_DIRECTIVE] = WARNING_DISABLED, [WARNING_EMPTY_ENTRY] = WARNING_DISABLED, + [WARNING_EMPTY_MACRO_ARG] = WARNING_DISABLED, [WARNING_EMPTY_STRRPL] = WARNING_DISABLED, [WARNING_LARGE_CONSTANT] = WARNING_DISABLED, [WARNING_LONG_STR] = WARNING_DISABLED, @@ -77,6 +78,7 @@ static char const *warningFlags[NB_WARNINGS_ALL] = { "div", "empty-data-directive", "empty-entry", + "empty-macro-arg", "empty-strrpl", "large-constant", "long-string", @@ -112,6 +114,7 @@ static uint8_t const _wallCommands[] = { /* Warnings that are less likely to indicate an error */ static uint8_t const _wextraCommands[] = { WARNING_EMPTY_ENTRY, + WARNING_EMPTY_MACRO_ARG, WARNING_MACRO_SHIFT, WARNING_NESTED_COMMENT, META_WARNING_DONE @@ -123,6 +126,7 @@ static uint8_t const _weverythingCommands[] = { WARNING_DIV, WARNING_EMPTY_DATA_DIRECTIVE, WARNING_EMPTY_ENTRY, + WARNING_EMPTY_MACRO_ARG, WARNING_EMPTY_STRRPL, WARNING_LARGE_CONSTANT, WARNING_LONG_STR, diff --git a/test/asm/macro-arguments.asm b/test/asm/macro-arguments.asm index 9ca1f946..fe11ad57 100644 --- a/test/asm/macro-arguments.asm +++ b/test/asm/macro-arguments.asm @@ -14,3 +14,10 @@ ENDM c, d mac 1, 2 + /* another ; ; comment */ 2, 3 + + mac + mac a,, + mac ,,z + mac a,,z + mac ,a,b,c, + mac ,,x,, diff --git a/test/asm/macro-arguments.err b/test/asm/macro-arguments.err index e69de29b..162e248a 100644 --- a/test/asm/macro-arguments.err +++ b/test/asm/macro-arguments.err @@ -0,0 +1,22 @@ +warning: macro-arguments.asm(19): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(19): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(20): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(20): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(21): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(22): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(22): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(23): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(23): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(23): [-Wempty-macro-arg] + Empty macro argument +warning: macro-arguments.asm(23): [-Wempty-macro-arg] + Empty macro argument diff --git a/test/asm/macro-arguments.out b/test/asm/macro-arguments.out index e25d20c3..3c29ba89 100644 --- a/test/asm/macro-arguments.out +++ b/test/asm/macro-arguments.out @@ -13,3 +13,34 @@ \2: <2 + 2> \3: <3> +'mac ': + +'mac a,,': +\1: +\2: <> +\3: <> + +'mac ,,z': +\1: <> +\2: <> +\3: + +'mac a,,z': +\1: +\2: <> +\3: + +'mac ,a,b,c,': +\1: <> +\2: +\3: +\4: +\5: <> + +'mac ,,x,,': +\1: <> +\2: <> +\3: +\4: <> +\5: <> +