Add a little more test coverage (#1805)

Format main.cpp files more consistently

Add `make format` to run clang-format on everything

Makefile

@@ -3,7 +3,7 @@
 .SUFFIXES:
 .SUFFIXES: .cpp .y .o
 
-.PHONY: all clean install checkdiff develop debug profile coverage tidy iwyu mingw32 mingw64 wine-shim dist
+.PHONY: all clean install checkdiff develop debug profile coverage format tidy iwyu mingw32 mingw64 wine-shim dist
 
 # User-defined variables
 
@@ -241,6 +241,10 @@ coverage:
 	$Qenv ${MAKE} \
 		CXXFLAGS="-ggdb3 -Og --coverage -fno-omit-frame-pointer -fno-optimize-sibling-calls"
 
+# Target used in development to format source code with clang-format.
+format:
+	$Qclang-format -i include/**/*.hpp src/**/*.cpp
+
 # Target used in development to check code with clang-tidy.
 # Requires Bison-generated header files to exist.
 tidy: src/asm/parser.hpp src/link/script.hpp

src/asm/actions.cpp

@@ -139,18 +139,22 @@ std::optional<std::string> act_ReadFile(std::string const &name, uint32_t maxLen
 			readSize = fileSize;
 		}
 		fseek(file, 0, SEEK_SET);
+		// LCOV_EXCL_START
 	} else if (errno != ESPIPE) {
 		error(
 		    "Error determining size of `READFILE` file \"%s\": %s", name.c_str(), strerror(errno)
 		);
+		// LCOV_EXCL_STOP
 	}
 
 	std::string contents;
 	contents.resize(readSize);
 
 	if (fread(&contents[0], 1, readSize, file) < readSize || ferror(file)) {
+		// LCOV_EXCL_START
 		error("Error reading `READFILE` file \"%s\": %s", name.c_str(), strerror(errno));
 		return "";
+		// LCOV_EXCL_STOP
 	}
 
 	return contents;

src/asm/main.cpp

@@ -300,10 +300,9 @@ int main(int argc, char *argv[]) {
 		options.maxErrors = 100;
 	}
 
+	// Parse CLI options
 	for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) {
 		switch (ch) {
-			char *endptr;
-
 		case 'B':
 			if (!trace_ParseTraceDepth(musl_optarg)) {
 				fatal("Invalid argument for option '-B'");
@@ -318,9 +317,8 @@ int main(int argc, char *argv[]) {
 			}
 			break;
 
-			char *equals;
-		case 'D':
-			equals = strchr(musl_optarg, '=');
+		case 'D': {
+			char *equals = strchr(musl_optarg, '=');
 			if (equals) {
 				*equals = '\0';
 				sym_AddString(musl_optarg, std::make_shared<std::string>(equals + 1));
@@ -328,6 +326,7 @@ int main(int argc, char *argv[]) {
 				sym_AddString(musl_optarg, std::make_shared<std::string>("1"));
 			}
 			break;
+		}
 
 		case 'E':
 			options.exportAll = true;
@@ -341,8 +340,10 @@ int main(int argc, char *argv[]) {
 			}
 			break;
 
+			// LCOV_EXCL_START
 		case 'h':
-			usage.printAndExit(0); // LCOV_EXCL_LINE
+			usage.printAndExit(0);
+			// LCOV_EXCL_STOP
 
 		case 'I':
 			fstk_AddIncludePath(musl_optarg);
@@ -369,9 +370,9 @@ int main(int argc, char *argv[]) {
 			fstk_AddPreIncludeFile(musl_optarg);
 			break;
 
-			unsigned long padByte;
-		case 'p':
-			padByte = strtoul(musl_optarg, &endptr, 0);
+		case 'p': {
+			char *endptr;
+			unsigned long padByte = strtoul(musl_optarg, &endptr, 0);
 
 			if (musl_optarg[0] == '\0' || *endptr != '\0') {
 				fatal("Invalid argument for option '-p'");
@@ -383,12 +384,14 @@ int main(int argc, char *argv[]) {
 
 			opt_P(padByte);
 			break;
+		}
 
 		case 'Q': {
 			char const *precisionArg = musl_optarg;
 			if (precisionArg[0] == '.') {
 				++precisionArg;
 			}
+			char *endptr;
 			unsigned long precision = strtoul(precisionArg, &endptr, 0);
 
 			if (musl_optarg[0] == '\0' || *endptr != '\0') {
@@ -403,13 +406,15 @@ int main(int argc, char *argv[]) {
 			break;
 		}
 
-		case 'r':
+		case 'r': {
+			char *endptr;
 			options.maxRecursionDepth = strtoul(musl_optarg, &endptr, 0);
 
 			if (musl_optarg[0] == '\0' || *endptr != '\0') {
 				fatal("Invalid argument for option '-r'");
 			}
 			break;
+		}
 
 		case 's': {
 			// Split "<features>:<name>" so `musl_optarg` is "<features>" and `name` is "<name>"
@@ -428,12 +433,12 @@ int main(int argc, char *argv[]) {
 			break;
 		}
 
+			// LCOV_EXCL_START
 		case 'V':
 			printf("rgbasm %s\n", get_package_version_string());
 			exit(0);
 
 		case 'v':
-			// LCOV_EXCL_START
 			incrementVerbosity();
 			break;
 			// LCOV_EXCL_STOP
@@ -447,6 +452,7 @@ int main(int argc, char *argv[]) {
 			break;
 
 		case 'X': {
+			char *endptr;
 			uint64_t maxErrors = strtoul(musl_optarg, &endptr, 0);
 
 			if (musl_optarg[0] == '\0' || *endptr != '\0') {
@@ -461,8 +467,7 @@ int main(int argc, char *argv[]) {
 			break;
 		}
 
-		// Long-only options
-		case 0:
+		case 0: // Long-only options
 			switch (longOpt) {
 			case 'c':
 				if (!style_Parse(musl_optarg)) {

src/asm/section.cpp

@@ -941,9 +941,11 @@ bool sect_BinaryFile(std::string const &name, uint32_t startPos) {
 		fseek(file, startPos, SEEK_SET);
 	} else {
 		if (errno != ESPIPE) {
+			// LCOV_EXCL_START
 			error(
 			    "Error determining size of `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno)
 			);
+			// LCOV_EXCL_STOP
 		}
 		// The file isn't seekable, so we'll just skip bytes one at a time
 		while (startPos--) {
@@ -961,7 +963,9 @@ bool sect_BinaryFile(std::string const &name, uint32_t startPos) {
 	}
 
 	if (ferror(file)) {
+		// LCOV_EXCL_START
 		error("Error reading `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno));
+		// LCOV_EXCL_STOP
 	}
 	return false;
 }
@@ -1002,9 +1006,11 @@ bool sect_BinaryFileSlice(std::string const &name, uint32_t startPos, uint32_t l
 		fseek(file, startPos, SEEK_SET);
 	} else {
 		if (errno != ESPIPE) {
+			// LCOV_EXCL_START
 			error(
 			    "Error determining size of `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno)
 			);
+			// LCOV_EXCL_STOP
 		}
 		// The file isn't seekable, so we'll just skip bytes one at a time
 		while (startPos--) {
@@ -1021,7 +1027,9 @@ bool sect_BinaryFileSlice(std::string const &name, uint32_t startPos, uint32_t l
 		if (int byte = fgetc(file); byte != EOF) {
 			writeByte(byte);
 		} else if (ferror(file)) {
+			// LCOV_EXCL_START
 			error("Error reading `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno));
+			// LCOV_EXCL_STOP
 		} else {
 			error(
 			    "Premature end of `INCBIN` file \"%s\" (%" PRId32 " bytes left to read)",

src/fix/main.cpp

@@ -683,10 +683,10 @@ static void initLogo() {
 
 int main(int argc, char *argv[]) {
 	char const *outputFilename = nullptr;
+
+	// Parse CLI options
 	for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) {
 		switch (ch) {
-			size_t len;
-
 		case 'C':
 		case 'c':
 			model = ch == 'c' ? BOTH : CGB;
@@ -723,12 +723,14 @@ int main(int argc, char *argv[]) {
 			}
 			break;
 
+			// LCOV_EXCL_START
 		case 'h':
-			usage.printAndExit(0); // LCOV_EXCL_LINE
+			usage.printAndExit(0);
+			// LCOV_EXCL_STOP
 
-		case 'i':
+		case 'i': {
 			gameID = musl_optarg;
-			len = strlen(gameID);
+			size_t len = strlen(gameID);
 			if (len > 4) {
 				len = 4;
 				warning(WARNING_TRUNCATION, "Truncating game ID \"%s\" to 4 chars", gameID);
@@ -739,14 +741,15 @@ int main(int argc, char *argv[]) {
 				warning(WARNING_TRUNCATION, "Truncating title \"%s\" to 11 chars", title);
 			}
 			break;
+		}
 
 		case 'j':
 			japanese = false;
 			break;
 
-		case 'k':
+		case 'k': {
 			newLicensee = musl_optarg;
-			len = strlen(newLicensee);
+			size_t len = strlen(newLicensee);
 			if (len > 2) {
 				len = 2;
 				warning(
@@ -755,6 +758,7 @@ int main(int argc, char *argv[]) {
 			}
 			newLicenseeLen = len;
 			break;
+		}
 
 		case 'L':
 			logoFilename = musl_optarg;
@@ -800,7 +804,7 @@ int main(int argc, char *argv[]) {
 
 		case 't': {
 			title = musl_optarg;
-			len = strlen(title);
+			size_t len = strlen(title);
 			uint8_t maxLen = maxTitleLen();
 
 			if (len > maxLen) {
@@ -811,15 +815,15 @@ int main(int argc, char *argv[]) {
 			break;
 		}
 
-		case 'V':
 			// LCOV_EXCL_START
+		case 'V':
 			printf("rgbfix %s\n", get_package_version_string());
 			exit(0);
-			// LCOV_EXCL_STOP
 
 		case 'v':
 			fixSpec = FIX_LOGO | FIX_HEADER_SUM | FIX_GLOBAL_SUM;
 			break;
+			// LCOV_EXCL_STOP
 
 		case 'W':
 			warnings.processWarningFlag(musl_optarg);
@@ -829,8 +833,7 @@ int main(int argc, char *argv[]) {
 			warnings.state.warningsEnabled = false;
 			break;
 
-		// Long-only options
-		case 0:
+		case 0: // Long-only options
 			if (longOpt == 'c' && !style_Parse(musl_optarg)) {
 				fatal("Invalid argument for option '--color'");
 			}

src/gfx/main.cpp

@@ -259,11 +259,12 @@ static std::vector<size_t> readAtFile(std::string const &path, std::vector<char>
 static char *parseArgv(int argc, char *argv[]) {
 	for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) {
 		char *arg = musl_optarg; // Make a copy for scanning
-		uint16_t number;
+
 		switch (ch) {
 		case 'A':
 			localOptions.autoAttrmap = true;
 			break;
+
 		case 'a':
 			localOptions.autoAttrmap = false;
 			if (!options.attrmap.empty()) {
@@ -271,11 +272,13 @@ static char *parseArgv(int argc, char *argv[]) {
 			}
 			options.attrmap = musl_optarg;
 			break;
+
 		case 'B':
 			parseBackgroundPalSpec(musl_optarg);
 			break;
-		case 'b':
-			number = parseNumber(arg, "Bank 0 base tile ID", 0);
+
+		case 'b': {
+			uint16_t number = parseNumber(arg, "Bank 0 base tile ID", 0);
 			if (number >= 256) {
 				error("Bank 0 base tile ID must be below 256");
 			} else {
@@ -309,9 +312,12 @@ static char *parseArgv(int argc, char *argv[]) {
 				break;
 			}
 			break;
+		}
+
 		case 'C':
 			options.useColorCurve = true;
 			break;
+
 		case 'c':
 			localOptions.externalPalSpec = nullptr; // Allow overriding a previous pal spec
 			if (musl_optarg[0] == '#') {
@@ -333,6 +339,7 @@ static char *parseArgv(int argc, char *argv[]) {
 				localOptions.externalPalSpec = musl_optarg;
 			}
 			break;
+
 		case 'd':
 			options.bitDepth = parseNumber(arg, "Bit depth", 2);
 			if (*arg != '\0') {
@@ -342,14 +349,19 @@ static char *parseArgv(int argc, char *argv[]) {
 				options.bitDepth = 2;
 			}
 			break;
+
+			// LCOV_EXCL_START
 		case 'h':
-			usage.printAndExit(0); // LCOV_EXCL_LINE
+			usage.printAndExit(0);
+			// LCOV_EXCL_STOP
+
 		case 'i':
 			if (!options.inputTileset.empty()) {
 				warnx("Overriding input tileset file \"%s\"", options.inputTileset.c_str());
 			}
 			options.inputTileset = musl_optarg;
 			break;
+
 		case 'L':
 			options.inputSlice.left = parseNumber(arg, "Input slice left coordinate");
 			if (options.inputSlice.left > INT16_MAX) {
@@ -390,8 +402,9 @@ static char *parseArgv(int argc, char *argv[]) {
 				error("Unexpected extra characters after slice spec in \"%s\"", musl_optarg);
 			}
 			break;
-		case 'l':
-			number = parseNumber(arg, "Base palette ID", 0);
+
+		case 'l': {
+			uint16_t number = parseNumber(arg, "Base palette ID", 0);
 			if (*arg != '\0') {
 				error("Base palette ID must be a valid number, not \"%s\"", musl_optarg);
 			} else if (number >= 256) {
@@ -400,13 +413,17 @@ static char *parseArgv(int argc, char *argv[]) {
 				options.basePalID = number;
 			}
 			break;
+		}
+
 		case 'm':
 			options.allowMirroringX = true; // Imply `-X`
 			options.allowMirroringY = true; // Imply `-Y`
 			[[fallthrough]];                // Imply `-u`
+
 		case 'u':
 			options.allowDedup = true;
 			break;
+
 		case 'N':
 			options.maxNbTiles[0] = parseNumber(arg, "Number of tiles in bank 0", 256);
 			if (options.maxNbTiles[0] > 256) {
@@ -438,8 +455,9 @@ static char *parseArgv(int argc, char *argv[]) {
 				break;
 			}
 			break;
-		case 'n':
-			number = parseNumber(arg, "Number of palettes", 256);
+
+		case 'n': {
+			uint16_t number = parseNumber(arg, "Number of palettes", 256);
 			if (*arg != '\0') {
 				error("Number of palettes ('-n') must be a valid number, not \"%s\"", musl_optarg);
 			}
@@ -451,18 +469,23 @@ static char *parseArgv(int argc, char *argv[]) {
 				options.nbPalettes = number;
 			}
 			break;
+		}
+
 		case 'O':
 			localOptions.groupOutputs = true;
 			break;
+
 		case 'o':
 			if (!options.output.empty()) {
 				warnx("Overriding tile data file %s", options.output.c_str());
 			}
 			options.output = musl_optarg;
 			break;
+
 		case 'P':
 			localOptions.autoPalettes = true;
 			break;
+
 		case 'p':
 			localOptions.autoPalettes = false;
 			if (!options.palettes.empty()) {
@@ -470,9 +493,11 @@ static char *parseArgv(int argc, char *argv[]) {
 			}
 			options.palettes = musl_optarg;
 			break;
+
 		case 'Q':
 			localOptions.autoPalmap = true;
 			break;
+
 		case 'q':
 			localOptions.autoPalmap = false;
 			if (!options.palmap.empty()) {
@@ -480,6 +505,7 @@ static char *parseArgv(int argc, char *argv[]) {
 			}
 			options.palmap = musl_optarg;
 			break;
+
 		case 'r':
 			localOptions.reverse = true;
 			options.reversedWidth = parseNumber(arg, "Reversed image stride");
@@ -489,6 +515,7 @@ static char *parseArgv(int argc, char *argv[]) {
 				);
 			}
 			break;
+
 		case 's':
 			options.nbColorsPerPal = parseNumber(arg, "Number of colors per palette", 4);
 			if (*arg != '\0') {
@@ -500,9 +527,11 @@ static char *parseArgv(int argc, char *argv[]) {
 				error("Palette size ('-s') may not be 0!");
 			}
 			break;
+
 		case 'T':
 			localOptions.autoTilemap = true;
 			break;
+
 		case 't':
 			localOptions.autoTilemap = false;
 			if (!options.tilemap.empty()) {
@@ -510,44 +539,52 @@ static char *parseArgv(int argc, char *argv[]) {
 			}
 			options.tilemap = musl_optarg;
 			break;
-		case 'V':
+
 			// LCOV_EXCL_START
+		case 'V':
 			printf("rgbgfx %s\n", get_package_version_string());
 			exit(0);
-			// LCOV_EXCL_STOP
+
 		case 'v':
-			// LCOV_EXCL_START
 			incrementVerbosity();
 			break;
 			// LCOV_EXCL_STOP
+
 		case 'W':
 			warnings.processWarningFlag(musl_optarg);
 			break;
+
 		case 'w':
 			warnings.state.warningsEnabled = false;
 			break;
+
 		case 'x':
 			options.trim = parseNumber(arg, "Number of tiles to trim", 0);
 			if (*arg != '\0') {
 				error("Tile trim ('-x') argument must be a valid number, not \"%s\"", musl_optarg);
 			}
 			break;
+
 		case 'X':
 			options.allowMirroringX = true;
 			options.allowDedup = true; // Imply `-u`
 			break;
+
 		case 'Y':
 			options.allowMirroringY = true;
 			options.allowDedup = true; // Imply `-u`
 			break;
+
 		case 'Z':
 			options.columnMajor = true;
 			break;
+
 		case 0: // Long-only options
 			if (longOpt == 'c' && !style_Parse(musl_optarg)) {
 				fatal("Invalid argument for option '--color'");
 			}
 			break;
+
 		case 1: // Positional argument, requested by leading `-` in opt string
 			if (musl_optarg[0] == '@') {
 				// Instruct the caller to process that at-file
@@ -556,6 +593,7 @@ static char *parseArgv(int argc, char *argv[]) {
 				registerInput(musl_optarg);
 			}
 			break;
+
 		default:
 			usage.printAndExit(1); // LCOV_EXCL_LINE
 		}
@@ -738,6 +776,7 @@ int main(int argc, char *argv[]) {
 	};
 	std::vector<AtFileStackEntry> atFileStack;
 
+	// Parse CLI options
 	int curArgc = argc;
 	char **curArgv = argv;
 	std::vector<std::vector<char>> argPools;

src/link/main.cpp

@@ -297,7 +297,7 @@ static void parseScrambleSpec(char *spec) {
 }
 
 int main(int argc, char *argv[]) {
-	// Parse options
+	// Parse CLI options
 	for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) {
 		switch (ch) {
 		case 'B':
@@ -305,45 +305,56 @@ int main(int argc, char *argv[]) {
 				fatal("Invalid argument for option '-B'");
 			}
 			break;
+
 		case 'd':
 			options.isDmgMode = true;
 			options.isWRAM0Mode = true;
 			break;
+
+			// LCOV_EXCL_START
 		case 'h':
-			usage.printAndExit(0); // LCOV_EXCL_LINE
+			usage.printAndExit(0);
+			// LCOV_EXCL_STOP
+
 		case 'l':
 			if (linkerScriptName) {
 				warnx("Overriding linker script file \"%s\"", linkerScriptName);
 			}
 			linkerScriptName = musl_optarg;
 			break;
+
 		case 'M':
 			options.noSymInMap = true;
 			break;
+
 		case 'm':
 			if (options.mapFileName) {
 				warnx("Overriding map file \"%s\"", options.mapFileName);
 			}
 			options.mapFileName = musl_optarg;
 			break;
+
 		case 'n':
 			if (options.symFileName) {
 				warnx("Overriding sym file \"%s\"", options.symFileName);
 			}
 			options.symFileName = musl_optarg;
 			break;
+
 		case 'O':
 			if (options.overlayFileName) {
 				warnx("Overriding overlay file \"%s\"", options.overlayFileName);
 			}
 			options.overlayFileName = musl_optarg;
 			break;
+
 		case 'o':
 			if (options.outputFileName) {
 				warnx("Overriding output file \"%s\"", options.outputFileName);
 			}
 			options.outputFileName = musl_optarg;
 			break;
+
 		case 'p': {
 			char *endptr;
 			unsigned long value = strtoul(musl_optarg, &endptr, 0);
@@ -359,38 +370,45 @@ int main(int argc, char *argv[]) {
 			options.hasPadValue = true;
 			break;
 		}
+
 		case 'S':
 			parseScrambleSpec(musl_optarg);
 			break;
+
 		case 't':
 			options.is32kMode = true;
 			break;
-		case 'V':
+
 			// LCOV_EXCL_START
+		case 'V':
 			printf("rgblink %s\n", get_package_version_string());
 			exit(0);
-			// LCOV_EXCL_STOP
+
 		case 'v':
-			// LCOV_EXCL_START
 			incrementVerbosity();
 			break;
 			// LCOV_EXCL_STOP
+
 		case 'W':
 			warnings.processWarningFlag(musl_optarg);
 			break;
+
 		case 'w':
 			options.isWRAM0Mode = true;
 			break;
+
 		case 'x':
 			options.disablePadding = true;
 			// implies tiny mode
 			options.is32kMode = true;
 			break;
+
 		case 0: // Long-only options
 			if (longOpt == 'c' && !style_Parse(musl_optarg)) {
 				fatal("Invalid argument for option '--color'");
 			}
 			break;
+
 		default:
 			usage.printAndExit(1); // LCOV_EXCL_LINE
 		}

test/asm/empty-state.flags

@@ -0,0 +1 @@
+-s all:-

test/asm/empty-state.out

@@ -0,0 +1,16 @@
+; File generated by rgbasm
+
+; Numeric constants
+; No values
+
+; Variables
+; No values
+
+; String constants
+; No values
+
+; Character maps
+newcharmap main
+
+; Macros
+; No values