Skip to content

Commit

Permalink
Require labels in functions to be unique
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
SquidDev committed Nov 8, 2023
1 parent 5cc8cd6 commit 981d5ca
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 6 deletions.
5 changes: 3 additions & 2 deletions src/main/java/org/squiddev/cobalt/compiler/FuncState.java
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/squiddev/cobalt/compiler/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 6 additions & 2 deletions src/test/resources/spec/goto_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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'
Expand Down

0 comments on commit 981d5ca

Please sign in to comment.