Allow empty macro arguments, with a warning

Fixes #739

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);

include/asm/macro.h

@@ -10,6 +10,7 @@
 #define RGBDS_MACRO_H
 
 #include <stdint.h>
+#include <stdbool.h>
 #include <stdlib.h>
 
 #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);

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 */

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;

src/asm/macro.c

@@ -2,6 +2,7 @@
 #include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -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) {

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 {
+		| macroargs T_STRING {
-			$$ = macro_NewArgs();
+			macro_AppendArg(&($$), strdup($2), !lexer_IsRawMode());
-			macro_AppendArg(&($$), strdup($1));
-		}
-		| macroargs T_COMMA T_STRING {
-			macro_AppendArg(&($$), strdup($3));
 		}
 ;
 

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

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,

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,,

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

test/asm/macro-arguments.out

@@ -13,3 +13,34 @@
 \2: <2 +  2>
 \3: <3>
 
+'mac ':
+
+'mac a,,':
+\1: <a>
+\2: <>
+\3: <>
+
+'mac ,,z':
+\1: <>
+\2: <>
+\3: <z>
+
+'mac a,,z':
+\1: <a>
+\2: <>
+\3: <z>
+
+'mac ,a,b,c,':
+\1: <>
+\2: <a>
+\3: <b>
+\4: <c>
+\5: <>
+
+'mac ,,x,,':
+\1: <>
+\2: <>
+\3: <x>
+\4: <>
+\5: <>
+