Take care of miscellaneous commented TODOs (#1676)

CMakeLists.txt

@@ -51,8 +51,8 @@ else()
     add_link_options(${SAN_FLAGS})
     add_definitions(-D_GLIBCXX_ASSERTIONS)
     # A non-zero optimization level is desired in debug mode, but allow overriding it nonetheless
-    # TODO: this overrides anything previously set... that's a bit sloppy!
+    set(CMAKE_CXX_FLAGS_DEBUG "-g -Og -fno-omit-frame-pointer -fno-optimize-sibling-calls ${CMAKE_CXX_FLAGS_DEBUG}"
-    set(CMAKE_CXX_FLAGS_DEBUG "-g -Og -fno-omit-frame-pointer -fno-optimize-sibling-calls" CACHE STRING "" FORCE)
+                              CACHE STRING "" FORCE)
   endif()
 
   if(MORE_WARNINGS)

contrib/gbdiff.bash

@@ -32,14 +32,15 @@ diff <(xxd "$1") <(xxd "$2") | while read -r LINE; do
 			# Ignore comment lines, only pick matching bank
 			# (The bank regex ignores comments already, make `cut` and `tr` process less lines)
 			grep -Ei "$(printf "^%02x:" $BANK)" "$SYMFILE" |
+			 sed "s/$(printf "^%02x:" $BANK)/0x/g" |
 			 cut -d ';' -f 1 |
 			 tr -d "\r" |
+			 sort -g |
 			 while read -r SYMADDR SYM; do
-				SYMADDR=$((0x${SYMADDR#*:}))
+				SYMADDR=$(($SYMADDR))
 				if [[ $SYMADDR -le $ADDR ]]; then
 					printf " (%s+0x%x)\n" "$SYM" $((ADDR - SYMADDR))
 				fi
-			# TODO: assumes sorted sym files
 			done | tail -n 1
 		fi)
 		printf "%02x:%04x %s\n" $BANK $ADDR "$EXTRA"

src/fix/main.cpp

@@ -719,8 +719,7 @@ static bool hasRAM(MbcType type) {
 	case MBC3_TIMER_BATTERY:
 	case MBC5:
 	case MBC5_RUMBLE:
-	case MBC6:         // TODO: not sure
+	case BANDAI_TAMA5: // "Game de Hakken!! Tamagotchi - Osutchi to Mesutchi" has RAM size 0
-	case BANDAI_TAMA5: // TODO: not sure
 	case MBC_NONE:
 	case MBC_BAD:
 	case MBC_WRONG_FEATURES:
@@ -741,6 +740,7 @@ static bool hasRAM(MbcType type) {
 	case MBC5_RAM_BATTERY:
 	case MBC5_RUMBLE_RAM:
 	case MBC5_RUMBLE_RAM_BATTERY:
+	case MBC6: // "Net de Get - Minigame @ 100" has RAM size 3 (32 KiB)
 	case MBC7_SENSOR_RUMBLE_RAM_BATTERY:
 	case POCKET_CAMERA:
 	case HUC3:
@@ -1276,7 +1276,7 @@ static bool processFilename(char const *name, char const *outputName) {
 				// LCOV_EXCL_START
 				report("FATAL: Failed to stat \"%s\": %s\n", name, strerror(errno));
 				// LCOV_EXCL_STOP
-			} else if (!S_ISREG(stat.st_mode)) { // TODO: Do we want to support FIFOs or symlinks?
+			} else if (!S_ISREG(stat.st_mode)) { // We do not support FIFOs or symlinks
 				// LCOV_EXCL_START
 				report(
 				    "FATAL: \"%s\" is not a regular file, and thus cannot be modified in-place\n",
@@ -1531,7 +1531,7 @@ int main(int argc, char *argv[]) {
 				    "warning: RAM size 1 (2 KiB) was specified for MBC \"%s\"\n",
 				    mbcName(cartridgeType)
 				);
-			} // TODO: check possible values?
+			}
 		} else if (ramSize) {
 			fprintf(
 			    stderr,

src/gfx/main.cpp

@@ -451,9 +451,9 @@ static char *parseArgv(int argc, char *argv[]) {
 			} else {
 				options.palSpecType = Options::EXPLICIT;
 				// Can't parse the file yet, as "flat" color collections need to know the palette
-				// size to be split; thus, we defer that
+				// size to be split; thus, we defer that.
-				// TODO: this does not validate the `fmt` part of any external spec but the last
+				// If a following `-c` overrides a previous one, the `fmt` part of an overridden
-				// one, but I guess that's okay
+				// external palette spec will not be validated, but I guess that's okay.
 				localOptions.externalPalSpec = musl_optarg;
 			}
 			break;

src/gfx/process.cpp

@@ -211,11 +211,6 @@ public:
 		png_set_read_fn(png, this, readData);
 		png_set_sig_bytes(png, pngHeader.size());
 
-		// TODO: png_set_crc_action(png, PNG_CRC_ERROR_QUIT, PNG_CRC_WARN_DISCARD);
-
-		// Skipping chunks we don't use should improve performance
-		// TODO: png_set_keep_unknown_chunks(png, ...);
-
 		// Process all chunks up to but not including the image data
 		png_read_info(png, info);
 
@@ -294,9 +289,7 @@ public:
 			options.verbosePrint(Options::VERB_INTERM, "No embedded palette\n");
 		}
 
-		// Set up transformations; to turn everything into RGBA888
+		// Set up transformations to turn everything into RGBA888 for simplicity of handling
-		// TODO: it's not necessary to uniformize the pixel data (in theory), and not doing
-		// so *might* improve performance, and should reduce memory usage.
 
 		// Convert grayscale to RGB
 		switch (colorType & ~PNG_COLOR_MASK_ALPHA) {
@@ -559,7 +552,6 @@ static void generatePalSpec(Png const &png) {
 static std::tuple<DefaultInitVec<size_t>, std::vector<Palette>>
     generatePalettes(std::vector<ProtoPalette> const &protoPalettes, Png const &png) {
 	// Run a "pagination" problem solver
-	// TODO: allow picking one of several solvers?
 	auto [mappings, nbPalettes] = overloadAndRemove(protoPalettes);
 	assume(mappings.size() == protoPalettes.size());
 

src/gfx/reverse.cpp

@@ -195,8 +195,6 @@ void reverse() {
 	    Options::VERB_INTERM, "Reversed image dimensions: %zux%zu tiles\n", width, height
 	);
 
-	// TODO: `-U` to configure tile size beyond 8x8px ("deduplication units")
-
 	std::vector<std::array<std::optional<Rgba>, 4>> palettes{
 	    {Rgba(0xFFFFFFFF), Rgba(0xAAAAAAFF), Rgba(0x555555FF), Rgba(0x000000FF)}
 	};

test/asm/label-diff.asm

@@ -1,6 +1,3 @@
-
-PUSHS
-
 SECTION "floating", ROM0
 
 Known: ; This symbol is known to be a label, but its value is not known yet
@@ -15,15 +12,14 @@ SECTION "fixed 2", ROM0[69]
 
 Constant2: ; Same as above
 
+ENDSECTION ; Ensure we are in neither section
+
 
 MACRO print_diff
 	PRINTLN (\1) - (\2)
 	PRINTLN (\2) - (\1)
 ENDM
 
-POPS ; Ensure we are in neither section
-
-; TODO: uncomment all that can be, there is seriously room for improvement here
 	; Diffing two constants should work
 	print_diff Constant, Constant2
 	; Diffing two labels in the same SECTION as well

test/asm/label-diff.err

@@ -1,37 +1,37 @@
+error: label-diff.asm(28) -> label-diff.asm::print_diff(19):
+    Expected constant expression: 'Known' is not constant at assembly time
+error: label-diff.asm(28) -> label-diff.asm::print_diff(20):
+    Expected constant expression: 'Known' is not constant at assembly time
+error: label-diff.asm(30) -> label-diff.asm::print_diff(19):
+    Expected constant expression: 'Unknown' is not constant at assembly time
+error: label-diff.asm(30) -> label-diff.asm::print_diff(20):
+    Expected constant expression: 'Unknown' is not constant at assembly time
+error: label-diff.asm(32) -> label-diff.asm::print_diff(19):
+    Expected constant expression: 'Known' is not constant at assembly time
 error: label-diff.asm(32) -> label-diff.asm::print_diff(20):
-    Expected constant expression: 'Known' is not constant at assembly time
+    Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(32) -> label-diff.asm::print_diff(21):
+error: label-diff.asm(34) -> label-diff.asm::print_diff(19):
-    Expected constant expression: 'Known' is not constant at assembly time
+    Expected constant expression: 'Unknown' is not constant at assembly time
 error: label-diff.asm(34) -> label-diff.asm::print_diff(20):
-    Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(34) -> label-diff.asm::print_diff(21):
-    Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(36) -> label-diff.asm::print_diff(20):
-    Expected constant expression: 'Known' is not constant at assembly time
-error: label-diff.asm(36) -> label-diff.asm::print_diff(21):
-    Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(38) -> label-diff.asm::print_diff(20):
-    Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(38) -> label-diff.asm::print_diff(21):
     Expected constant expression: 'Unknown2' is not constant at assembly time
-error: label-diff.asm(45) -> label-diff.asm::print_diff(20):
+error: label-diff.asm(41) -> label-diff.asm::print_diff(19):
     Expected constant expression: 'Known' is not constant at assembly time
-error: label-diff.asm(45) -> label-diff.asm::print_diff(21):
+error: label-diff.asm(41) -> label-diff.asm::print_diff(20):
     Expected constant expression: 'Known' is not constant at assembly time
-error: label-diff.asm(47) -> label-diff.asm::print_diff(20):
+error: label-diff.asm(43) -> label-diff.asm::print_diff(19):
     Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(47) -> label-diff.asm::print_diff(21):
+error: label-diff.asm(43) -> label-diff.asm::print_diff(20):
+    Expected constant expression: 'Unknown' is not constant at assembly time
+error: label-diff.asm(54) -> label-diff.asm::print_diff(19):
+    Expected constant expression: PC is not constant at assembly time
+error: label-diff.asm(54) -> label-diff.asm::print_diff(20):
+    Expected constant expression: PC is not constant at assembly time
+error: label-diff.asm(56) -> label-diff.asm::print_diff(19):
+    Expected constant expression: 'Known' is not constant at assembly time
+error: label-diff.asm(56) -> label-diff.asm::print_diff(20):
+    Expected constant expression: PC is not constant at assembly time
+error: label-diff.asm(58) -> label-diff.asm::print_diff(19):
     Expected constant expression: 'Unknown' is not constant at assembly time
 error: label-diff.asm(58) -> label-diff.asm::print_diff(20):
     Expected constant expression: PC is not constant at assembly time
-error: label-diff.asm(58) -> label-diff.asm::print_diff(21):
-    Expected constant expression: PC is not constant at assembly time
-error: label-diff.asm(60) -> label-diff.asm::print_diff(20):
-    Expected constant expression: 'Known' is not constant at assembly time
-error: label-diff.asm(60) -> label-diff.asm::print_diff(21):
-    Expected constant expression: PC is not constant at assembly time
-error: label-diff.asm(62) -> label-diff.asm::print_diff(20):
-    Expected constant expression: 'Unknown' is not constant at assembly time
-error: label-diff.asm(62) -> label-diff.asm::print_diff(21):
-    Expected constant expression: PC is not constant at assembly time
 error: Assembly aborted (18 errors)!

test/asm/unique-id.asm

@@ -7,9 +7,9 @@ macro m
 	ENDR
 	warn_unique
 endm
-	; TODO: Ideally we'd test now as well, but it'd cause a fatal error
+
-	;warn_unique
+	warn_unique
-	m
+	m
-	;warn_unique
+	warn_unique
 	m
 	warn_unique

test/asm/unique-id.err

@@ -1,3 +1,9 @@
+error: unique-id.asm(11):
+    '\@' cannot be used outside of a macro or REPT/FOR block
+while expanding symbol "warn_unique"
+warning: unique-id.asm(11): [-Wuser]
+    !
+while expanding symbol "warn_unique"
 warning: unique-id.asm(12) -> unique-id.asm::m(4): [-Wuser]
     _u1!
 while expanding symbol "warn_unique"
@@ -10,6 +16,12 @@ while expanding symbol "warn_unique"
 warning: unique-id.asm(12) -> unique-id.asm::m(8): [-Wuser]
     _u1!
 while expanding symbol "warn_unique"
+error: unique-id.asm(13):
+    '\@' cannot be used outside of a macro or REPT/FOR block
+while expanding symbol "warn_unique"
+warning: unique-id.asm(13): [-Wuser]
+    !
+while expanding symbol "warn_unique"
 warning: unique-id.asm(14) -> unique-id.asm::m(4): [-Wuser]
     _u4!
 while expanding symbol "warn_unique"
@@ -28,4 +40,4 @@ while expanding symbol "warn_unique"
 warning: unique-id.asm(15): [-Wuser]
     !
 while expanding symbol "warn_unique"
-error: Assembly aborted (1 error)!
+error: Assembly aborted (3 errors)!

test/gfx/rgbgfx_test.cpp

@@ -173,11 +173,6 @@ public:
 		png_set_read_fn(png, this, readData);
 		png_set_sig_bytes(png, pngHeader.size());
 
-		// TODO: png_set_crc_action(png, PNG_CRC_ERROR_QUIT, PNG_CRC_WARN_DISCARD);
-
-		// Skipping chunks we don't use should improve performance
-		// TODO: png_set_keep_unknown_chunks(png, ...);
-
 		// Process all chunks up to but not including the image data
 		png_read_info(png, info);
 
@@ -202,9 +197,7 @@ public:
 			}
 		}
 
-		// Set up transformations; to turn everything into RGBA888
+		// Set up transformations to turn everything into RGBA888 for simplicity of handling
-		// TODO: it's not necessary to uniformize the pixel data (in theory), and not doing
-		// so *might* improve performance, and should reduce memory usage.
 
 		// Convert grayscale to RGB
 		switch (colorType & ~PNG_COLOR_MASK_ALPHA) {