From 1fc9ba86c4eee9e2e96eb6a8046f45be0ca24248 Mon Sep 17 00:00:00 2001
From: Rangi42 <sylvie.oukaour+rangi42@gmail.com>
Date: Fri, 5 Sep 2025 16:34:51 -0400
Subject: [PATCH] Some RGBLINK refactoring

- Consistently refer to `Section` fragments/unions as "pieces" (renaming `.nextu`)
- Remove `Symbol`'s `.label()` accessors (use `std::get<Label>`)
- Move some `Label`-related logic into `Symbol` methods
---
 include/link/section.hpp |  2 +-
 include/link/symbol.hpp  |  4 +--
 src/link/assign.cpp      |  7 ++--
 src/link/layout.cpp      |  7 ++--
 src/link/object.cpp      | 69 +++++++++-------------------------------
 src/link/output.cpp      | 15 +++++----
 src/link/patch.cpp       |  6 ++--
 src/link/sdas_obj.cpp    | 17 ++--------
 src/link/section.cpp     |  6 ++--
 src/link/symbol.cpp      | 40 +++++++++++++++++++++++
 10 files changed, 82 insertions(+), 91 deletions(-)

diff --git a/include/link/section.hpp b/include/link/section.hpp
index a474f279..6e0a163b 100644
--- a/include/link/section.hpp
+++ b/include/link/section.hpp
@@ -48,7 +48,7 @@ struct Section {
 	// Extra info computed during linking
 	std::vector<Symbol> *fileSymbols;
 	std::vector<Symbol *> symbols;
-	std::unique_ptr<Section> nextu; // The next "component" of this unionized sect
+	std::unique_ptr<Section> nextPiece; // The next fragment or union "piece" of this section
 };
 
 // Execute a callback for each section currently registered.
diff --git a/include/link/symbol.hpp b/include/link/symbol.hpp
index d6c8e776..e6bd63dd 100644
--- a/include/link/symbol.hpp
+++ b/include/link/symbol.hpp
@@ -33,8 +33,8 @@ struct Symbol {
 	    >
 	    data;
 
-	Label &label() { return std::get<Label>(data); }
-	Label const &label() const { return std::get<Label>(data); }
+	void linkToSection(Section &section);
+	void fixSectionOffset();
 };
 
 void sym_ForEach(void (*callback)(Symbol &));
diff --git a/src/link/assign.cpp b/src/link/assign.cpp
index 8b2d5733..8c488e48 100644
--- a/src/link/assign.cpp
+++ b/src/link/assign.cpp
@@ -53,11 +53,10 @@ static void initFreeSpace() {
 static void assignSection(Section &section, MemoryLocation const &location) {
 	// Propagate the assigned location to all UNIONs/FRAGMENTs
 	// so `jr` patches in them will have the correct offset
-	for (Section *next = &section; next != nullptr; next = next->nextu.get()) {
-		next->org = location.address;
-		next->bank = location.bank;
+	for (Section *piece = &section; piece != nullptr; piece = piece->nextPiece.get()) {
+		piece->org = location.address;
+		piece->bank = location.bank;
 	}
-
 	out_AddSection(section);
 }
 
diff --git a/src/link/layout.cpp b/src/link/layout.cpp
index ab4a925f..07f141c8 100644
--- a/src/link/layout.cpp
+++ b/src/link/layout.cpp
@@ -248,10 +248,9 @@ void layout_PlaceSection(std::string const &name, bool isOptional) {
 			    typeInfo.name.c_str()
 			);
 		} else {
-			// SDCC areas don't have a type assigned yet, so the linker script is used to give them
-			// one.
-			for (Section *fragment = section; fragment; fragment = fragment->nextu.get()) {
-				fragment->type = activeType;
+			// SDCC areas don't have a type assigned yet, so the linker script gives them one.
+			for (Section *piece = section; piece != nullptr; piece = piece->nextPiece.get()) {
+				piece->type = activeType;
 			}
 		}
 	} else if (section->type != activeType) {
diff --git a/src/link/object.cpp b/src/link/object.cpp
index 423f3e18..0f3650eb 100644
--- a/src/link/object.cpp
+++ b/src/link/object.cpp
@@ -390,25 +390,6 @@ static void readSection(
 	}
 }
 
-// Links a symbol to a section, keeping the section's symbol list sorted.
-static void linkSymToSect(Symbol &symbol, Section &section) {
-	uint32_t a = 0, b = section.symbols.size();
-	int32_t symbolOffset = symbol.label().offset;
-
-	while (a != b) {
-		uint32_t c = (a + b) / 2;
-		int32_t otherOffset = section.symbols[c]->label().offset;
-
-		if (otherOffset > symbolOffset) {
-			b = c;
-		} else {
-			a = c + 1;
-		}
-	}
-
-	section.symbols.insert(section.symbols.begin() + a, &symbol);
-}
-
 // Reads an assertion from a file.
 static void readAssertion(
     FILE *file,
@@ -512,15 +493,11 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 	std::vector<uint32_t> nbSymPerSect(nbSections, 0);
 
 	verbosePrint(VERB_INFO, "Reading %" PRIu32 " symbols...\n", nbSymbols);
-	for (uint32_t i = 0; i < nbSymbols; ++i) {
-		// Read symbol
-		Symbol &symbol = fileSymbols[i];
-
-		readSymbol(file, symbol, fileName, nodes[fileID]);
-
-		sym_AddSymbol(symbol);
-		if (std::holds_alternative<Label>(symbol.data)) {
-			++nbSymPerSect[std::get<Label>(symbol.data).sectionID];
+	for (Symbol &sym : fileSymbols) {
+		readSymbol(file, sym, fileName, nodes[fileID]);
+		sym_AddSymbol(sym);
+		if (std::holds_alternative<Label>(sym.data)) {
+			++nbSymPerSect[std::get<Label>(sym.data).sectionID];
 		}
 	}
 
@@ -529,9 +506,8 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 
 	verbosePrint(VERB_INFO, "Reading %" PRIu32 " sections...\n", nbSections);
 	for (uint32_t i = 0; i < nbSections; ++i) {
-		// Read section
 		fileSections[i] = std::make_unique<Section>();
-		fileSections[i]->nextu = nullptr;
+		fileSections[i]->nextPiece = nullptr;
 		readSection(file, *fileSections[i], fileName, nodes[fileID]);
 		fileSections[i]->fileSymbols = &fileSymbols;
 		fileSections[i]->symbols.reserve(nbSymPerSect[i]);
@@ -549,21 +525,18 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 	}
 
 	// Give patches' PC section pointers to their sections
-	for (uint32_t i = 0; i < nbSections; ++i) {
-		if (sectTypeHasData(fileSections[i]->type)) {
-			for (Patch &patch : fileSections[i]->patches) {
+	for (std::unique_ptr<Section> const &sect : fileSections) {
+		if (sectTypeHasData(sect->type)) {
+			for (Patch &patch : sect->patches) {
 				linkPatchToPCSect(patch, fileSections);
 			}
 		}
 	}
 
 	// Give symbols' section pointers to their sections
-	for (uint32_t i = 0; i < nbSymbols; ++i) {
-		if (std::holds_alternative<Label>(fileSymbols[i].data)) {
-			Label &label = std::get<Label>(fileSymbols[i].data);
-			label.section = fileSections[label.sectionID].get();
-			// Give the section a pointer to the symbol as well
-			linkSymToSect(fileSymbols[i], *label.section);
+	for (Symbol &sym : fileSymbols) {
+		if (std::holds_alternative<Label>(sym.data)) {
+			sym.linkToSection(*fileSections[std::get<Label>(sym.data).sectionID]);
 		}
 	}
 
@@ -572,23 +545,11 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 		sect_AddSection(std::move(fileSections[i]));
 	}
 
-	// Fix symbols' section pointers to component sections
+	// Fix symbols' section pointers to section "pieces"
 	// This has to run **after** all the `sect_AddSection()` calls,
 	// so that `sect_GetSection()` will work
-	for (uint32_t i = 0; i < nbSymbols; ++i) {
-		if (std::holds_alternative<Label>(fileSymbols[i].data)) {
-			Label &label = std::get<Label>(fileSymbols[i].data);
-			Section *section = label.section;
-			if (section->modifier != SECTION_NORMAL) {
-				// Associate the symbol with the main section, not the "component" one
-				label.section = sect_GetSection(section->name);
-			}
-			if (section->modifier == SECTION_FRAGMENT) {
-				// Add the fragment's offset to the symbol's
-				// (`section->offset` is computed by `sect_AddSection`)
-				label.offset += section->offset;
-			}
-		}
+	for (Symbol &sym : fileSymbols) {
+		sym.fixSectionOffset();
 	}
 }
 
diff --git a/src/link/output.cpp b/src/link/output.cpp
index c4c8868c..1141fa6b 100644
--- a/src/link/output.cpp
+++ b/src/link/output.cpp
@@ -317,12 +317,12 @@ static bool compareSymbols(SortedSymbol const &sym1, SortedSymbol const &sym2) {
 template<typename F>
 static void forEachSortedSection(SortedSections const &bankSections, F callback) {
 	for (Section const *sect : bankSections.zeroLenSections) {
-		for (; sect; sect = sect->nextu.get()) {
+		for (; sect != nullptr; sect = sect->nextPiece.get()) {
 			callback(*sect);
 		}
 	}
 	for (Section const *sect : bankSections.sections) {
-		for (; sect; sect = sect->nextu.get()) {
+		for (; sect != nullptr; sect = sect->nextPiece.get()) {
 			callback(*sect);
 		}
 	}
@@ -349,13 +349,14 @@ static void writeSymBank(SortedSections const &bankSections, SectionType type, u
 			if (sym->name.empty() || !startsIdentifier(sym->name[0])) {
 				continue;
 			}
-			uint16_t addr = static_cast<uint16_t>(sym->label().offset + sect.org);
+			assume(std::holds_alternative<Label>(sym->data));
+			uint16_t addr = static_cast<uint16_t>(std::get<Label>(sym->data).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)) {
-					Label const &parentLabel = parentSym->label();
+					Label const &parentLabel = std::get<Label>(parentSym->data);
 					Section const &parentSection = *parentLabel.section;
 					parentAddr = static_cast<uint16_t>(parentLabel.offset + parentSection.org);
 				}
@@ -435,7 +436,7 @@ uint16_t forEachSection(SortedSections const &sectList, F callback) {
 static void writeMapSymbols(Section const *sect) {
 	uint16_t org = sect->org;
 
-	for (bool announced = true; sect; sect = sect->nextu.get(), announced = false) {
+	for (bool announced = true; sect != nullptr; sect = sect->nextPiece.get(), announced = false) {
 		for (Symbol *sym : sect->symbols) {
 			// Don't output symbols that begin with an illegal character
 			if (sym->name.empty() || !startsIdentifier(sym->name[0])) {
@@ -450,8 +451,10 @@ static void writeMapSymbols(Section const *sect) {
 				}
 				announced = true;
 			}
+			assume(std::holds_alternative<Label>(sym->data));
+			uint32_t address = std::get<Label>(sym->data).offset + org;
 			// Space matches "\tSECTION: $xxxx ..."
-			fprintf(mapFile, "\t         $%04" PRIx32 " = ", sym->label().offset + org);
+			fprintf(mapFile, "\t         $%04" PRIx32 " = ", address);
 			writeSymName(sym->name, mapFile);
 			putc('\n', mapFile);
 		}
diff --git a/src/link/patch.cpp b/src/link/patch.cpp
index 57028110..c61d9082 100644
--- a/src/link/patch.cpp
+++ b/src/link/patch.cpp
@@ -580,14 +580,14 @@ static void applyFilePatches(Section &section, Section &dataSection) {
 	}
 }
 
-// Applies all of a section's patches, iterating over "components" of unionized sections
+// Applies all of a section's patches, iterating over "pieces" of unionized sections
 static void applyPatches(Section &section) {
 	if (!sectTypeHasData(section.type)) {
 		return;
 	}
 
-	for (Section *component = &section; component; component = component->nextu.get()) {
-		applyFilePatches(*component, section);
+	for (Section *piece = &section; piece != nullptr; piece = piece->nextPiece.get()) {
+		applyFilePatches(*piece, section);
 	}
 }
 
diff --git a/src/link/sdas_obj.cpp b/src/link/sdas_obj.cpp
index c115dc6c..8d4ab11d 100644
--- a/src/link/sdas_obj.cpp
+++ b/src/link/sdas_obj.cpp
@@ -364,7 +364,7 @@ void sdobj_ReadFile(FileStackNode const &src, FILE *file, std::vector<Symbol> &f
 			}
 			curSection->isAlignFixed = false;       // No such concept!
 			curSection->fileSymbols = &fileSymbols; // IDs are instead per-section
-			curSection->nextu = nullptr;
+			curSection->nextPiece = nullptr;
 
 			fileSections.push_back({.section = std::move(curSection), .writeIndex = 0});
 			break;
@@ -902,21 +902,10 @@ void sdobj_ReadFile(FileStackNode const &src, FILE *file, std::vector<Symbol> &f
 		sect_AddSection(std::move(section));
 	}
 
-	// Fix symbols' section pointers to component sections
+	// Fix symbols' section pointers to section "pieces"
 	// This has to run **after** all the `sect_AddSection()` calls,
 	// so that `sect_GetSection()` will work
 	for (Symbol &sym : fileSymbols) {
-		if (std::holds_alternative<Label>(sym.data)) {
-			Label &label = std::get<Label>(sym.data);
-			if (Section *section = label.section; section->modifier != SECTION_NORMAL) {
-				if (section->modifier == SECTION_FRAGMENT) {
-					// Add the fragment's offset to the symbol's
-					// (`section->offset` is computed by `sect_AddSection`)
-					label.offset += section->offset;
-				}
-				// Associate the symbol with the main section, not the "component" one
-				label.section = sect_GetSection(section->name);
-			}
-		}
+		sym.fixSectionOffset();
 	}
 }
diff --git a/src/link/section.cpp b/src/link/section.cpp
index 9876fdd0..5a116675 100644
--- a/src/link/section.cpp
+++ b/src/link/section.cpp
@@ -189,10 +189,10 @@ static void mergeSections(Section &target, std::unique_ptr<Section> &&other) {
 	}
 	// LCOV_EXCL_STOP
 
-	// Note that the order in which fragments are stored in the `nextu` list does not
+	// Note that the order in which fragments are stored in the `nextPiece` list does not
 	// really matter, only that offsets were properly computed above
-	other->nextu = std::move(target.nextu);
-	target.nextu = std::move(other);
+	other->nextPiece = std::move(target.nextPiece);
+	target.nextPiece = std::move(other);
 }
 
 void sect_AddSection(std::unique_ptr<Section> &&section) {
diff --git a/src/link/symbol.cpp b/src/link/symbol.cpp
index 5fc02b36..8f5caaa9 100644
--- a/src/link/symbol.cpp
+++ b/src/link/symbol.cpp
@@ -90,3 +90,43 @@ void sym_TraceLocalAliasedSymbols(std::string const &name) {
 		}
 	}
 }
+
+void Symbol::linkToSection(Section &section) {
+	assume(std::holds_alternative<Label>(data));
+	Label &label = std::get<Label>(data);
+	// Link the symbol to the section
+	label.section = &section;
+	// Link the section to the symbol, keeping the section's symbol list sorted
+	uint32_t a = 0, b = section.symbols.size();
+	while (a != b) {
+		uint32_t c = (a + b) / 2;
+		assume(std::holds_alternative<Label>(section.symbols[c]->data));
+		Label const &other = std::get<Label>(section.symbols[c]->data);
+		if (other.offset > label.offset) {
+			b = c;
+		} else {
+			a = c + 1;
+		}
+	}
+	section.symbols.insert(section.symbols.begin() + a, this);
+}
+
+void Symbol::fixSectionOffset() {
+	if (!std::holds_alternative<Label>(data)) {
+		return;
+	}
+
+	Label &label = std::get<Label>(data);
+	Section *section = label.section;
+	assume(section);
+
+	if (section->modifier != SECTION_NORMAL) {
+		// Associate the symbol with the main section, not the "piece"
+		label.section = sect_GetSection(section->name);
+	}
+	if (section->modifier == SECTION_FRAGMENT) {
+		// Add the fragment's offset to the symbol's
+		// (`section->offset` is computed by `sect_AddSection`)
+		label.offset += section->offset;
+	}
+}