Refactor macro args to be owned collectively by their fstack contexts

include/asm/fstack.hpp

@@ -54,6 +54,7 @@ struct MacroArgs;
 void fstk_DumpCurrent();
 std::shared_ptr<FileStackNode> fstk_GetFileStack();
 std::shared_ptr<std::string> fstk_GetUniqueIDStr();
+MacroArgs *fstk_GetCurrentMacroArgs();
 
 void fstk_AddIncludePath(std::string const &path);
 void fstk_SetPreIncludeFile(std::string const &path);
@@ -61,7 +62,7 @@ std::optional<std::string> fstk_FindFile(std::string const &path);
 
 bool yywrap();
 void fstk_RunInclude(std::string const &path);
-void fstk_RunMacro(std::string const &macroName, std::shared_ptr<MacroArgs> args);
+void fstk_RunMacro(std::string const &macroName, std::shared_ptr<MacroArgs> macroArgs);
 void fstk_RunRept(uint32_t count, int32_t reptLineNo, char const *body, size_t size);
 void fstk_RunFor(
     std::string const &symName,

include/asm/macro.hpp

@@ -12,15 +12,13 @@ struct MacroArgs {
 	unsigned int shift;
 	std::vector<std::shared_ptr<std::string>> args;
 
-	void append(std::shared_ptr<std::string> arg);
+	uint32_t nbArgs() const { return args.size() - shift; }
+	std::shared_ptr<std::string> getArg(uint32_t i) const;
+	std::shared_ptr<std::string> getAllArgs() const;
+
+	void appendArg(std::shared_ptr<std::string> arg);
+	void shiftArgs(int32_t count);
 };
 
-bool macro_HasCurrentArgs();
-std::shared_ptr<MacroArgs> macro_GetCurrentArgs();
-void macro_UseNewArgs(std::shared_ptr<MacroArgs> args);
-uint32_t macro_NbArgs();
-std::shared_ptr<std::string> macro_GetArg(uint32_t i);
-std::shared_ptr<std::string> macro_GetAllArgs();
-void macro_ShiftCurrentArgs(int32_t count);
 
 #endif // RGBDS_MACRO_H

src/asm/fstack.cpp

@@ -119,6 +119,12 @@ std::shared_ptr<std::string> fstk_GetUniqueIDStr() {
 	return str;
 }
 
+MacroArgs *fstk_GetCurrentMacroArgs() {
+	// This returns a raw pointer, *not* a shared pointer, so its returned value
+	// does *not* keep the current macro args alive!
+	return contextStack.top().macroArgs.get();
+}
+
 void fstk_AddIncludePath(std::string const &path) {
 	if (path.empty())
 		return;
@@ -214,15 +220,8 @@ bool yywrap() {
 		return true;
 	}
 
-	Context oldContext = std::move(contextStack.top());
+	lexer_CleanupState(contextStack.top().lexerState);
 	contextStack.pop();
-
-	lexer_CleanupState(oldContext.lexerState);
-
-	// Restore args if a macro (not REPT) saved them
-	if (oldContext.fileInfo->type == NODE_MACRO)
-		macro_UseNewArgs(contextStack.top().macroArgs);
-
 	lexer_SetState(&contextStack.top().lexerState);
 
 	return false;
@@ -243,6 +242,7 @@ static Context &newContext(std::shared_ptr<FileStackNode> fileInfo) {
 	    .uniqueIDStr = fileInfo->generatesUniqueID()
 	                       ? std::make_shared<std::string>() // Create a new, not-yet-generated ID.
 	                       : contextStack.top().uniqueIDStr, // Make a copy.
+	    .macroArgs = contextStack.top().macroArgs,
 	});
 }
 
@@ -285,7 +285,7 @@ static void runPreIncludeFile() {
 	lexer_SetState(&context.lexerState);
 }
 
-void fstk_RunMacro(std::string const &macroName, std::shared_ptr<MacroArgs> args) {
+void fstk_RunMacro(std::string const &macroName, std::shared_ptr<MacroArgs> macroArgs) {
 	Symbol *macro = sym_FindExactSymbol(macroName);
 
 	if (!macro) {
@@ -296,7 +296,6 @@ void fstk_RunMacro(std::string const &macroName, std::shared_ptr<MacroArgs> args
 		error("\"%s\" is not a macro\n", macroName.c_str());
 		return;
 	}
-	contextStack.top().macroArgs = macro_GetCurrentArgs();
 
 	auto fileInfo = std::make_shared<FileStackNode>(NODE_MACRO, "");
 
@@ -325,7 +324,7 @@ void fstk_RunMacro(std::string const &macroName, std::shared_ptr<MacroArgs> args
 	    context.lexerState, "MACRO", macroView->data(), macroView->size(), macro->fileLine
 	);
 	lexer_SetStateAtEOL(&context.lexerState);
-	macro_UseNewArgs(args);
+	context.macroArgs = macroArgs;
 }
 
 static bool newReptContext(int32_t reptLineNo, char const *body, size_t size) {

src/asm/lexer.cpp

@@ -583,9 +583,10 @@ static std::shared_ptr<std::string> readMacroArg(char name) {
 		}
 		return str;
 	} else if (name == '#') {
-		auto str = macro_GetAllArgs();
+		MacroArgs *macroArgs = fstk_GetCurrentMacroArgs();
+		auto str = macroArgs ? macroArgs->getAllArgs() : nullptr;
 		if (!str) {
-			error("'\\#' cannot be used outside of a macro");
+			error("'\\#' cannot be used outside of a macro\n");
 		}
 		return str;
 	} else if (name == '<') {
@@ -595,7 +596,13 @@ static std::shared_ptr<std::string> readMacroArg(char name) {
 			return nullptr;
 		}
 
-		auto str = macro_GetArg(num);
+		MacroArgs *macroArgs = fstk_GetCurrentMacroArgs();
+		if (!macroArgs) {
+			error("'\\<%" PRIu32 ">' cannot be used outside of a macro\n", num);
+			return nullptr;
+		}
+
+		auto str = macroArgs->getArg(num);
 		if (!str) {
 			error("Macro argument '\\<%" PRIu32 ">' not defined\n", num);
 		}
@@ -605,7 +612,14 @@ static std::shared_ptr<std::string> readMacroArg(char name) {
 		return nullptr;
 	} else {
 		assert(name > '0' && name <= '9');
-		auto str = macro_GetArg(name - '0');
+
+		MacroArgs *macroArgs = fstk_GetCurrentMacroArgs();
+		if (!macroArgs) {
+			error("'\\%c' cannot be used outside of a macro\n", name);
+			return nullptr;
+		}
+
+		auto str = macroArgs->getArg(name - '0');
 		if (!str) {
 			error("Macro argument '\\%c' not defined\n", name);
 		}

src/asm/macro.cpp

@@ -12,60 +12,28 @@
 
 #define MAXMACROARGS 99999
 
-static std::shared_ptr<MacroArgs> macroArgs = nullptr;
+std::shared_ptr<std::string> MacroArgs::getArg(uint32_t i) const {
+	uint32_t realIndex = i + shift - 1;
 
-void MacroArgs::append(std::shared_ptr<std::string> arg) {
+	return realIndex >= args.size() ? nullptr : args[realIndex];
-	if (arg->empty())
-		warning(WARNING_EMPTY_MACRO_ARG, "Empty macro argument\n");
-	if (args.size() == MAXMACROARGS)
-		error("A maximum of " EXPAND_AND_STR(MAXMACROARGS) " arguments is allowed\n");
-	args.push_back(arg);
 }
 
-bool macro_HasCurrentArgs() {
+std::shared_ptr<std::string> MacroArgs::getAllArgs() const {
-	return macroArgs != nullptr;
+	size_t nbArgs = args.size();
-}
 
-std::shared_ptr<MacroArgs> macro_GetCurrentArgs() {
+	if (shift >= nbArgs)
-	return macroArgs;
-}
-
-void macro_UseNewArgs(std::shared_ptr<MacroArgs> args) {
-	macroArgs = args;
-}
-
-uint32_t macro_NbArgs() {
-	return macroArgs->args.size() - macroArgs->shift;
-}
-
-std::shared_ptr<std::string> macro_GetArg(uint32_t i) {
-	if (!macroArgs)
-		return nullptr;
-
-	uint32_t realIndex = i + macroArgs->shift - 1;
-
-	return realIndex >= macroArgs->args.size() ? nullptr : macroArgs->args[realIndex];
-}
-
-std::shared_ptr<std::string> macro_GetAllArgs() {
-	if (!macroArgs)
-		return nullptr;
-
-	size_t nbArgs = macroArgs->args.size();
-
-	if (macroArgs->shift >= nbArgs)
 		return std::make_shared<std::string>("");
 
 	size_t len = 0;
 
-	for (uint32_t i = macroArgs->shift; i < nbArgs; i++)
+	for (uint32_t i = shift; i < nbArgs; i++)
-		len += macroArgs->args[i]->length() + 1; // 1 for comma
+		len += args[i]->length() + 1; // 1 for comma
 
 	auto str = std::make_shared<std::string>();
 	str->reserve(len + 1); // 1 for comma
 
-	for (uint32_t i = macroArgs->shift; i < nbArgs; i++) {
+	for (uint32_t i = shift; i < nbArgs; i++) {
-		auto const &arg = macroArgs->args[i];
+		auto const &arg = args[i];
 
 		str->append(*arg);
 
@@ -77,17 +45,22 @@ std::shared_ptr<std::string> macro_GetAllArgs() {
 	return str;
 }
 
-void macro_ShiftCurrentArgs(int32_t count) {
+void MacroArgs::appendArg(std::shared_ptr<std::string> arg) {
-	if (!macroArgs) {
+	if (arg->empty())
-		error("Cannot shift macro arguments outside of a macro\n");
+		warning(WARNING_EMPTY_MACRO_ARG, "Empty macro argument\n");
-	} else if (size_t nbArgs = macroArgs->args.size();
+	if (args.size() == MAXMACROARGS)
-	           count > 0 && ((uint32_t)count > nbArgs || macroArgs->shift > nbArgs - count)) {
+		error("A maximum of " EXPAND_AND_STR(MAXMACROARGS) " arguments is allowed\n");
+	args.push_back(arg);
+}
+
+void MacroArgs::shiftArgs(int32_t count) {
+	if (size_t nbArgs = args.size(); count > 0 && ((uint32_t)count > nbArgs || shift > nbArgs - count)) {
 		warning(WARNING_MACRO_SHIFT, "Cannot shift macro arguments past their end\n");
-		macroArgs->shift = nbArgs;
+		shift = nbArgs;
-	} else if (count < 0 && macroArgs->shift < (uint32_t)-count) {
+	} else if (count < 0 && shift < (uint32_t)-count) {
 		warning(WARNING_MACRO_SHIFT, "Cannot shift macro arguments past their beginning\n");
-		macroArgs->shift = 0;
+		shift = 0;
 	} else {
-		macroArgs->shift += count;
+		shift += count;
 	}
 }

src/asm/parser.y

@@ -522,7 +522,7 @@ macro_args:
 	}
 	| macro_args STRING {
 		$$ = std::move($1);
-		$$->append(std::make_shared<std::string>($2));
+		$$->appendArg(std::make_shared<std::string>($2));
 	}
 ;
 
@@ -843,10 +843,18 @@ assert:
 
 shift:
 	POP_SHIFT {
-		macro_ShiftCurrentArgs(1);
+		if (MacroArgs *macroArgs = fstk_GetCurrentMacroArgs(); macroArgs) {
+			macroArgs->shiftArgs(1);
+		} else {
+			::error("Cannot shift macro arguments outside of a macro\n");
+		}
 	}
 	| POP_SHIFT const {
-		macro_ShiftCurrentArgs($2);
+		if (MacroArgs *macroArgs = fstk_GetCurrentMacroArgs(); macroArgs) {
+			macroArgs->shiftArgs($2);
+		} else {
+			::error("Cannot shift macro arguments outside of a macro\n");
+		}
 	}
 ;
 

src/asm/symbol.cpp

@@ -43,11 +43,12 @@ void sym_ForEach(void (*callback)(Symbol &)) {
 }
 
 static int32_t Callback_NARG() {
-	if (!macro_HasCurrentArgs()) {
+	if (MacroArgs const *macroArgs = fstk_GetCurrentMacroArgs(); macroArgs) {
+		return macroArgs->nbArgs();
+	} else {
 		error("_NARG does not make sense outside of a macro\n");
 		return 0;
 	}
-	return macro_NbArgs();
 }
 
 static int32_t CallbackPC() {
@@ -147,7 +148,7 @@ Symbol *sym_FindScopedValidSymbol(std::string const &symName) {
 		return nullptr;
 	}
 	// `_NARG` has no value outside a macro
-	if (sym == _NARGSymbol && !macro_HasCurrentArgs()) {
+	if (sym == _NARGSymbol && !fstk_GetCurrentMacroArgs()) {
 		return nullptr;
 	}
 	return sym;

test/asm/interpolation-overflow.err

@@ -4,7 +4,7 @@ warning: interpolation-overflow.asm(4): [-Wlarge-constant]
     Precision of fixed-point constant is too large
 while expanding symbol "0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
 error: interpolation-overflow.asm(4):
-    Macro argument '\1' not defined
+    '\1' cannot be used outside of a macro
 error: interpolation-overflow.asm(4):
     syntax error, unexpected number
 error: Assembly aborted (3 errors)!

test/asm/macro-syntax.err

@@ -3,7 +3,7 @@ error: macro-syntax.asm(7):
 error: macro-syntax.asm(7):
     syntax error, unexpected MACRO
 error: macro-syntax.asm(8):
-    Macro argument '\1' not defined
+    '\1' cannot be used outside of a macro
 error: macro-syntax.asm(9):
     syntax error, unexpected ENDM
 error: Assembly aborted (4 errors)!

test/asm/macro-syntax.simple.err

@@ -3,7 +3,7 @@ error: macro-syntax.asm(7):
 error: macro-syntax.asm(7):
     syntax error
 error: macro-syntax.asm(8):
-    Macro argument '\1' not defined
+    '\1' cannot be used outside of a macro
 error: macro-syntax.asm(9):
     syntax error
 error: Assembly aborted (4 errors)!