.sym file sorting accounts for local labels' parents' addresses and names (#1684)

src/asm/main.cpp

@@ -36,7 +36,7 @@ static std::string make_escape(std::string &str) {
 	size_t pos = 0;
 	for (;;) {
 		// All dollars needs to be doubled
-		size_t nextPos = str.find("$", pos);
+		size_t nextPos = str.find('$', pos);
 		if (nextPos == std::string::npos) {
 			break;
 		}

src/link/output.cpp

@@ -30,6 +30,7 @@ FILE *mapFile;
 struct SortedSymbol {
 	Symbol const *sym;
 	uint16_t addr;
+	uint16_t parentAddr;
 };
 
 struct SortedSections {
@@ -311,33 +312,28 @@ static void printSymName(std::string const &name, FILE *file) {
 }
 
 // Comparator function for `std::stable_sort` to sort symbols
-// Symbols are ordered by address, then by parentage
 static bool compareSymbols(SortedSymbol const &sym1, SortedSymbol const &sym2) {
+	// First, sort by address
 	if (sym1.addr != sym2.addr) {
 		return sym1.addr < sym2.addr;
 	}
 
+	// Second, sort by locality (global before local)
 	std::string const &sym1_name = sym1.sym->name;
 	std::string const &sym2_name = sym2.sym->name;
-	bool sym1_local = sym1_name.find(".") != std::string::npos;
-	bool sym2_local = sym2_name.find(".") != std::string::npos;
-
+	bool sym1_local = sym1_name.find('.') != std::string::npos;
+	bool sym2_local = sym2_name.find('.') != std::string::npos;
 	if (sym1_local != sym2_local) {
-		size_t sym1_len = sym1_name.length();
-		size_t sym2_len = sym2_name.length();
-
-		// Sort parent labels before their child local labels
-		if (sym2_name.starts_with(sym1_name) && sym2_name[sym1_len] == '.') {
-			return true;
-		}
-		if (sym1_name.starts_with(sym2_name) && sym1_name[sym2_len] == '.') {
-			return false;
-		}
-		// Sort local labels before unrelated global labels
-		return sym1_local;
+		return sym1_local < sym2_local;
 	}
 
-	return false;
+	// Third, sort by parent address
+	if (sym1.parentAddr != sym2.parentAddr) {
+		return sym1.parentAddr < sym2.parentAddr;
+	}
+
+	// Fourth, sort by name
+	return sym1_name < sym2_name;
 }
 
 // Write a bank's contents to the sym file
@@ -374,10 +370,19 @@ static void writeSymBank(SortedSections const &bankSections, SectionType type, u
 		for (Symbol const *sym : sect->symbols) {
 			// Don't output symbols that begin with an illegal character
 			if (isLegalSymbol(*sym)) {
-				symList.push_back({
-				    .sym = sym,
-				    .addr = static_cast<uint16_t>(sym->label().offset + sect->org),
-				});
+				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 && parentSym->data.holds<Label>()) {
+						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});
 			}
 		}
 	});

test/link/constant-parent/a.asm

@@ -0,0 +1,3 @@
+section "a", rom0
+export def parent = 42
+db 1, 2, 3

test/link/constant-parent/b.asm

@@ -0,0 +1,4 @@
+section "b", rom0
+db 4, 5, 6
+parent.child::
+db 7, 8, 9

test/link/constant-parent/ref.out.bin

@@ -0,0 +1 @@
+	


test/link/constant-parent/ref.out.sym

@@ -0,0 +1,3 @@
+; File generated by rgblink
+00:0003 parent.child
+2a parent

test/link/load-fragment/section-fragment/ref.out.sym

@@ -2,8 +2,8 @@
 00:0000 Part1
 00:0004 Part1End
 00:0004 Part2
-00:0010 Part3
 00:0010 Part2End
+00:0010 Part3
 00:0014 Part3End
 00:c000 wPart1
 00:c004 wPart3

test/link/symbols/good/ref.out.sym

@@ -1,16 +1,16 @@
 ; File generated by rgblink
 00:0000 Beta
-00:0003 End
 00:0003 Alpha
+00:0003 End
 00:0006 End
 00:c000 wStart
 00:c000 wStart
-00:c000 wStart.word1
 00:c000 wStart.long1
+00:c000 wStart.word1
 00:c002 wStart.word2
-00:c004 wEnd
-00:c004 wEnd
 00:c004 wBeta
-00:c007 wBeta.End
+00:c004 wEnd
+00:c004 wEnd
 00:c007 wAlpha
+00:c007 wBeta.End
 00:c00a wAlpha.End

test/link/test.sh

@@ -149,6 +149,17 @@ rgblinkQuiet -o "$gbtemp" "$otemp" "$gbtemp2" 2>"$outtemp"
 tryDiff "$test"/out.err "$outtemp"
 evaluateTest
 
+test="constant-parent"
+startTest
+"$RGBASM" -o "$otemp" "$test"/a.asm
+"$RGBASM" -o "$gbtemp2" "$test"/b.asm
+continueTest
+rgblinkQuiet -o "$gbtemp" -n "$outtemp2" "$otemp" "$gbtemp2" 2>"$outtemp"
+tryDiff "$test"/out.err "$outtemp"
+tryDiff "$test"/ref.out.sym "$outtemp2"
+tryCmpRom "$test"/ref.out.bin
+evaluateTest
+
 for test in fragment-align/*; do
 	startTest
 	"$RGBASM" -o "$otemp" "$test"/a.asm