From 323028d9f229a413579bb6ee2c57b2fe5afea6f0 Mon Sep 17 00:00:00 2001
From: Sylvie <35663410+Rangi42@users.noreply.github.com>
Date: Fri, 6 Sep 2024 21:31:13 -0400
Subject: [PATCH] RGBLINK lists local symbols when encountering an unknown
 symbol reference (#1496)

---
 include/link/symbol.hpp                  |   2 ++
 src/link/object.cpp                      |   3 +-
 src/link/patch.cpp                       |   1 +
 src/link/sdas_obj.cpp                    |   1 +
 src/link/symbol.cpp                      |  35 +++++++++++++++++++++++
 test/link/symbols/{ => good}/a.asm       |   0
 test/link/symbols/{ => good}/b.asm       |   0
 test/link/symbols/{ => good}/out.err     |   0
 test/link/symbols/{ => good}/ref.out.bin | Bin
 test/link/symbols/{ => good}/ref.out.sym |   0
 test/link/symbols/unknown/a.asm          |   2 ++
 test/link/symbols/unknown/b.asm          |   2 ++
 test/link/symbols/unknown/c.asm          |   1 +
 test/link/symbols/unknown/d.asm          |   1 +
 test/link/symbols/unknown/e.asm          |   2 ++
 test/link/symbols/unknown/out.err        |   6 ++++
 test/link/test.sh                        |  17 +++++++++--
 17 files changed, 69 insertions(+), 4 deletions(-)
 rename test/link/symbols/{ => good}/a.asm (100%)
 rename test/link/symbols/{ => good}/b.asm (100%)
 rename test/link/symbols/{ => good}/out.err (100%)
 rename test/link/symbols/{ => good}/ref.out.bin (100%)
 rename test/link/symbols/{ => good}/ref.out.sym (100%)
 create mode 100644 test/link/symbols/unknown/a.asm
 create mode 100644 test/link/symbols/unknown/b.asm
 create mode 100644 test/link/symbols/unknown/c.asm
 create mode 100644 test/link/symbols/unknown/d.asm
 create mode 100644 test/link/symbols/unknown/e.asm
 create mode 100644 test/link/symbols/unknown/out.err

diff --git a/include/link/symbol.hpp b/include/link/symbol.hpp
index 34a36bf1..daa5c0e8 100644
--- a/include/link/symbol.hpp
+++ b/include/link/symbol.hpp
@@ -49,4 +49,6 @@ void sym_AddSymbol(Symbol &symbol);
  */
 Symbol *sym_GetSymbol(std::string const &name);
 
+void sym_DumpLocalAliasedSymbols(std::string const &name);
+
 #endif // RGBDS_LINK_SYMBOL_HPP
diff --git a/src/link/object.cpp b/src/link/object.cpp
index 76ac310f..32ceb86d 100644
--- a/src/link/object.cpp
+++ b/src/link/object.cpp
@@ -564,8 +564,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
 
 		readSymbol(file, symbol, fileName, nodes[fileID]);
 
-		if (symbol.type == SYMTYPE_EXPORT)
-			sym_AddSymbol(symbol);
+		sym_AddSymbol(symbol);
 		if (symbol.data.holds<Label>())
 			nbSymPerSect[symbol.data.get<Label>().sectionID]++;
 	}
diff --git a/src/link/patch.cpp b/src/link/patch.cpp
index 9390ac95..25d4de64 100644
--- a/src/link/patch.cpp
+++ b/src/link/patch.cpp
@@ -388,6 +388,7 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector<Symbol> const &fil
 					    "Unknown symbol \"%s\"",
 					    fileSymbols[value].name.c_str()
 					);
+					sym_DumpLocalAliasedSymbols(fileSymbols[value].name);
 					isError = true;
 				} else if (symbol->data.holds<Label>()) {
 					Label const &label = symbol->data.get<Label>();
diff --git a/src/link/sdas_obj.cpp b/src/link/sdas_obj.cpp
index d841257a..049a91d1 100644
--- a/src/link/sdas_obj.cpp
+++ b/src/link/sdas_obj.cpp
@@ -382,6 +382,7 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
 			// Expected format: /[DR]ef[0-9A-F]+/i
 			if (token[0] == 'R' || token[0] == 'r') {
 				symbol.type = SYMTYPE_IMPORT;
+				sym_AddSymbol(symbol);
 				// TODO: hard error if the rest is not zero
 			} else if (token[0] != 'D' && token[0] != 'd') {
 				fatal(&where, lineNo, "'S' line is neither \"Def\" nor \"Ref\"");
diff --git a/src/link/symbol.cpp b/src/link/symbol.cpp
index 41b474cc..293e91f2 100644
--- a/src/link/symbol.cpp
+++ b/src/link/symbol.cpp
@@ -4,6 +4,7 @@
 
 #include <stdlib.h>
 #include <unordered_map>
+#include <vector>
 
 #include "helpers.hpp" // assume
 
@@ -11,6 +12,7 @@
 #include "link/section.hpp"
 
 std::unordered_map<std::string, Symbol *> symbols;
+std::unordered_map<std::string, std::vector<Symbol *>> localSymbols;
 
 void sym_ForEach(void (*callback)(Symbol &)) {
 	for (auto &it : symbols)
@@ -18,6 +20,12 @@ void sym_ForEach(void (*callback)(Symbol &)) {
 }
 
 void sym_AddSymbol(Symbol &symbol) {
+	if (symbol.type != SYMTYPE_EXPORT) {
+		if (symbol.type != SYMTYPE_IMPORT)
+			localSymbols[symbol.name].push_back(&symbol);
+		return;
+	}
+
 	Symbol *other = sym_GetSymbol(symbol.name);
 	int32_t *symValue = symbol.data.holds<int32_t>() ? &symbol.data.get<int32_t>() : nullptr;
 	int32_t *otherValue =
@@ -41,3 +49,30 @@ Symbol *sym_GetSymbol(std::string const &name) {
 	auto search = symbols.find(name);
 	return search != symbols.end() ? search->second : nullptr;
 }
+
+void sym_DumpLocalAliasedSymbols(std::string const &name) {
+	std::vector<Symbol *> const &locals = localSymbols[name];
+	int count = 0;
+	for (Symbol *local : locals) {
+		if (count++ == 3) {
+			size_t remaining = locals.size() - 3;
+			bool plural = remaining != 1;
+			fprintf(
+			    stderr,
+			    "    ...and %zu more symbol%s with that name %s defined but not exported\n",
+			    remaining,
+			    plural ? "s" : "",
+			    plural ? "are" : "is"
+			);
+			break;
+		}
+		fprintf(
+		    stderr,
+		    "    A %s with that name is defined but not exported at ",
+		    local->data.holds<Label>() ? "label" : "constant"
+		);
+		assume(local->src);
+		local->src->dump(local->lineNo);
+		putc('\n', stderr);
+	}
+}
diff --git a/test/link/symbols/a.asm b/test/link/symbols/good/a.asm
similarity index 100%
rename from test/link/symbols/a.asm
rename to test/link/symbols/good/a.asm
diff --git a/test/link/symbols/b.asm b/test/link/symbols/good/b.asm
similarity index 100%
rename from test/link/symbols/b.asm
rename to test/link/symbols/good/b.asm
diff --git a/test/link/symbols/out.err b/test/link/symbols/good/out.err
similarity index 100%
rename from test/link/symbols/out.err
rename to test/link/symbols/good/out.err
diff --git a/test/link/symbols/ref.out.bin b/test/link/symbols/good/ref.out.bin
similarity index 100%
rename from test/link/symbols/ref.out.bin
rename to test/link/symbols/good/ref.out.bin
diff --git a/test/link/symbols/ref.out.sym b/test/link/symbols/good/ref.out.sym
similarity index 100%
rename from test/link/symbols/ref.out.sym
rename to test/link/symbols/good/ref.out.sym
diff --git a/test/link/symbols/unknown/a.asm b/test/link/symbols/unknown/a.asm
new file mode 100644
index 00000000..aa5541a9
--- /dev/null
+++ b/test/link/symbols/unknown/a.asm
@@ -0,0 +1,2 @@
+section "a", rom0
+ld hl, Label
diff --git a/test/link/symbols/unknown/b.asm b/test/link/symbols/unknown/b.asm
new file mode 100644
index 00000000..da581a83
--- /dev/null
+++ b/test/link/symbols/unknown/b.asm
@@ -0,0 +1,2 @@
+section "b", rom0
+Label:
diff --git a/test/link/symbols/unknown/c.asm b/test/link/symbols/unknown/c.asm
new file mode 100644
index 00000000..5f8d1837
--- /dev/null
+++ b/test/link/symbols/unknown/c.asm
@@ -0,0 +1 @@
+def Label equ 42
diff --git a/test/link/symbols/unknown/d.asm b/test/link/symbols/unknown/d.asm
new file mode 100644
index 00000000..d187e33b
--- /dev/null
+++ b/test/link/symbols/unknown/d.asm
@@ -0,0 +1 @@
+def Label = 123
diff --git a/test/link/symbols/unknown/e.asm b/test/link/symbols/unknown/e.asm
new file mode 100644
index 00000000..e5bc5dbf
--- /dev/null
+++ b/test/link/symbols/unknown/e.asm
@@ -0,0 +1,2 @@
+section "e", wram0
+Label:
diff --git a/test/link/symbols/unknown/out.err b/test/link/symbols/unknown/out.err
new file mode 100644
index 00000000..40ada55c
--- /dev/null
+++ b/test/link/symbols/unknown/out.err
@@ -0,0 +1,6 @@
+error: symbols/unknown/a.asm(2): Unknown symbol "Label"
+    A label with that name is defined but not exported at symbols/unknown/b.asm(2)
+    A constant with that name is defined but not exported at symbols/unknown/c.asm(1)
+    A constant with that name is defined but not exported at symbols/unknown/d.asm(1)
+    ...and 1 more symbol with that name is defined but not exported
+Linking failed with 1 error
diff --git a/test/link/test.sh b/test/link/test.sh
index 95b8f26b..54b05569 100755
--- a/test/link/test.sh
+++ b/test/link/test.sh
@@ -8,10 +8,11 @@ gbtemp="$(mktemp)"
 gbtemp2="$(mktemp)"
 outtemp="$(mktemp)"
 outtemp2="$(mktemp)"
+outtemp3="$(mktemp)"
 
 # Immediate expansion is the desired behavior.
 # shellcheck disable=SC2064
-trap "rm -f ${otemp@Q} ${gbtemp@Q} ${gbtemp2@Q} ${outtemp@Q} ${outtemp2@Q}" EXIT
+trap "rm -f ${otemp@Q} ${gbtemp@Q} ${gbtemp2@Q} ${outtemp@Q} ${outtemp2@Q} ${outtemp3@Q}" EXIT
 
 tests=0
 failed=0
@@ -316,7 +317,7 @@ for i in section-union/*.asm; do
 	evaluateTest
 done
 
-test="symbols"
+test="symbols/good"
 startTest
 "$RGBASM" -o "$otemp" "$test"/a.asm
 "$RGBASM" -o "$gbtemp2" "$test"/b.asm
@@ -327,6 +328,18 @@ tryDiff "$test"/ref.out.sym "$outtemp2"
 tryCmpRom "$test"/ref.out.bin
 evaluateTest
 
+test="symbols/unknown"
+startTest
+"$RGBASM" -o "$otemp" "$test"/a.asm
+"$RGBASM" -o "$gbtemp" "$test"/b.asm
+"$RGBASM" -o "$gbtemp2" "$test"/c.asm
+"$RGBASM" -o "$outtemp" "$test"/d.asm
+"$RGBASM" -o "$outtemp2" "$test"/e.asm
+continueTest
+rgblinkQuiet "$otemp" "$gbtemp" "$gbtemp2" "$outtemp" "$outtemp2" 2>"$outtemp3"
+tryDiff "$test"/out.err "$outtemp3"
+evaluateTest
+
 if [[ "$failed" -eq 0 ]]; then
 	echo "${bold}${green}All ${tests} tests passed!${rescolors}${resbold}"
 else