Add -B/--backtrace option to RGBASM and RGBLINK (#1787)

src/asm/fstack.cpp

@@ -60,42 +60,79 @@ std::string FileStackNode::reptChain() const {
 	return chain;
 }
 
-std::string const &FileStackNode::dump(uint32_t curLineNo) const {
+std::vector<std::pair<std::string, uint32_t>> FileStackNode::backtrace(uint32_t curLineNo) const {
 	if (std::holds_alternative<std::vector<uint32_t>>(data)) {
 		assume(parent); // REPT nodes use their parent's name
-		std::string const &lastName = parent->dump(lineNo);
-		style_Set(stderr, STYLE_CYAN, false);
-		fputs(" -> ", stderr);
-		style_Set(stderr, STYLE_CYAN, true);
-		fputs(lastName.c_str(), stderr);
-		fputs(reptChain().c_str(), stderr);
-		style_Set(stderr, STYLE_CYAN, false);
-		fprintf(stderr, "(%" PRIu32 ")", curLineNo);
-		style_Reset(stderr);
-		return lastName;
+		std::vector<std::pair<std::string, uint32_t>> nodes = parent->backtrace(lineNo);
+		assume(!nodes.empty());
+		nodes.emplace_back(nodes.back().first + reptChain(), curLineNo);
+		return nodes;
+	} else if (parent) {
+		std::vector<std::pair<std::string, uint32_t>> nodes = parent->backtrace(lineNo);
+		nodes.emplace_back(name(), curLineNo);
+		return nodes;
 	} else {
-		if (parent) {
-			parent->dump(lineNo);
-			style_Set(stderr, STYLE_CYAN, false);
-			fputs(" -> ", stderr);
-		}
-		std::string const &nodeName = name();
-		style_Set(stderr, STYLE_CYAN, true);
-		fputs(nodeName.c_str(), stderr);
-		style_Set(stderr, STYLE_CYAN, false);
-		fprintf(stderr, "(%" PRIu32 ")", curLineNo);
-		style_Reset(stderr);
-		return nodeName;
+		return {
+		    {name(), curLineNo}
+		};
 	}
 }
 
-bool fstk_DumpCurrent() {
-	if (lexer_AtTopLevel()) {
-		return false;
+static void printNode(std::pair<std::string, uint32_t> const &node) {
+	style_Set(stderr, STYLE_CYAN, true);
+	fputs(node.first.c_str(), stderr);
+	style_Set(stderr, STYLE_CYAN, false);
+	fprintf(stderr, "(%" PRIu32 ")", node.second);
+}
+
+void FileStackNode::printBacktrace(uint32_t curLineNo) const {
+	std::vector<std::pair<std::string, uint32_t>> nodes = backtrace(curLineNo);
+	size_t n = nodes.size();
+
+	if (warnings.traceDepth == TRACE_COLLAPSE) {
+		fputs("   ", stderr); // Just three spaces; the fourth will be handled by the loop
+		for (size_t i = 0; i < n; ++i) {
+			style_Reset(stderr);
+			fprintf(stderr, " %s ", i == 0 ? "at" : "<-");
+			printNode(nodes[n - i - 1]);
+		}
+		putc('\n', stderr);
+	} else if (warnings.traceDepth == 0 || static_cast<size_t>(warnings.traceDepth) >= n) {
+		for (size_t i = 0; i < n; ++i) {
+			style_Reset(stderr);
+			fprintf(stderr, "    %s ", i == 0 ? "at" : "<-");
+			printNode(nodes[n - i - 1]);
+			putc('\n', stderr);
+		}
+	} else {
+		size_t last = warnings.traceDepth / 2;
+		size_t first = warnings.traceDepth - last;
+		size_t skipped = n - warnings.traceDepth;
+		for (size_t i = 0; i < first; ++i) {
+			style_Reset(stderr);
+			fprintf(stderr, "    %s ", i == 0 ? "at" : "<-");
+			printNode(nodes[n - i - 1]);
+			putc('\n', stderr);
+		}
+		style_Reset(stderr);
+		fprintf(stderr, "    ...%zu more%s\n", skipped, last ? "..." : "");
+		for (size_t i = n - last; i < n; ++i) {
+			style_Reset(stderr);
+			fputs("    <- ", stderr);
+			printNode(nodes[n - i - 1]);
+			putc('\n', stderr);
+		}
 	}
-	assume(!contextStack.empty());
-	contextStack.top().fileInfo->dump(lexer_GetLineNo());
-	return true;
+
+	style_Reset(stderr);
+}
+
+void fstk_TraceCurrent() {
+	if (!lexer_AtTopLevel()) {
+		assume(!contextStack.empty());
+		contextStack.top().fileInfo->printBacktrace(lexer_GetLineNo());
+	}
+	lexer_TraceStringExpansions();
 }
 
 // LCOV_EXCL_START

src/asm/lexer.cpp

@@ -838,16 +838,16 @@ uint32_t lexer_GetLineNo() {
 	return lexerState->lineNo;
 }
 
-void lexer_DumpStringExpansions() {
+void lexer_TraceStringExpansions() {
 	if (!lexerState) {
 		return;
 	}
 
 	for (Expansion &exp : lexerState->expansions) {
-		// Only register EQUS expansions, not string args
+		// Only print EQUS expansions, not string args
 		if (exp.name) {
 			style_Set(stderr, STYLE_CYAN, false);
-			fputs("while expanding symbol \"", stderr);
+			fputs("    while expanding symbol \"", stderr);
 			style_Set(stderr, STYLE_CYAN, true);
 			fputs(exp.name->c_str(), stderr);
 			style_Set(stderr, STYLE_CYAN, false);

src/asm/main.cpp

@@ -34,7 +34,7 @@ static char const *dependFileName = nullptr;
 static std::unordered_map<std::string, std::vector<StateFeature>> stateFileSpecs; // -s
 
 // Short options
-static char const *optstring = "b:D:Eg:hI:M:o:P:p:Q:r:s:VvW:wX:";
+static char const *optstring = "B:b:D:Eg:hI:M:o:P:p:Q:r:s:VvW:wX:";
 
 // Variables for the long-only options
 static int longOpt; // `--color` and variants of `-M`
@@ -47,6 +47,7 @@ static int longOpt; // `--color` and variants of `-M`
 // This is because long opt matching, even to a single char, is prioritized
 // over short opt matching.
 static option const longopts[] = {
+    {"backtrace",       required_argument, nullptr,  'B'},
     {"binary-digits",   required_argument, nullptr,  'b'},
     {"define",          required_argument, nullptr,  'D'},
     {"export-all",      no_argument,       nullptr,  'E'},
@@ -77,7 +78,7 @@ static option const longopts[] = {
 static Usage usage = {
     .name = "rgbasm",
     .flags = {
-        "[-EhVvw]", "[-b chars]", "[-D name[=value]]", "[-g chars]", "[-I path]",
+        "[-EhVvw]", "[-B depth]", "[-b chars]", "[-D name[=value]]", "[-g chars]", "[-I path]",
         "[-M depend_file]", "[-MC]", "[-MG]", "[-MP]", "[-MT target_file]", "[-MQ target_file]",
         "[-o out_file]", "[-P include_file]", "[-p pad_value]", "[-Q precision]", "[-r depth]",
         "[-s features:state_file]", "[-W warning]", "[-X max_errors]", "<file>",
@@ -215,7 +216,6 @@ static void verboseOutputConfig(int argc, char *argv[]) {
 		if (options.generatePhonyDeps) {
 			fputs("\tGenerate phony dependencies\n", stderr);
 		}
-		// [-MG] [-MC]
 	}
 	fputs("Ready.\n", stderr);
 
@@ -303,6 +303,24 @@ int main(int argc, char *argv[]) {
 		switch (ch) {
 			char *endptr;
 
+		case 'B': {
+			if (!strcasecmp(musl_optarg, "collapse")) {
+				warnings.traceDepth = TRACE_COLLAPSE;
+				break;
+			}
+
+			warnings.traceDepth = strtoul(musl_optarg, &endptr, 0);
+
+			if (musl_optarg[0] == '\0' || *endptr != '\0') {
+				fatal("Invalid argument for option 'B'");
+			}
+
+			if (warnings.traceDepth >= UINT64_MAX) {
+				fatal("Argument for option 'B' is too large");
+			}
+			break;
+		}
+
 		case 'b':
 			if (strlen(musl_optarg) == 2) {
 				opt_B(musl_optarg);

src/asm/section.cpp

@@ -281,9 +281,12 @@ static void mergeSections(
 			break;
 
 		case SECTION_NORMAL:
-			sectError([&]() {
-				fputs("Section already defined previously at ", stderr);
-				sect.src->dump(sect.fileLine);
+			errorNoTrace([&]() {
+				fputs("Section already defined\n", stderr);
+				fstk_TraceCurrent();
+				fputs("    and also:\n", stderr);
+				sect.src->printBacktrace(sect.fileLine);
+				++nbSectErrors;
 			});
 			break;
 		}

src/asm/symbol.cpp

@@ -113,14 +113,15 @@ std::shared_ptr<std::string> Symbol::getEqus() const {
 	return std::get<std::shared_ptr<std::string>>(data);
 }
 
-static void dumpFilename(Symbol const &sym) {
-	fputs(" at ", stderr);
+// Meant to be called last in an `errorNoTrace` callback
+static void printBacktraces(Symbol const &sym) {
+	putc('\n', stderr);
+	fstk_TraceCurrent();
+	fputs("    and also:\n", stderr);
 	if (sym.src) {
-		sym.src->dump(sym.fileLine);
-	} else if (sym.isBuiltin) {
-		fputs("<builtin>", stderr);
+		sym.src->printBacktrace(sym.fileLine);
 	} else {
-		fputs("<command-line>", stderr);
+		fprintf(stderr, "    at <%s>\n", sym.isBuiltin ? "builtin" : "command-line");
 	}
 }
 
@@ -141,37 +142,52 @@ static bool isValidIdentifier(std::string const &s) {
 }
 
 static void alreadyDefinedError(Symbol const &sym, char const *asType) {
-	if (sym.isBuiltin && !sym_FindScopedValidSymbol(sym.name)) {
-		// `DEF()` would return false, so we should not claim the symbol is already defined
-		error("'%s' is reserved for a built-in symbol", sym.name.c_str());
+	auto suggestion = [&]() {
+		std::string s;
+		if (auto const &contents = sym.type == SYM_EQUS ? sym.getEqus() : nullptr;
+		    contents && isValidIdentifier(*contents)) {
+			s.append(" (should it be {interpolated} to define its contents \"");
+			s.append(*contents);
+			s.append("\"?)");
+		}
+		return s;
+	};
+
+	if (sym.isBuiltin) {
+		if (sym_FindScopedValidSymbol(sym.name)) {
+			if (std::string s = suggestion(); asType) {
+				error("'%s' already defined as built-in %s%s", sym.name.c_str(), asType, s.c_str());
+			} else {
+				error("'%s' already defined as built-in%s", sym.name.c_str(), s.c_str());
+			}
+		} else {
+			// `DEF()` would return false, so we should not claim the symbol is already defined,
+			// nor suggest to interpolate it
+			if (asType) {
+				error("'%s' is reserved for a built-in %s symbol", sym.name.c_str(), asType);
+			} else {
+				error("'%s' is reserved for a built-in symbol", sym.name.c_str());
+			}
+		}
 	} else {
-		error([&]() {
+		errorNoTrace([&]() {
 			fprintf(stderr, "'%s' already defined", sym.name.c_str());
 			if (asType) {
 				fprintf(stderr, " as %s", asType);
 			}
-			dumpFilename(sym);
-			if (sym.type != SYM_EQUS) {
-				return;
-			}
-			if (std::string const &contents = *sym.getEqus(); isValidIdentifier(contents)) {
-				fprintf(
-				    stderr,
-				    "\n    (should it be {interpolated} to define its contents \"%s\"?)",
-				    contents.c_str()
-				);
-			}
+			fputs(suggestion().c_str(), stderr);
+			printBacktraces(sym);
 		});
 	}
 }
 
 static void redefinedError(Symbol const &sym) {
 	assume(sym.isBuiltin);
-	if (!sym_FindScopedValidSymbol(sym.name)) {
+	if (sym_FindScopedValidSymbol(sym.name)) {
+		error("Built-in symbol '%s' cannot be redefined", sym.name.c_str());
+	} else {
 		// `DEF()` would return false, so we should not imply the symbol is already defined
 		error("'%s' is reserved for a built-in symbol", sym.name.c_str());
-	} else {
-		error("Built-in symbol '%s' cannot be redefined", sym.name.c_str());
 	}
 }
 
@@ -376,9 +392,9 @@ static Symbol *createNonrelocSymbol(std::string const &symName, bool numeric) {
 		return nullptr; // Don't allow overriding the symbol, that'd be bad!
 	} else if (!numeric) {
 		// The symbol has already been referenced, but it's not allowed
-		error([&]() {
+		errorNoTrace([&]() {
 			fprintf(stderr, "'%s' already referenced", symName.c_str());
-			dumpFilename(*sym);
+			printBacktraces(*sym);
 		});
 		return nullptr; // Don't allow overriding the symbol, that'd be bad!
 	}
@@ -444,9 +460,9 @@ Symbol *sym_RedefString(std::string const &symName, std::shared_ptr<std::string>
 		if (sym->isDefined()) {
 			alreadyDefinedError(*sym, "non-EQUS");
 		} else {
-			error([&]() {
+			errorNoTrace([&]() {
 				fprintf(stderr, "'%s' already referenced", symName.c_str());
-				dumpFilename(*sym);
+				printBacktraces(*sym);
 			});
 		}
 		return nullptr;

src/asm/warning.cpp

@@ -60,6 +60,7 @@ Diagnostics<WarningLevel, WarningID> warnings = {
         {WARNING_UNMAPPED_CHAR_1,  WARNING_UNMAPPED_CHAR_2,  1},
     },
     .state = DiagnosticsState<WarningID>(),
+    .traceDepth = 0,
     .nbErrors = 0,
 };
 // clang-format on
@@ -74,19 +75,16 @@ static void printDiag(
 ) {
 	style_Set(stderr, color, true);
 	fprintf(stderr, "%s: ", type);
-	if (fstk_DumpCurrent()) {
-		putc(':', stderr);
-		if (flagfmt) {
-			style_Set(stderr, color, true);
-			fprintf(stderr, flagfmt, flag);
-		}
-		fputs("\n    ", stderr);
-	}
 	style_Reset(stderr);
 	vfprintf(stderr, fmt, args);
+	if (flagfmt) {
+		style_Set(stderr, color, true);
+		putc(' ', stderr);
+		fprintf(stderr, flagfmt, flag);
+	}
 	putc('\n', stderr);
 
-	lexer_DumpStringExpansions();
+	fstk_TraceCurrent();
 }
 
 static void incrementErrors() {
@@ -117,16 +115,11 @@ void error(char const *fmt, ...) {
 	incrementErrors();
 }
 
-void error(std::function<void()> callback) {
+void errorNoTrace(std::function<void()> callback) {
 	style_Set(stderr, STYLE_RED, true);
 	fputs("error: ", stderr);
-	if (fstk_DumpCurrent()) {
-		fputs(":\n    ", stderr);
-	}
 	style_Reset(stderr);
 	callback();
-	putc('\n', stderr);
-	lexer_DumpStringExpansions();
 
 	incrementErrors();
 }
@@ -167,11 +160,11 @@ void warning(WarningID id, char const *fmt, ...) {
 		break;
 
 	case WarningBehavior::ENABLED:
-		printDiag(fmt, args, "warning", STYLE_YELLOW, " [-W%s]", flag);
+		printDiag(fmt, args, "warning", STYLE_YELLOW, "[-W%s]", flag);
 		break;
 
 	case WarningBehavior::ERROR:
-		printDiag(fmt, args, "error", STYLE_RED, " [-Werror=%s]", flag);
+		printDiag(fmt, args, "error", STYLE_RED, "[-Werror=%s]", flag);
 		break;
 	}