From 462fd7539cf4a24897571738e0b453e65bdfc8c1 Mon Sep 17 00:00:00 2001
From: ISSOtm <eldredhabert0@gmail.com>
Date: Wed, 9 Dec 2020 10:44:39 +0100
Subject: [PATCH] Prohibit nested macros

After discussion (starting there:
https://github.com/gbdev/rgbds/pull/594#issuecomment-706437458
), it was decided that plain nested macros should not be
allowed.
Since #590 is fixed, EQUS can be used as a workaround;
multiline strings (#589) will make that easier on the
user when implemented.
Fixes #588, supersedes and closes #594.
Additionally, closes #388.
---
 src/asm/lexer.c              | 47 +++++++++---------------------------
 src/asm/rgbasm.5             | 20 +++++++++++++++
 test/asm/nested-macrodef.asm | 27 +++++++++++++++++++++
 test/asm/nested-macrodef.err |  5 ++++
 test/asm/nested-macrodef.out |  3 +++
 5 files changed, 67 insertions(+), 35 deletions(-)
 create mode 100644 test/asm/nested-macrodef.asm
 create mode 100644 test/asm/nested-macrodef.err
 create mode 100644 test/asm/nested-macrodef.out

diff --git a/src/asm/lexer.c b/src/asm/lexer.c
index 779618bb..a3ecdebc 100644
--- a/src/asm/lexer.c
+++ b/src/asm/lexer.c
@@ -2045,7 +2045,6 @@ finish:
 void lexer_CaptureMacroBody(char **capture, size_t *size)
 {
 	char *captureStart = startCapture();
-	unsigned int level = 0;
 	int c = peek(0);
 
 	/* If the file is `mmap`ed, we need not to unmap it to keep access to the macro */
@@ -2082,41 +2081,19 @@ void lexer_CaptureMacroBody(char **capture, size_t *size)
 		} while (isWhitespace(c));
 		/* Now, try to match either `REPT` or `ENDR` as a **whole** identifier */
 		if (startsIdentifier(c)) {
-			switch (readIdentifier(c)) {
-			case T_ID:
-				/* We have an initial label, look for a single colon */
+			if (readIdentifier(c) == T_POP_ENDM) {
+				/* Read (but don't capture) until EOL or EOF */
+				lexerState->capturing = false;
 				do {
-					c = nextChar();
-				} while (isWhitespace(c));
-				if (c != ':') /* If not a colon, give up */
-					break;
-				/* And finally, a `MACRO` token */
-				do {
-					c = nextChar();
-				} while (isWhitespace(c));
-				if (!startsIdentifier(c))
-					break;
-				if (readIdentifier(c) != T_POP_MACRO)
-					break;
-				level++;
-				break;
-
-			case T_POP_ENDM:
-				if (!level) {
-					/* Read (but don't capture) until EOL or EOF */
-					lexerState->capturing = false;
-					do {
-						c = peek(0);
-						if (c == EOF || c == '\r' || c == '\n')
-							break;
-						shiftChars(1);
-					} while (c != EOF && c != '\r' && c != '\n');
-					/* Handle Windows CRLF */
-					if (c == '\r' && peek(1) == '\n')
-						shiftChars(1);
-					goto finish;
-				}
-				level--;
+					c = peek(0);
+					if (c == EOF || c == '\r' || c == '\n')
+						break;
+					shiftChars(1);
+				} while (c != EOF && c != '\r' && c != '\n');
+				/* Handle Windows CRLF */
+				if (c == '\r' && peek(1) == '\n')
+					shiftChars(1);
+				goto finish;
 			}
 		}
 		lexerState->lineNo++;
diff --git a/src/asm/rgbasm.5 b/src/asm/rgbasm.5
index aec95c77..dc6de113 100644
--- a/src/asm/rgbasm.5
+++ b/src/asm/rgbasm.5
@@ -923,6 +923,26 @@ Note that a single colon
 .Ql \&:
 following the macro's name is required.
 Macros can't be exported or imported.
+.Pp
+Plainly nesting macro definitions is not allowed, but this can be worked around using
+.Ic EQUS .
+This won't work:
+.Bd -literal -offset indent
+outer: MACRO
+inner: MACRO
+    PRINTT "Hello!\[rs]n"
+ENDM
+ENDM
+.Ed
+.Pp
+But this will:
+.Bd -literal -offset indent
+outer: MACRO
+definition equs "inner: MACRO\[rs]nPRINTT \[rs]"Hello!\[rs]\[rs]n\[rs]"\[rs]nENDM"
+definition
+    PURGE definition
+ENDM
+.Ed
 .El
 .Ss Exporting and importing symbols
 Importing and exporting of symbols is a feature that is very useful when your project spans many source files and, for example, you need to jump to a routine defined in another file.
diff --git a/test/asm/nested-macrodef.asm b/test/asm/nested-macrodef.asm
new file mode 100644
index 00000000..498fdfc6
--- /dev/null
+++ b/test/asm/nested-macrodef.asm
@@ -0,0 +1,27 @@
+outer_ok: MACRO
+definition equs "inner_ok: MACRO\nPRINTT \"Hello!\\n\"\nENDM"
+definition
+	PURGE definition
+ENDM
+
+	outer_ok
+	inner_ok
+
+
+outer_arg: MACRO
+definition equs "inner_arg: MACRO\nPRINTT \"outer: \1\\ninner: \\1\\n\"\nENDM"
+definition
+	PURGE definition
+ENDM
+
+	outer_arg outside
+	inner_arg inside
+
+
+outer: MACRO
+	WARN "Nested macros shouldn't work, whose argument would be \\1?"
+inner: MACRO
+ENDM
+
+	outer
+	inner
diff --git a/test/asm/nested-macrodef.err b/test/asm/nested-macrodef.err
new file mode 100644
index 00000000..eea37314
--- /dev/null
+++ b/test/asm/nested-macrodef.err
@@ -0,0 +1,5 @@
+warning: nested-macrodef.asm(26) -> nested-macrodef.asm::outer(22): [-Wuser]
+    Nested macros shouldn't work, whose argument would be \1?
+ERROR: nested-macrodef.asm(26) -> nested-macrodef.asm::outer(25):
+    Unterminated macro definition
+error: Assembly aborted (1 errors)!
diff --git a/test/asm/nested-macrodef.out b/test/asm/nested-macrodef.out
new file mode 100644
index 00000000..cb5c4d43
--- /dev/null
+++ b/test/asm/nested-macrodef.out
@@ -0,0 +1,3 @@
+Hello!
+outer: outside
+inner: inside