Refactor warning diagnostic code to reduce repeated output logic

include/diagnostics.hpp

@@ -15,9 +15,20 @@
 
 #include "helpers.hpp"
 #include "itertools.hpp"
+#include "style.hpp"
 
 [[gnu::format(printf, 1, 2)]]
 void warnx(char const *fmt, ...);
+[[gnu::format(printf, 1, 0)]]
+void vwarnx(char const *fmt, va_list args);
+[[gnu::format(printf, 1, 2)]]
+void errorx(char const *fmt, ...);
+[[gnu::format(printf, 1, 0)]]
+void verrorx(char const *fmt, va_list args);
+[[gnu::format(printf, 1, 2)]]
+void fatalx(char const *fmt, ...);
+[[gnu::format(printf, 1, 0)]]
+void vfatalx(char const *fmt, va_list args);
 
 enum WarningAbled { WARNING_DEFAULT, WARNING_ENABLED, WARNING_DISABLED };
 
@@ -208,4 +219,43 @@ void Diagnostics<LevelEnumT, WarningEnumT>::processWarningFlag(char const *flag)
 	warnx("Unknown warning flag \"%s\"", rootFlag.c_str());
 }
 
+template<Enum LevelEnumT, Enum WarningEnumT>
+[[gnu::format(printf, 3, 0)]]
+WarningBehavior printDiagnostic(
+    Diagnostics<LevelEnumT, WarningEnumT> const &warnings,
+    WarningEnumT id,
+    char const *fmt,
+    va_list args
+) {
+	char const *flag = warnings.warningFlags[id].name;
+	WarningBehavior behavior = warnings.getWarningBehavior(id);
+
+	switch (behavior) {
+	case WarningBehavior::DISABLED:
+		break;
+
+	case WarningBehavior::ENABLED:
+		style_Set(stderr, STYLE_YELLOW, true);
+		fputs("warning: ", stderr);
+		style_Reset(stderr);
+		vfprintf(stderr, fmt, args);
+		style_Set(stderr, STYLE_YELLOW, true);
+		fprintf(stderr, " [-W%s]\n", flag);
+		style_Reset(stderr);
+		break;
+
+	case WarningBehavior::ERROR:
+		style_Set(stderr, STYLE_RED, true);
+		fputs("error: ", stderr);
+		style_Reset(stderr);
+		vfprintf(stderr, fmt, args);
+		style_Set(stderr, STYLE_RED, true);
+		fprintf(stderr, " [-Werror=%s]\n", flag);
+		style_Reset(stderr);
+		break;
+	}
+
+	return behavior;
+}
+
 #endif // RGBDS_DIAGNOSTICS_HPP

src/asm/warning.cpp

@@ -61,28 +61,6 @@ Diagnostics<WarningLevel, WarningID> warnings = {
 };
 // clang-format on
 
-static void printDiag(
-    char const *fmt,
-    va_list args,
-    char const *type,
-    StyleColor color,
-    char const *flagfmt,
-    char const *flag
-) {
-	style_Set(stderr, color, true);
-	fprintf(stderr, "%s: ", type);
-	style_Reset(stderr);
-	vfprintf(stderr, fmt, args);
-	if (flagfmt) {
-		style_Set(stderr, color, true);
-		putc(' ', stderr);
-		fprintf(stderr, flagfmt, flag);
-	}
-	putc('\n', stderr);
-
-	fstk_TraceCurrent();
-}
-
 static void incrementErrors() {
 	// This intentionally makes 0 act as "unlimited"
 	warnings.incrementErrors();
@@ -103,11 +81,11 @@ static void incrementErrors() {
 
 void error(char const *fmt, ...) {
 	va_list args;
-
 	va_start(args, fmt);
-	printDiag(fmt, args, "error", STYLE_RED, nullptr, nullptr);
+	verrorx(fmt, args);
 	va_end(args);
 
+	fstk_TraceCurrent();
 	incrementErrors();
 }
 
@@ -115,19 +93,19 @@ void errorNoTrace(std::function<void()> callback) {
 	style_Set(stderr, STYLE_RED, true);
 	fputs("error: ", stderr);
 	style_Reset(stderr);
-	callback();
 
+	callback();
 	incrementErrors();
 }
 
 [[noreturn]]
 void fatal(char const *fmt, ...) {
 	va_list args;
-
 	va_start(args, fmt);
-	printDiag(fmt, args, "FATAL", STYLE_RED, nullptr, nullptr);
+	vfatalx(fmt, args);
 	va_end(args);
 
+	fstk_TraceCurrent();
 	exit(1);
 }
 
@@ -136,8 +114,8 @@ void fatalNoTrace(std::function<void()> callback) {
 	style_Set(stderr, STYLE_RED, true);
 	fputs("FATAL: ", stderr);
 	style_Reset(stderr);
-	callback();
 
+	callback();
 	exit(1);
 }
 
@@ -156,25 +134,15 @@ void requireZeroErrors() {
 }
 
 void warning(WarningID id, char const *fmt, ...) {
-	char const *flag = warnings.warningFlags[id].name;
 	va_list args;
-
 	va_start(args, fmt);
-
+	WarningBehavior behavior = printDiagnostic(warnings, id, fmt, args);
-	switch (warnings.getWarningBehavior(id)) {
-	case WarningBehavior::DISABLED:
-		break;
-
-	case WarningBehavior::ENABLED:
-		printDiag(fmt, args, "warning", STYLE_YELLOW, "[-W%s]", flag);
-		break;
-
-	case WarningBehavior::ERROR:
-		printDiag(fmt, args, "error", STYLE_RED, "[-Werror=%s]", flag);
-
-		incrementErrors();
-		break;
-	}
-
 	va_end(args);
+
+	if (behavior != WarningBehavior::DISABLED) {
+		fstk_TraceCurrent();
+		if (behavior == WarningBehavior::ERROR) {
+			incrementErrors();
+		}
+	}
 }

src/cli.cpp

@@ -9,8 +9,8 @@
 #include <string>
 #include <vector>
 
+#include "diagnostics.hpp" // fatalx
 #include "extern/getopt.hpp"
-#include "style.hpp"
 #include "usage.hpp"
 #include "util.hpp" // isBlankSpace
 
@@ -24,10 +24,7 @@ static std::vector<size_t>
 	std::filebuf file;
 	if (!file.open(path, std::ios_base::in)) {
 		int errnum = errno;
-		style_Set(stderr, STYLE_RED, true);
+		fatalx("Failed to open at-file \"%s\": %s", path.c_str(), strerror(errnum));
-		fputs("FATAL: ", stderr);
-		style_Reset(stderr);
-		fprintf(stderr, "Failed to open at-file \"%s\": %s\n", path.c_str(), strerror(errnum));
 		usage.printAndExit(1);
 	}
 

src/diagnostics.cpp

@@ -14,13 +14,47 @@
 #include "util.hpp" // parseNumber
 
 void warnx(char const *fmt, ...) {
-	va_list ap;
+	va_list args;
+	va_start(args, fmt);
+	vwarnx(fmt, args);
+	va_end(args);
+}
+
+void vwarnx(char const *fmt, va_list args) {
 	style_Set(stderr, STYLE_YELLOW, true);
 	fputs("warning: ", stderr);
 	style_Reset(stderr);
-	va_start(ap, fmt);
+	vfprintf(stderr, fmt, args);
-	vfprintf(stderr, fmt, ap);
+	putc('\n', stderr);
-	va_end(ap);
+}
+
+void errorx(char const *fmt, ...) {
+	va_list args;
+	va_start(args, fmt);
+	verrorx(fmt, args);
+	va_end(args);
+}
+
+void verrorx(char const *fmt, va_list args) {
+	style_Set(stderr, STYLE_RED, true);
+	fputs("error: ", stderr);
+	style_Reset(stderr);
+	vfprintf(stderr, fmt, args);
+	putc('\n', stderr);
+}
+
+void fatalx(char const *fmt, ...) {
+	va_list args;
+	va_start(args, fmt);
+	vfatalx(fmt, args);
+	va_end(args);
+}
+
+void vfatalx(char const *fmt, va_list args) {
+	style_Set(stderr, STYLE_RED, true);
+	fputs("FATAL: ", stderr);
+	style_Reset(stderr);
+	vfprintf(stderr, fmt, args);
 	putc('\n', stderr);
 }
 

src/extern/getopt.cpp

@@ -4,33 +4,19 @@
 
 #include "extern/getopt.hpp"
 
-#include <limits.h>
+#include <limits.h> // MB_LEN_MAX
-#include <stdarg.h>
 #include <stddef.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <wchar.h>
 
-#include "style.hpp"
+#include "diagnostics.hpp" // errorx
 
 char *musl_optarg;
 int musl_optind = 1, musl_optopt;
 
 static int musl_optpos;
 
-[[gnu::format(printf, 1, 2)]]
-static void musl_getopt_error(char const *fmt, ...) {
-	style_Set(stderr, STYLE_RED, true);
-	fputs("error: ", stderr);
-	style_Reset(stderr);
-	va_list args;
-	va_start(args, fmt);
-	vfprintf(stderr, fmt, args);
-	va_end(args);
-	putc('\n', stderr);
-}
-
 static int musl_getopt(int argc, char *argv[], char const *optstring) {
 	if (!musl_optind) {
 		musl_optpos = 0;
@@ -96,7 +82,7 @@ static int musl_getopt(int argc, char *argv[], char const *optstring) {
 	if (d != c || c == ':') {
 		musl_optopt = c;
 		if (optstring[0] != ':') {
-			musl_getopt_error("Unrecognized option '-%s'", optchar);
+			errorx("Unrecognized option '-%s'", optchar);
 		}
 		return '?';
 	}
@@ -111,7 +97,7 @@ static int musl_getopt(int argc, char *argv[], char const *optstring) {
 			if (optstring[0] == ':') {
 				return ':';
 			}
-			musl_getopt_error("Missing argument for option '-%s'", optchar);
+			errorx("Missing argument for option '-%s'", optchar);
 			return '?';
 		}
 	}
@@ -183,7 +169,7 @@ static int
 				if (colon) {
 					return '?';
 				}
-				musl_getopt_error("Option '--%s' does not take an argument", longopts[i].name);
+				errorx("Option '--%s' does not take an argument", longopts[i].name);
 				return '?';
 			}
 			musl_optarg = opt + 1;
@@ -194,7 +180,7 @@ static int
 				if (colon) {
 					return ':';
 				}
-				musl_getopt_error("Missing argument for option '--%s'", longopts[i].name);
+				errorx("Missing argument for option '--%s'", longopts[i].name);
 				return '?';
 			}
 			++musl_optind;
@@ -209,9 +195,9 @@ static int
 		musl_optopt = 0;
 		if (!colon) {
 			if (cnt) {
-				musl_getopt_error("Ambiguous option '--%s'", argv[musl_optind] + 2);
+				errorx("Ambiguous option '--%s'", argv[musl_optind] + 2);
 			} else {
-				musl_getopt_error("Unrecognized option '--%s'", argv[musl_optind] + 2);
+				errorx("Unrecognized option '--%s'", argv[musl_optind] + 2);
 			}
 		}
 		++musl_optind;

src/fix/warning.cpp

@@ -46,63 +46,30 @@ uint32_t checkErrors(char const *filename) {
 }
 
 void error(char const *fmt, ...) {
-	va_list ap;
+	va_list args;
-	style_Set(stderr, STYLE_RED, true);
+	va_start(args, fmt);
-	fputs("error: ", stderr);
+	verrorx(fmt, args);
-	style_Reset(stderr);
+	va_end(args);
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	va_end(ap);
-	putc('\n', stderr);
 
 	warnings.incrementErrors();
 }
 
 void fatal(char const *fmt, ...) {
-	va_list ap;
+	va_list args;
-	style_Set(stderr, STYLE_RED, true);
+	va_start(args, fmt);
-	fputs("FATAL: ", stderr);
+	vfatalx(fmt, args);
-	style_Reset(stderr);
+	va_end(args);
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	va_end(ap);
-	putc('\n', stderr);
 
 	exit(1);
 }
 
 void warning(WarningID id, char const *fmt, ...) {
-	char const *flag = warnings.warningFlags[id].name;
+	va_list args;
-	va_list ap;
+	va_start(args, fmt);
-
+	WarningBehavior behavior = printDiagnostic(warnings, id, fmt, args);
-	switch (warnings.getWarningBehavior(id)) {
+	va_end(args);
-	case WarningBehavior::DISABLED:
-		break;
-
-	case WarningBehavior::ENABLED:
-		style_Set(stderr, STYLE_YELLOW, true);
-		fputs("warning: ", stderr);
-		style_Reset(stderr);
-		va_start(ap, fmt);
-		vfprintf(stderr, fmt, ap);
-		va_end(ap);
-		style_Set(stderr, STYLE_YELLOW, true);
-		fprintf(stderr, " [-W%s]\n", flag);
-		style_Reset(stderr);
-		break;
-
-	case WarningBehavior::ERROR:
-		style_Set(stderr, STYLE_RED, true);
-		fputs("error: ", stderr);
-		style_Reset(stderr);
-		va_start(ap, fmt);
-		vfprintf(stderr, fmt, ap);
-		va_end(ap);
-		style_Set(stderr, STYLE_RED, true);
-		fprintf(stderr, " [-Werror=%s]\n", flag);
-		style_Reset(stderr);
 
+	if (behavior == WarningBehavior::ERROR) {
 		warnings.incrementErrors();
-		break;
 	}
 }

src/gfx/warning.cpp

@@ -47,65 +47,32 @@ void requireZeroErrors() {
 }
 
 void error(char const *fmt, ...) {
-	va_list ap;
+	va_list args;
-	style_Set(stderr, STYLE_RED, true);
+	va_start(args, fmt);
-	fputs("error: ", stderr);
+	verrorx(fmt, args);
-	style_Reset(stderr);
+	va_end(args);
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	va_end(ap);
-	putc('\n', stderr);
 
 	warnings.incrementErrors();
 }
 
 [[noreturn]]
 void fatal(char const *fmt, ...) {
-	va_list ap;
+	va_list args;
-	style_Set(stderr, STYLE_RED, true);
+	va_start(args, fmt);
-	fputs("FATAL: ", stderr);
+	vfatalx(fmt, args);
-	style_Reset(stderr);
+	va_end(args);
-	va_start(ap, fmt);
-	vfprintf(stderr, fmt, ap);
-	va_end(ap);
-	putc('\n', stderr);
 
 	warnings.incrementErrors();
 	giveUp();
 }
 
 void warning(WarningID id, char const *fmt, ...) {
-	char const *flag = warnings.warningFlags[id].name;
+	va_list args;
-	va_list ap;
+	va_start(args, fmt);
-
+	WarningBehavior behavior = printDiagnostic(warnings, id, fmt, args);
-	switch (warnings.getWarningBehavior(id)) {
+	va_end(args);
-	case WarningBehavior::DISABLED:
-		break;
-
-	case WarningBehavior::ENABLED:
-		style_Set(stderr, STYLE_YELLOW, true);
-		fputs("warning: ", stderr);
-		style_Reset(stderr);
-		va_start(ap, fmt);
-		vfprintf(stderr, fmt, ap);
-		va_end(ap);
-		style_Set(stderr, STYLE_YELLOW, true);
-		fprintf(stderr, " [-W%s]\n", flag);
-		style_Reset(stderr);
-		break;
-
-	case WarningBehavior::ERROR:
-		style_Set(stderr, STYLE_RED, true);
-		fputs("error: ", stderr);
-		style_Reset(stderr);
-		va_start(ap, fmt);
-		vfprintf(stderr, fmt, ap);
-		va_end(ap);
-		style_Set(stderr, STYLE_RED, true);
-		fprintf(stderr, " [-Werror=%s]\n", flag);
-		style_Reset(stderr);
 
+	if (behavior == WarningBehavior::ERROR) {
 		warnings.incrementErrors();
-		break;
 	}
 }

src/link/warning.cpp

@@ -39,32 +39,6 @@ Diagnostics<WarningLevel, WarningID> warnings = {
 };
 // clang-format on
 
-static void printDiag(
-    FileStackNode const *src,
-    uint32_t lineNo,
-    char const *fmt,
-    va_list args,
-    char const *type,
-    StyleColor color,
-    char const *flagfmt,
-    char const *flag
-) {
-	style_Set(stderr, color, true);
-	fprintf(stderr, "%s: ", type);
-	style_Reset(stderr);
-	vfprintf(stderr, fmt, args);
-	if (flagfmt) {
-		style_Set(stderr, color, true);
-		putc(' ', stderr);
-		fprintf(stderr, flagfmt, flag);
-	}
-	putc('\n', stderr);
-
-	if (src) {
-		src->printBacktrace(lineNo);
-	}
-}
-
 [[noreturn]]
 static void abortLinking(char const *verb) {
 	style_Set(stderr, STYLE_RED, true);
@@ -82,30 +56,36 @@ static void abortLinking(char const *verb) {
 void warning(FileStackNode const *src, uint32_t lineNo, char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(src, lineNo, fmt, args, "warning", STYLE_YELLOW, nullptr, nullptr);
+	vwarnx(fmt, args);
 	va_end(args);
+	if (src) {
+		src->printBacktrace(lineNo);
+	}
 }
 
 void warning(char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(nullptr, 0, fmt, args, "warning", STYLE_YELLOW, nullptr, nullptr);
+	vwarnx(fmt, args);
 	va_end(args);
 }
 
 void error(FileStackNode const *src, uint32_t lineNo, char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(src, lineNo, fmt, args, "error", STYLE_RED, nullptr, nullptr);
+	verrorx(fmt, args);
 	va_end(args);
 
+	if (src) {
+		src->printBacktrace(lineNo);
+	}
 	warnings.incrementErrors();
 }
 
 void error(char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(nullptr, 0, fmt, args, "error", STYLE_RED, nullptr, nullptr);
+	verrorx(fmt, args);
 	va_end(args);
 
 	warnings.incrementErrors();
@@ -114,11 +94,10 @@ void error(char const *fmt, ...) {
 void scriptError(char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(nullptr, 0, fmt, args, "error", STYLE_RED, nullptr, nullptr);
+	verrorx(fmt, args);
 	va_end(args);
 
 	lexer_TraceCurrent();
-
 	warnings.incrementErrors();
 }
 
@@ -126,9 +105,12 @@ void scriptError(char const *fmt, ...) {
 void fatal(FileStackNode const *src, uint32_t lineNo, char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(src, lineNo, fmt, args, "FATAL", STYLE_RED, nullptr, nullptr);
+	vfatalx(fmt, args);
 	va_end(args);
 
+	if (src) {
+		src->printBacktrace(lineNo);
+	}
 	warnings.incrementErrors();
 	abortLinking(nullptr);
 }
@@ -137,7 +119,7 @@ void fatal(FileStackNode const *src, uint32_t lineNo, char const *fmt, ...) {
 void fatal(char const *fmt, ...) {
 	va_list args;
 	va_start(args, fmt);
-	printDiag(nullptr, 0, fmt, args, "FATAL", STYLE_RED, nullptr, nullptr);
+	vfatalx(fmt, args);
 	va_end(args);
 
 	warnings.incrementErrors();
@@ -154,18 +136,13 @@ void fatalTwo(
     ...
 ) {
 	va_list args;
-	style_Set(stderr, STYLE_RED, true);
-	fputs("FATAL: ", stderr);
-	style_Reset(stderr);
 	va_start(args, fmt);
-	vfprintf(stderr, fmt, args);
+	vfatalx(fmt, args);
 	va_end(args);
-	putc('\n', stderr);
 
 	src1.printBacktrace(lineNo1);
 	fputs("    and also:\n", stderr);
 	src2.printBacktrace(lineNo2);
-
 	warnings.incrementErrors();
 	abortLinking(nullptr);
 }
@@ -177,53 +154,31 @@ void requireZeroErrors() {
 }
 
 void warning(FileStackNode const *src, uint32_t lineNo, WarningID id, char const *fmt, ...) {
-	char const *flag = warnings.warningFlags[id].name;
 	va_list args;
-
 	va_start(args, fmt);
-
+	WarningBehavior behavior = printDiagnostic(warnings, id, fmt, args);
-	switch (warnings.getWarningBehavior(id)) {
-	case WarningBehavior::DISABLED:
-		break;
-
-	case WarningBehavior::ENABLED:
-		printDiag(src, lineNo, fmt, args, "warning", STYLE_YELLOW, "[-W%s]", flag);
-		break;
-
-	case WarningBehavior::ERROR:
-		printDiag(src, lineNo, fmt, args, "error", STYLE_RED, "[-Werror=%s]", flag);
-
-		warnings.incrementErrors();
-		break;
-	}
-
 	va_end(args);
+
+	if (behavior != WarningBehavior::DISABLED) {
+		if (src) {
+			src->printBacktrace(lineNo);
+		}
+		if (behavior == WarningBehavior::ERROR) {
+			warnings.incrementErrors();
+		}
+	}
 }
 
 void scriptWarning(WarningID id, char const *fmt, ...) {
-	char const *flag = warnings.warningFlags[id].name;
 	va_list args;
-
 	va_start(args, fmt);
-
+	WarningBehavior behavior = printDiagnostic(warnings, id, fmt, args);
-	switch (warnings.getWarningBehavior(id)) {
-	case WarningBehavior::DISABLED:
-		break;
-
-	case WarningBehavior::ENABLED:
-		printDiag(nullptr, 0, fmt, args, "warning", STYLE_YELLOW, "[-W%s]", flag);
-
-		lexer_TraceCurrent();
-		break;
-
-	case WarningBehavior::ERROR:
-		printDiag(nullptr, 0, fmt, args, "error", STYLE_RED, "[-Werror=%s]", flag);
-
-		lexer_TraceCurrent();
-		warnings.incrementErrors();
-		break;
-	}
-
 	va_end(args);
 
+	if (behavior != WarningBehavior::DISABLED) {
+		lexer_TraceCurrent();
+		if (behavior == WarningBehavior::ERROR) {
+			warnings.incrementErrors();
+		}
+	}
 }

src/usage.cpp

@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include "diagnostics.hpp" // vfatalx
 #include "helpers.hpp"
 #include "platform.hpp"
 #include "style.hpp"
@@ -156,13 +157,9 @@ void Usage::printAndExit(int code) const {
 
 void Usage::printAndExit(char const *fmt, ...) const {
 	va_list args;
-	style_Set(stderr, STYLE_RED, true);
-	fputs("FATAL: ", stderr);
-	style_Reset(stderr);
 	va_start(args, fmt);
-	vfprintf(stderr, fmt, args);
+	vfatalx(fmt, args);
 	va_end(args);
-	putc('\n', stderr);
 
 	printAndExit(1);
 }