Refactor to reduce nesting depth some more (#1740)

src/asm/lexer.cpp

@@ -1735,6 +1735,23 @@ static bool isGarbageCharacter(int c) {
 	       && (c == '\0' || !strchr("; \t~[](),+-*/|^=!<>:&%`\"\r\n\\", c));
 }
 
+static void reportGarbageCharacters(int c) {
+	// '#' can be garbage if it doesn't start a raw string or identifier
+	assume(isGarbageCharacter(c) || c == '#');
+	if (isGarbageCharacter(peek())) {
+		// At least two characters are garbage; group them into one error report
+		std::string garbage = printChar(c);
+		while (isGarbageCharacter(peek())) {
+			c = nextChar();
+			garbage += ", ";
+			garbage += printChar(c);
+		}
+		error("Unknown characters %s", garbage.c_str());
+	} else {
+		error("Unknown character %s", printChar(c));
+	}
+}
+
 static Token yylex_NORMAL() {
 	if (int nextToken = lexerState->nextToken; nextToken) {
 		lexerState->nextToken = 0;
@@ -2083,19 +2100,7 @@ static Token yylex_NORMAL() {
 
 			// Do not report weird characters when capturing, it'll be done later
 			if (!lexerState->capturing) {
-				assume(isGarbageCharacter(c) || c == '#');
+				reportGarbageCharacters(c);
-				if (isGarbageCharacter(peek())) {
-					// At least two characters are garbage; group them into one error report
-					std::string garbage = printChar(c);
-					while (isGarbageCharacter(peek())) {
-						c = nextChar();
-						garbage += ", ";
-						garbage += printChar(c);
-					}
-					error("Unknown characters %s", garbage.c_str());
-				} else {
-					error("Unknown character %s", printChar(c));
-				}
 			}
 		}
 		lexerState->atLineStart = false;

src/asm/main.cpp

@@ -124,6 +124,51 @@ static void fatalWithUsage(char const *fmt, ...) {
 	exit(1);
 }
 
+// Parse a comma-separated string of '-s/--state' features
+static std::vector<StateFeature> parseStateFeatures(char *str) {
+	std::vector<StateFeature> features;
+	for (char *feature = str; feature;) {
+		// Split "<feature>,<rest>" so `feature` is "<feature>" and `next` is "<rest>"
+		char *next = strchr(feature, ',');
+		if (next) {
+			*next++ = '\0';
+		}
+		// Trim whitespace from the beginning of `feature`...
+		feature += strspn(feature, " \t");
+		// ...and from the end
+		if (char *end = strpbrk(feature, " \t"); end) {
+			*end = '\0';
+		}
+		// A feature must be specified
+		if (*feature == '\0') {
+			fatal("Empty feature for option 's'");
+		}
+		// Parse the `feature` and update the `features` list
+		if (!strcasecmp(feature, "all")) {
+			if (!features.empty()) {
+				warnx("Redundant feature before \"%s\" for option 's'", feature);
+			}
+			features.assign({STATE_EQU, STATE_VAR, STATE_EQUS, STATE_CHAR, STATE_MACRO});
+		} else {
+			StateFeature value = !strcasecmp(feature, "equ")     ? STATE_EQU
+			                     : !strcasecmp(feature, "var")   ? STATE_VAR
+			                     : !strcasecmp(feature, "equs")  ? STATE_EQUS
+			                     : !strcasecmp(feature, "char")  ? STATE_CHAR
+			                     : !strcasecmp(feature, "macro") ? STATE_MACRO
+			                                                     : NB_STATE_FEATURES;
+			if (value == NB_STATE_FEATURES) {
+				fatal("Invalid feature for option 's': \"%s\"", feature);
+			} else if (std::find(RANGE(features), value) != features.end()) {
+				warnx("Ignoring duplicate feature for option 's': \"%s\"", feature);
+			} else {
+				features.push_back(value);
+			}
+		}
+		feature = next;
+	}
+	return features;
+}
+
 int main(int argc, char *argv[]) {
 	time_t now = time(nullptr);
 	// Support SOURCE_DATE_EPOCH for reproducible builds
@@ -278,46 +323,7 @@ int main(int argc, char *argv[]) {
 			}
 			*name++ = '\0';
 
-			std::vector<StateFeature> features;
+			std::vector<StateFeature> features = parseStateFeatures(musl_optarg);
-			for (char *feature = musl_optarg; feature;) {
-				// Split "<feature>,<rest>" so `feature` is "<feature>" and `next` is "<rest>"
-				char *next = strchr(feature, ',');
-				if (next) {
-					*next++ = '\0';
-				}
-				// Trim whitespace from the beginning of `feature`...
-				feature += strspn(feature, " \t");
-				// ...and from the end
-				if (char *end = strpbrk(feature, " \t"); end) {
-					*end = '\0';
-				}
-				// A feature must be specified
-				if (*feature == '\0') {
-					fatal("Empty feature for option 's'");
-				}
-				// Parse the `feature` and update the `features` list
-				if (!strcasecmp(feature, "all")) {
-					if (!features.empty()) {
-						warnx("Redundant feature before \"%s\" for option 's'", feature);
-					}
-					features.assign({STATE_EQU, STATE_VAR, STATE_EQUS, STATE_CHAR, STATE_MACRO});
-				} else {
-					StateFeature value = !strcasecmp(feature, "equ")     ? STATE_EQU
-					                     : !strcasecmp(feature, "var")   ? STATE_VAR
-					                     : !strcasecmp(feature, "equs")  ? STATE_EQUS
-					                     : !strcasecmp(feature, "char")  ? STATE_CHAR
-					                     : !strcasecmp(feature, "macro") ? STATE_MACRO
-					                                                     : NB_STATE_FEATURES;
-					if (value == NB_STATE_FEATURES) {
-						fatal("Invalid feature for option 's': \"%s\"", feature);
-					} else if (std::find(RANGE(features), value) != features.end()) {
-						warnx("Ignoring duplicate feature for option 's': \"%s\"", feature);
-					} else {
-						features.push_back(value);
-					}
-				}
-				feature = next;
-			}
 
 			if (stateFileSpecs.find(name) != stateFileSpecs.end()) {
 				warnx("Overriding state filename %s", name);

src/extern/getopt.cpp

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-// This implementation was taken from musl and modified for RGBDS
+// This implementation was taken from musl and modified for RGBDS.
 
 #include "extern/getopt.hpp"
 

src/extern/utf8decoder.cpp

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 
-// UTF-8 decoder: http://bjoern.hoehrmann.de/utf-8/decoder/dfa/
+// This implementation was taken from
+// http://bjoern.hoehrmann.de/utf-8/decoder/dfa/
+// and modified for RGBDS.
 
 #include "extern/utf8decoder.hpp"
 
@@ -33,10 +35,8 @@ static uint8_t const utf8d[] = {
 };
 
 uint32_t decode(uint32_t *state, uint32_t *codep, uint8_t byte) {
-	uint32_t type = utf8d[byte];
+	uint8_t type = utf8d[byte];
-
+	*codep = *state != 0 ? (byte & 0b111111) | (*codep << 6) : byte & (0xFF >> type);
-	*codep = (*state != 0) ? (byte & 0x3FU) | (*codep << 6) : (0xFF >> type) & (byte);
+	*state = utf8d[0x100 + *state * 0x10 + type];
-
-	*state = utf8d[256 + *state * 16 + type];
 	return *state;
 }

src/fix/main.cpp

@@ -461,10 +461,7 @@ static MbcType parseMBC(char const *name) {
 					break;
 				case 'A':
 				case 'a':
-					if (*ptr != 'M' && *ptr != 'm') {
+					tryReadSlice("M");
-						return MBC_BAD;
-					}
-					ptr++;
 					features |= RAM;
 					break;
 				default:

src/gfx/main.cpp

@@ -594,6 +594,143 @@ static char *parseArgv(int argc, char *argv[]) {
 	return nullptr; // Done processing this argv
 }
 
+static void verboseOutputConfig() {
+	fprintf(stderr, "rgbgfx %s\n", get_package_version_string());
+
+	if (options.verbosity >= Options::VERB_VVVVVV) {
+		putc('\n', stderr);
+		// clang-format off: vertically align values
+		static std::array<uint16_t, 21> gfx{
+		    0b0111111110,
+		    0b1111111111,
+		    0b1110011001,
+		    0b1110011001,
+		    0b1111111111,
+		    0b1111111111,
+		    0b1110000001,
+		    0b1111000011,
+		    0b0111111110,
+		    0b0001111000,
+		    0b0111111110,
+		    0b1111111111,
+		    0b1111111111,
+		    0b1111111111,
+		    0b1101111011,
+		    0b1101111011,
+		    0b0011111100,
+		    0b0011001100,
+		    0b0111001110,
+		    0b0111001110,
+		    0b0111001110,
+		};
+		// clang-format on
+		static std::array<char const *, 3> textbox{
+		    "  ,----------------------------------------.",
+		    "  | Augh, dimensional interference again?! |",
+		    "  `----------------------------------------'",
+		};
+		for (size_t i = 0; i < gfx.size(); ++i) {
+			uint16_t row = gfx[i];
+			for (uint8_t _ = 0; _ < 10; ++_) {
+				unsigned char c = row & 1 ? '0' : ' ';
+				putc(c, stderr);
+				// Double the pixel horizontally, otherwise the aspect ratio looks wrong
+				putc(c, stderr);
+				row >>= 1;
+			}
+			if (i < textbox.size()) {
+				fputs(textbox[i], stderr);
+			}
+			putc('\n', stderr);
+		}
+		putc('\n', stderr);
+	}
+
+	fputs("Options:\n", stderr);
+	if (options.columnMajor) {
+		fputs("\tVisit image in column-major order\n", stderr);
+	}
+	if (options.allowDedup) {
+		fputs("\tAllow deduplicating tiles\n", stderr);
+	}
+	if (options.allowMirroringX) {
+		fputs("\tAllow deduplicating horizontally mirrored tiles\n", stderr);
+	}
+	if (options.allowMirroringY) {
+		fputs("\tAllow deduplicating vertically mirrored tiles\n", stderr);
+	}
+	if (options.useColorCurve) {
+		fputs("\tUse color curve\n", stderr);
+	}
+	fprintf(stderr, "\tBit depth: %" PRIu8 "bpp\n", options.bitDepth);
+	if (options.trim != 0) {
+		fprintf(stderr, "\tTrim the last %" PRIu64 " tiles\n", options.trim);
+	}
+	fprintf(stderr, "\tMaximum %" PRIu16 " palettes\n", options.nbPalettes);
+	fprintf(stderr, "\tPalettes contain %" PRIu8 " colors\n", options.nbColorsPerPal);
+	fprintf(stderr, "\t%s palette spec\n", [] {
+		switch (options.palSpecType) {
+		case Options::NO_SPEC:
+			return "No";
+		case Options::EXPLICIT:
+			return "Explicit";
+		case Options::EMBEDDED:
+			return "Embedded";
+		case Options::DMG:
+			return "DMG";
+		}
+		return "???";
+	}());
+	if (options.palSpecType == Options::EXPLICIT) {
+		fputs("\t[\n", stderr);
+		for (auto const &pal : options.palSpec) {
+			fputs("\t\t", stderr);
+			for (auto &color : pal) {
+				if (color) {
+					fprintf(stderr, "#%06x, ", color->toCSS() >> 8);
+				} else {
+					fputs("#none, ", stderr);
+				}
+			}
+			putc('\n', stderr);
+		}
+		fputs("\t]\n", stderr);
+	}
+	fprintf(
+	    stderr,
+	    "\tInput image slice: %" PRIu16 "x%" PRIu16 " pixels starting at (%" PRIu16 ", %" PRIu16
+	    ")\n",
+	    options.inputSlice.width,
+	    options.inputSlice.height,
+	    options.inputSlice.left,
+	    options.inputSlice.top
+	);
+	fprintf(
+	    stderr,
+	    "\tBase tile IDs: [%" PRIu8 ", %" PRIu8 "]\n",
+	    options.baseTileIDs[0],
+	    options.baseTileIDs[1]
+	);
+	fprintf(stderr, "\tBase palette ID: %" PRIu8 "\n", options.basePalID);
+	fprintf(
+	    stderr,
+	    "\tMaximum %" PRIu16 " tiles in bank 0, %" PRIu16 " in bank 1\n",
+	    options.maxNbTiles[0],
+	    options.maxNbTiles[1]
+	);
+	auto printPath = [](char const *name, std::string const &path) {
+		if (!path.empty()) {
+			fprintf(stderr, "\t%s: %s\n", name, path.c_str());
+		}
+	};
+	printPath("Input image", options.input);
+	printPath("Output tile data", options.output);
+	printPath("Output tilemap", options.tilemap);
+	printPath("Output attrmap", options.attrmap);
+	printPath("Output palettes", options.palettes);
+	fputs("Ready.\n", stderr);
+}
+
 int main(int argc, char *argv[]) {
 	struct AtFileStackEntry {
 		int parentInd;            // Saved offset into parent argv
@@ -714,140 +851,7 @@ int main(int argc, char *argv[]) {
 
 	// LCOV_EXCL_START
 	if (options.verbosity >= Options::VERB_CFG) {
-		fprintf(stderr, "rgbgfx %s\n", get_package_version_string());
+		verboseOutputConfig();
-
-		if (options.verbosity >= Options::VERB_VVVVVV) {
-			putc('\n', stderr);
-			// clang-format off: vertically align values
-			static std::array<uint16_t, 21> gfx{
-			    0b0111111110,
-			    0b1111111111,
-			    0b1110011001,
-			    0b1110011001,
-			    0b1111111111,
-			    0b1111111111,
-			    0b1110000001,
-			    0b1111000011,
-			    0b0111111110,
-			    0b0001111000,
-			    0b0111111110,
-			    0b1111111111,
-			    0b1111111111,
-			    0b1111111111,
-			    0b1101111011,
-			    0b1101111011,
-			    0b0011111100,
-			    0b0011001100,
-			    0b0111001110,
-			    0b0111001110,
-			    0b0111001110,
-			};
-			// clang-format on
-			static std::array<char const *, 3> textbox{
-			    "  ,----------------------------------------.",
-			    "  | Augh, dimensional interference again?! |",
-			    "  `----------------------------------------'",
-			};
-			for (size_t i = 0; i < gfx.size(); ++i) {
-				uint16_t row = gfx[i];
-				for (uint8_t _ = 0; _ < 10; ++_) {
-					unsigned char c = row & 1 ? '0' : ' ';
-					putc(c, stderr);
-					// Double the pixel horizontally, otherwise the aspect ratio looks wrong
-					putc(c, stderr);
-					row >>= 1;
-				}
-				if (i < textbox.size()) {
-					fputs(textbox[i], stderr);
-				}
-				putc('\n', stderr);
-			}
-			putc('\n', stderr);
-		}
-
-		fputs("Options:\n", stderr);
-		if (options.columnMajor) {
-			fputs("\tVisit image in column-major order\n", stderr);
-		}
-		if (options.allowDedup) {
-			fputs("\tAllow deduplicating tiles\n", stderr);
-		}
-		if (options.allowMirroringX) {
-			fputs("\tAllow deduplicating horizontally mirrored tiles\n", stderr);
-		}
-		if (options.allowMirroringY) {
-			fputs("\tAllow deduplicating vertically mirrored tiles\n", stderr);
-		}
-		if (options.useColorCurve) {
-			fputs("\tUse color curve\n", stderr);
-		}
-		fprintf(stderr, "\tBit depth: %" PRIu8 "bpp\n", options.bitDepth);
-		if (options.trim != 0) {
-			fprintf(stderr, "\tTrim the last %" PRIu64 " tiles\n", options.trim);
-		}
-		fprintf(stderr, "\tMaximum %" PRIu16 " palettes\n", options.nbPalettes);
-		fprintf(stderr, "\tPalettes contain %" PRIu8 " colors\n", options.nbColorsPerPal);
-		fprintf(stderr, "\t%s palette spec\n", [] {
-			switch (options.palSpecType) {
-			case Options::NO_SPEC:
-				return "No";
-			case Options::EXPLICIT:
-				return "Explicit";
-			case Options::EMBEDDED:
-				return "Embedded";
-			case Options::DMG:
-				return "DMG";
-			}
-			return "???";
-		}());
-		if (options.palSpecType == Options::EXPLICIT) {
-			fputs("\t[\n", stderr);
-			for (auto const &pal : options.palSpec) {
-				fputs("\t\t", stderr);
-				for (auto &color : pal) {
-					if (color) {
-						fprintf(stderr, "#%06x, ", color->toCSS() >> 8);
-					} else {
-						fputs("#none, ", stderr);
-					}
-				}
-				putc('\n', stderr);
-			}
-			fputs("\t]\n", stderr);
-		}
-		fprintf(
-		    stderr,
-		    "\tInput image slice: %" PRIu16 "x%" PRIu16 " pixels starting at (%" PRIu16 ", %" PRIu16
-		    ")\n",
-		    options.inputSlice.width,
-		    options.inputSlice.height,
-		    options.inputSlice.left,
-		    options.inputSlice.top
-		);
-		fprintf(
-		    stderr,
-		    "\tBase tile IDs: [%" PRIu8 ", %" PRIu8 "]\n",
-		    options.baseTileIDs[0],
-		    options.baseTileIDs[1]
-		);
-		fprintf(stderr, "\tBase palette ID: %" PRIu8 "\n", options.basePalID);
-		fprintf(
-		    stderr,
-		    "\tMaximum %" PRIu16 " tiles in bank 0, %" PRIu16 " in bank 1\n",
-		    options.maxNbTiles[0],
-		    options.maxNbTiles[1]
-		);
-		auto printPath = [](char const *name, std::string const &path) {
-			if (!path.empty()) {
-				fprintf(stderr, "\t%s: %s\n", name, path.c_str());
-			}
-		};
-		printPath("Input image", options.input);
-		printPath("Output tile data", options.output);
-		printPath("Output tilemap", options.tilemap);
-		printPath("Output attrmap", options.attrmap);
-		printPath("Output palettes", options.palettes);
-		fputs("Ready.\n", stderr);
 	}
 	// LCOV_EXCL_STOP
 

src/link/assign.cpp

@@ -122,49 +122,49 @@ static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
 
 		if (spaceIdx < bankMem.size()) {
 			location.address = bankMem[spaceIdx].address;
+		}
 
-			// Process locations in that bank
+		// Process locations in that bank
-			while (spaceIdx < bankMem.size()) {
+		while (spaceIdx < bankMem.size()) {
-				// If that location is OK, return it
+			// If that location is OK, return it
-				if (isLocationSuitable(section, bankMem[spaceIdx], location)) {
+			if (isLocationSuitable(section, bankMem[spaceIdx], location)) {
-					return spaceIdx;
+				return spaceIdx;
-				}
-
-				// Go to the next *possible* location
-				if (section.isAddressFixed) {
-					// If the address is fixed, there can be only
-					// one candidate block per bank; if we already
-					// reached it, give up.
-					if (location.address < section.org) {
-						location.address = section.org;
-					} else {
-						break; // Try again in next bank
-					}
-				} else if (section.isAlignFixed) {
-					// Move to next aligned location
-					// Move back to alignment boundary
-					location.address -= section.alignOfs;
-					// Ensure we're there (e.g. on first check)
-					location.address &= ~section.alignMask;
-					// Go to next align boundary and add offset
-					location.address += section.alignMask + 1 + section.alignOfs;
-				} else {
-					// Any location is fine, so, next free block
-					spaceIdx++;
-					if (spaceIdx < bankMem.size()) {
-						location.address = bankMem[spaceIdx].address;
-					}
-				}
-
-				// If that location is past the current block's end,
-				// go forwards until that is no longer the case.
-				while (spaceIdx < bankMem.size()
-				       && location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) {
-					spaceIdx++;
-				}
-
-				// Try again with the new location/free space combo
 			}
+
+			// Go to the next *possible* location
+			if (section.isAddressFixed) {
+				// If the address is fixed, there can be only
+				// one candidate block per bank; if we already
+				// reached it, give up.
+				if (location.address < section.org) {
+					location.address = section.org;
+				} else {
+					break; // Try again in next bank
+				}
+			} else if (section.isAlignFixed) {
+				// Move to next aligned location
+				// Move back to alignment boundary
+				location.address -= section.alignOfs;
+				// Ensure we're there (e.g. on first check)
+				location.address &= ~section.alignMask;
+				// Go to next align boundary and add offset
+				location.address += section.alignMask + 1 + section.alignOfs;
+			} else {
+				// Any location is fine, so, next free block
+				spaceIdx++;
+				if (spaceIdx < bankMem.size()) {
+					location.address = bankMem[spaceIdx].address;
+				}
+			}
+
+			// If that location is past the current block's end,
+			// go forwards until that is no longer the case.
+			while (spaceIdx < bankMem.size()
+			       && location.address >= bankMem[spaceIdx].address + bankMem[spaceIdx].size) {
+				spaceIdx++;
+			}
+
+			// Try again with the new location/free space combo
 		}
 
 		// Try again in the next bank, if one is available.
@@ -208,22 +208,22 @@ static ssize_t getPlacement(Section const &section, MemoryLocation &location) {
 // Places a section in a suitable location, or error out if it fails to.
 // Due to the implemented algorithm, this should be called with sections of decreasing size!
 static void placeSection(Section &section) {
-	MemoryLocation location;
-
 	// Specially handle 0-byte SECTIONs, as they can't overlap anything
 	if (section.size == 0) {
 		// Unless the SECTION's address was fixed, the starting address
 		// is fine for any alignment, as checked in sect_DoSanityChecks.
-		location.address =
+		MemoryLocation location = {
-		    section.isAddressFixed ? section.org : sectionTypeInfo[section.type].startAddr;
+		    .address =
-		location.bank =
+		        section.isAddressFixed ? section.org : sectionTypeInfo[section.type].startAddr,
-		    section.isBankFixed ? section.bank : sectionTypeInfo[section.type].firstBank;
+		    .bank = section.isBankFixed ? section.bank : sectionTypeInfo[section.type].firstBank,
+		};
 		assignSection(section, location);
 		return;
 	}
 
 	// Place section using first-fit decreasing algorithm
 	// https://en.wikipedia.org/wiki/Bin_packing_problem#First-fit_algorithm
+	MemoryLocation location;
 	if (ssize_t spaceIdx = getPlacement(section, location); spaceIdx != -1) {
 		std::deque<FreeSpace> &bankMem =
 		    memory[section.type][location.bank - sectionTypeInfo[section.type].firstBank];

src/link/output.cpp

@@ -168,7 +168,6 @@ static uint8_t getNextFillByte() {
 	return padValue;
 }
 
-// Write a ROM bank's sections, ordered by increasing address, to the output file.
 static void
     writeBank(std::deque<Section const *> *bankSections, uint16_t baseOffset, uint16_t size) {
 	uint16_t offset = 0;
@@ -202,7 +201,6 @@ static void
 	}
 }
 
-// Writes a ROM file to the output.
 static void writeROM() {
 	if (outputFileName) {
 		if (strcmp(outputFileName, "-")) {
@@ -263,9 +261,7 @@ static void writeROM() {
 	}
 }
 
-// Prints a symbol's name to a file, assuming that the first character is legal.
+static void writeSymName(std::string const &name, FILE *file) {
-// Illegal characters are UTF-8-decoded (errors are replaced by U+FFFD) and emitted as '\u'/'\U'.
-static void printSymName(std::string const &name, FILE *file) {
 	for (char const *ptr = name.c_str(); *ptr != '\0';) {
 		char c = *ptr;
 
@@ -274,7 +270,7 @@ static void printSymName(std::string const &name, FILE *file) {
 			putc(c, file);
 			++ptr;
 		} else {
-			// Output illegal characters using Unicode escapes
+			// Output illegal characters using Unicode escapes ('\u' or '\U')
 			// Decode the UTF-8 codepoint; or at least attempt to
 			uint32_t state = 0, codepoint;
 
@@ -323,27 +319,26 @@ static bool compareSymbols(SortedSymbol const &sym1, SortedSymbol const &sym2) {
 	return sym1_name < sym2_name;
 }
 
-// Write a bank's contents to the sym file
+template<typename F>
-static void writeSymBank(SortedSections const &bankSections, SectionType type, uint32_t bank) {
+static void forEachSortedSection(SortedSections const &bankSections, F callback) {
-#define forEachSortedSection(sect, ...) \
+	for (Section const *sect : bankSections.zeroLenSections) {
-	do { \
+		for (; sect; sect = sect->nextu.get()) {
-		for (auto it = bankSections.zeroLenSections.begin(); \
+			callback(*sect);
-		     it != bankSections.zeroLenSections.end(); \
+		}
-		     it++) { \
+	}
-			for (Section const *sect = *it; sect; sect = sect->nextu.get()) { \
+	for (Section const *sect : bankSections.sections) {
-				__VA_ARGS__ \
+		for (; sect; sect = sect->nextu.get()) {
-			} \
+			callback(*sect);
-		} \
+		}
-		for (auto it = bankSections.sections.begin(); it != bankSections.sections.end(); it++) { \
+	}
-			for (Section const *sect = *it; sect; sect = sect->nextu.get()) { \
+}
-				__VA_ARGS__ \
-			} \
-		} \
-	} while (0)
 
+static void writeSymBank(SortedSections const &bankSections, SectionType type, uint32_t bank) {
 	uint32_t nbSymbols = 0;
 
-	forEachSortedSection(sect, { nbSymbols += sect->symbols.size(); });
+	forEachSortedSection(bankSections, [&](Section const &sect) {
+		nbSymbols += sect.symbols.size();
+	});
 
 	if (!nbSymbols) {
 		return;
@@ -353,36 +348,35 @@ static void writeSymBank(SortedSections const &bankSections, SectionType type, u
 
 	symList.reserve(nbSymbols);
 
-	forEachSortedSection(sect, {
+	forEachSortedSection(bankSections, [&](Section const &sect) {
-		for (Symbol const *sym : sect->symbols) {
+		for (Symbol const *sym : sect.symbols) {
 			// Don't output symbols that begin with an illegal character
-			if (!sym->name.empty() && startsIdentifier(sym->name[0])) {
+			if (sym->name.empty() || !startsIdentifier(sym->name[0])) {
-				uint16_t addr = static_cast<uint16_t>(sym->label().offset + sect->org);
+				continue;
-				uint16_t parentAddr = addr;
-				if (auto pos = sym->name.find('.'); pos != std::string::npos) {
-					std::string parentName = sym->name.substr(0, pos);
-					if (Symbol const *parentSym = sym_GetSymbol(parentName);
-					    parentSym && std::holds_alternative<Label>(parentSym->data)) {
-						auto const &parentLabel = parentSym->label();
-						assume(parentLabel.section != nullptr);
-						parentAddr =
-						    static_cast<uint16_t>(parentLabel.offset + parentLabel.section->org);
-					}
-				}
-				symList.push_back({.sym = sym, .addr = addr, .parentAddr = parentAddr});
 			}
+			uint16_t addr = static_cast<uint16_t>(sym->label().offset + sect.org);
+			uint16_t parentAddr = addr;
+			if (auto pos = sym->name.find('.'); pos != std::string::npos) {
+				std::string parentName = sym->name.substr(0, pos);
+				if (Symbol const *parentSym = sym_GetSymbol(parentName);
+				    parentSym && std::holds_alternative<Label>(parentSym->data)) {
+					auto const &parentLabel = parentSym->label();
+					assume(parentLabel.section != nullptr);
+					parentAddr =
+					    static_cast<uint16_t>(parentLabel.offset + parentLabel.section->org);
+				}
+			}
+			symList.push_back({.sym = sym, .addr = addr, .parentAddr = parentAddr});
 		}
 	});
 
-#undef forEachSortedSection
-
 	std::stable_sort(RANGE(symList), compareSymbols);
 
 	uint32_t symBank = bank + sectionTypeInfo[type].firstBank;
 
 	for (SortedSymbol &sym : symList) {
 		fprintf(symFile, "%02" PRIx32 ":%04" PRIx16 " ", symBank, sym.addr);
-		printSymName(sym.sym->name, symFile);
+		writeSymName(sym.sym->name, symFile);
 		putc('\n', symFile);
 	}
 }
@@ -402,8 +396,7 @@ static void writeEmptySpace(uint16_t begin, uint16_t end) {
 	}
 }
 
-// Prints a section's name to a file.
+static void writeSectionName(std::string const &name, FILE *file) {
-static void printSectionName(std::string const &name, FILE *file) {
 	for (char c : name) {
 		// Escape characters that need escaping
 		switch (c) {
@@ -427,7 +420,47 @@ static void printSectionName(std::string const &name, FILE *file) {
 	}
 }
 
-// Write a bank's contents to the map file
+template<typename F>
+uint16_t forEachSection(SortedSections const &sectList, F callback) {
+	uint16_t used = 0;
+	auto section = sectList.sections.begin();
+	auto zeroLenSection = sectList.zeroLenSections.begin();
+	while (section != sectList.sections.end() || zeroLenSection != sectList.zeroLenSections.end()) {
+		// Pick the lowest section by address out of the two
+		auto &pickedSection = section == sectList.sections.end()                 ? zeroLenSection
+		                      : zeroLenSection == sectList.zeroLenSections.end() ? section
+		                      : (*section)->org < (*zeroLenSection)->org         ? section
+		                                                                         : zeroLenSection;
+		used += (*pickedSection)->size;
+		callback(**pickedSection);
+		pickedSection++;
+	}
+	return used;
+}
+
+static void writeMapSymbols(Section const *sect) {
+	for (uint16_t org = sect->org; sect; sect = sect->nextu.get()) {
+		for (Symbol *sym : sect->symbols) {
+			// Don't output symbols that begin with an illegal character
+			if (sym->name.empty() || !startsIdentifier(sym->name[0])) {
+				continue;
+			}
+			// Space matches "\tSECTION: $xxxx ..."
+			fprintf(mapFile, "\t         $%04" PRIx32 " = ", sym->label().offset + org);
+			writeSymName(sym->name, mapFile);
+			putc('\n', mapFile);
+		}
+
+		// Announce the following "piece"
+		if (SectionModifier mod = sect->nextu ? sect->nextu->modifier : SECTION_NORMAL;
+		    mod == SECTION_UNION) {
+			fputs("\t         ; Next union\n", mapFile);
+		} else if (mod == SECTION_FRAGMENT) {
+			fputs("\t         ; Next fragment\n", mapFile);
+		}
+	}
+}
+
 static void writeMapBank(SortedSections const &sectList, SectionType type, uint32_t bank) {
 	fprintf(
 	    mapFile,
@@ -436,60 +469,27 @@ static void writeMapBank(SortedSections const &sectList, SectionType type, uint3
 	    bank + sectionTypeInfo[type].firstBank
 	);
 
-	uint16_t used = 0;
-	auto section = sectList.sections.begin();
-	auto zeroLenSection = sectList.zeroLenSections.begin();
 	uint16_t prevEndAddr = sectionTypeInfo[type].startAddr;
+	uint16_t used = forEachSection(sectList, [&](Section const &sect) {
+		assume(sect.offset == 0);
 
-	while (section != sectList.sections.end() || zeroLenSection != sectList.zeroLenSections.end()) {
+		writeEmptySpace(prevEndAddr, sect.org);
-		// Pick the lowest section by address out of the two
-		auto &pickedSection = section == sectList.sections.end()                 ? zeroLenSection
-		                      : zeroLenSection == sectList.zeroLenSections.end() ? section
-		                      : (*section)->org < (*zeroLenSection)->org         ? section
-		                                                                         : zeroLenSection;
-		Section const *sect = *pickedSection;
 
-		used += sect->size;
+		prevEndAddr = sect.org + sect.size;
-		assume(sect->offset == 0);
 
-		writeEmptySpace(prevEndAddr, sect->org);
+		fprintf(mapFile, "\tSECTION: $%04" PRIx16, sect.org);
-
+		if (sect.size != 0) {
-		prevEndAddr = sect->org + sect->size;
-
-		fprintf(mapFile, "\tSECTION: $%04" PRIx16, sect->org);
-		if (sect->size != 0) {
 			fprintf(mapFile, "-$%04x", prevEndAddr - 1);
 		}
-		fprintf(mapFile, " ($%04" PRIx16 " byte%s) [\"", sect->size, sect->size == 1 ? "" : "s");
+		fprintf(mapFile, " ($%04" PRIx16 " byte%s) [\"", sect.size, sect.size == 1 ? "" : "s");
-		printSectionName(sect->name, mapFile);
+		writeSectionName(sect.name, mapFile);
 		fputs("\"]\n", mapFile);
 
 		if (!noSymInMap) {
 			// Also print symbols in the following "pieces"
-			for (uint16_t org = sect->org; sect; sect = sect->nextu.get()) {
+			writeMapSymbols(&sect);
-				for (Symbol *sym : sect->symbols) {
-					// Don't output symbols that begin with an illegal character
-					if (!sym->name.empty() && startsIdentifier(sym->name[0])) {
-						// Space matches "\tSECTION: $xxxx ..."
-						fprintf(mapFile, "\t         $%04" PRIx32 " = ", sym->label().offset + org);
-						printSymName(sym->name, mapFile);
-						putc('\n', mapFile);
-					}
-				}
-
-				if (sect->nextu) {
-					// Announce the following "piece"
-					if (sect->nextu->modifier == SECTION_UNION) {
-						fputs("\t         ; Next union\n", mapFile);
-					} else if (sect->nextu->modifier == SECTION_FRAGMENT) {
-						fputs("\t         ; Next fragment\n", mapFile);
-					}
-				}
-			}
 		}
-
+	});
-		pickedSection++;
-	}
 
 	if (used == 0) {
 		fputs("\tEMPTY\n", mapFile);
@@ -504,7 +504,6 @@ static void writeMapBank(SortedSections const &sectList, SectionType type, uint3
 	}
 }
 
-// Write the total used and free space by section type to the map file
 static void writeMapSummary() {
 	fputs("SUMMARY:\n", mapFile);
 
@@ -525,24 +524,7 @@ static void writeMapSummary() {
 		uint32_t usedTotal = 0;
 
 		for (uint32_t bank = 0; bank < nbBanks; bank++) {
-			uint16_t used = 0;
+			usedTotal += forEachSection(sections[type][bank], [](Section const &) {});
-			auto &sectList = sections[type][bank];
-			auto section = sectList.sections.begin();
-			auto zeroLenSection = sectList.zeroLenSections.begin();
-
-			while (section != sectList.sections.end()
-			       || zeroLenSection != sectList.zeroLenSections.end()) {
-				// Pick the lowest section by address out of the two
-				auto &pickedSection = section == sectList.sections.end() ? zeroLenSection
-				                      : zeroLenSection == sectList.zeroLenSections.end() ? section
-				                      : (*section)->org < (*zeroLenSection)->org         ? section
-				                                                                 : zeroLenSection;
-
-				used += (*pickedSection)->size;
-				pickedSection++;
-			}
-
-			usedTotal += used;
 		}
 
 		fprintf(
@@ -560,7 +542,6 @@ static void writeMapSummary() {
 	}
 }
 
-// Writes the sym file, if applicable.
 static void writeSym() {
 	if (!symFileName) {
 		return;
@@ -606,12 +587,11 @@ static void writeSym() {
 		int32_t val = std::get<int32_t>(sym->data);
 		int width = val < 0x100 ? 2 : val < 0x10000 ? 4 : 8;
 		fprintf(symFile, "%0*" PRIx32 " ", width, val);
-		printSymName(sym->name, symFile);
+		writeSymName(sym->name, symFile);
 		putc('\n', symFile);
 	}
 }
 
-// Writes the map file, if applicable.
 static void writeMap() {
 	if (!mapFileName) {
 		return;