From 981d5cac7c0fd623a442a0cf12e699194fe88be5 Mon Sep 17 00:00:00 2001
From: Jonathan Coates <git@squiddev.cc>
Date: Wed, 8 Nov 2023 18:10:49 +0000
Subject: [PATCH] Require labels in functions to be unique

While one cannot have duplicate labels within a block, Lua 5.2 and 5.3
allowed duplicate labels within a function, jumping to the label in the
closest block.

Lua 5.4 removed this feature, requiring unique labels across a function.
We apply the same change for Cobalt, as (IMO) label resolution rules are
confusing.

Note, this does *not* port the rest of Lua 5.4's goto code. Lua 5.4 now
searches all active labels when jumping (not just those in the current
block). This simplifies the code, but also generates slightly different
bytecode (as it can't eliminate closes in as many cases). To aid
testing, we leave the rest of the code as-is.
---
 src/main/java/org/squiddev/cobalt/compiler/FuncState.java | 5 +++--
 src/main/java/org/squiddev/cobalt/compiler/Parser.java    | 4 ++--
 src/test/resources/spec/goto_spec.lua                     | 8 ++++++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/squiddev/cobalt/compiler/FuncState.java b/src/main/java/org/squiddev/cobalt/compiler/FuncState.java
index c2e22a44..6283551c 100644
--- a/src/main/java/org/squiddev/cobalt/compiler/FuncState.java
+++ b/src/main/java/org/squiddev/cobalt/compiler/FuncState.java
@@ -81,12 +81,13 @@ static class BlockCnt {
 
 	short activeVariableCount;  /* number of active local variables */
 
-	final int firstLocal;
+	final int firstLocal, firstLabel;
 
-	FuncState(Lex lexer, FuncState prev, int firstLocal) {
+	FuncState(Lex lexer, FuncState prev, int firstLocal, int firstLabel) {
 		this.lexer = lexer;
 		this.prev = prev;
 		this.firstLocal = firstLocal;
+		this.firstLabel = firstLabel;
 	}
 
 	Prototype toPrototype() {
diff --git a/src/main/java/org/squiddev/cobalt/compiler/Parser.java b/src/main/java/org/squiddev/cobalt/compiler/Parser.java
index f4d1299d..62377e05 100644
--- a/src/main/java/org/squiddev/cobalt/compiler/Parser.java
+++ b/src/main/java/org/squiddev/cobalt/compiler/Parser.java
@@ -528,7 +528,7 @@ private void codeClosure(Prototype childPrototype, ExpDesc v) throws CompileExce
 
 	FuncState openFunc() throws CompileException {
 		if (fs != null) checkLimit(fs, fs.children.size(), MAXARG_Bx, "functions");
-		FuncState fs = new FuncState(lexer, this.fs, activeVariableSize);
+		FuncState fs = new FuncState(lexer, this.fs, activeVariableSize, activeLabels.size());
 		this.fs = fs;
 		enterBlock(fs, false);
 		return fs;
@@ -1061,7 +1061,7 @@ private void gotoStat(int pc) throws CompileException, LuaError, UnwindThrowable
 	 * @param name The name of the label.
 	 */
 	private void checkRepeated(LuaString name) throws CompileException {
-		for (int i = fs.block.firstLabel; i < activeLabels.size(); i++) {
+		for (int i = fs.firstLabel; i < activeLabels.size(); i++) {
 			var label = activeLabels.get(i);
 			if (label.name == name) throw semError("label '" + name + "' already defined on line " + label.line);
 		}
diff --git a/src/test/resources/spec/goto_spec.lua b/src/test/resources/spec/goto_spec.lua
index ed37ac88..27ad679c 100644
--- a/src/test/resources/spec/goto_spec.lua
+++ b/src/test/resources/spec/goto_spec.lua
@@ -2,7 +2,7 @@ describe("goto statements :lua>=5.2", function()
 	describe("parse failures", function()
 		local function fail_parse(code, err)
 			local st, msg = load(code)
-			expect(st):describe("Code failed to load"):eq(nil)
+			expect(st):describe("Resulting function"):eq(nil)
 			expect(msg):str_match(err)
 		end
 
@@ -15,6 +15,10 @@ describe("goto statements :lua>=5.2", function()
 			fail_parse([[ ::l1:: ::l1:: ]], "label 'l1' already defined")
 		end)
 
+		it("repeated label in different blocks :lua>=5.4", function()
+			fail_parse([[::l1:: do ::l1:: end]], "label 'l1' already defined")
+		end)
+
 		it("undefined label", function()
 			fail_parse([[ goto l1; local aa ::l1:: ::l2:: print(3) ]], "into the scope of local 'aa'")
 		end)
@@ -79,7 +83,7 @@ describe("goto statements :lua>=5.2", function()
 	end
 
 	-- This fails on Lua 5.4 as labels cannot repeat.
-	it_str("goes to nearest label :lua<5.4", [[
+	it_str("goes to nearest label :lua<5.4 :!cobalt", [[
 		::l3::
 		-- goto to correct label when nested
 		do goto l3; ::l3:: end   -- does not loop jumping to previous label 'l3'