add support for continue statement by InfiniteRain · Pull Request #112 · TypeScriptToLua/TypeScriptToLua

InfiniteRain · 2018-06-04T14:18:35Z

This implements the support for TS's continue statement. Normally, tstl would throw an exception whenever the continue statement is used, saying that Lua does not support the statement.

However, while Lua indeed does not support the continue statement, it is still perfectly possible to implement the functionality of a continue statement by changing how loops get transpiled.

Here's a simple example:

The following TS

for (let i = 1; i <= 100; i++) {
  if (i < 25) {
    continue;
  }
  
  print(i);
}

Gets transpiled to

local i = 1
while(i<=100) do
    local ____continue = false
    repeat
        if i<25 then
            ____continue = true
            break
        end
        print(i)
        ____continue = true
    until true
    i=i+1
    if not ____continue then break end
end

As can be seen from the example above, the functionality of a continue statement gets achieved via a clever usage of the repeat until true block, which acts a simple do end block, but allows for a possibility of stopping the execution of the block via a break statement. The ____continue local allows for differentiation between a break statement for the repeat until true and the parent loop. If a break statement was used without setting the ____continue local to true, then it will have an effect of a normal break (break from repeat until true and break from the parent loop). However, if a break statement was used after setting the ____continue local to true, then it will act with accordance to how the effect of the continue statement should be, which is: stop executing the current cycle, and jump to the next one (break repeat until true, but do not break the parent loop). The ____continue = true gets automatically added at the end of the body of the parent loop.

So, the continue statement simply gets transpiled to:

____continue = true
break

With this implementation, you can nest multiple loops inside of each other, and the ____continue locals of the nested loops will not interfere with each other, thanks to how scoping works in Lua.

The following loops were used for testing:

declare function print(...messages: any[]): void;

for (let i = 1; i <= 100; i++) {
  if (i < 25) {
    continue;
  }
  print(i);

  for (let x = 1; x <= 10; x++) {
    if (x < 5) {
      continue;
    }
    print(x)
  }
}

let d = 10
while (d > 0) {
  d--;
  if (d > 5) {
    let o = 4;
    while (o > 0) {
      o--;
      if (o > 2) {
        continue;
      }
      print(o);
    }
    continue;
  }
  if (d == 2) {
    continue;
  }
  print(d);
}

let e = 10
do {
  e--;
  if (e > 5) {
    let o = 4;
    do {
      o--;
      if (o > 2) {
        continue;
      }
      print(o);
    } while (o > 0)
    continue;
  }
  if (e == 2) {
    continue;
  }
  print(e);
} while (e > 0)

for (let i of [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) {
  if (i < 3) {
    continue;
  }

  for (let i of [5, 6, 7, 8]) {
    if (i !== 6) {
      continue
    }
    print(i)
  }

  if (i > 8) {
    continue;
  }

  print(i);
}

for (let i in {
  a: 1,
  b: 2,
  c: 3,
  d: 4
}) {
 if (i == 'a') {
   continue;
 }

 if (i == 'b') {
   continue;
 }

 print(i);
}

The transpiled code:

-- Generated by TypescriptToLua v0.2.1
-- https://github.com/Perryvw/TypescriptToLua
require("typescript_lualib")
local i = 1
while(i<=100) do
    local ____continue = false
    repeat
        if i<25 then
            ____continue = true
            break
        end
        print(i)
        local x = 1
        while(x<=10) do
            local ____continue = false
            repeat
                if x<5 then
                    ____continue = true
                    break
                end
                print(x)
                ____continue = true
            until true
            x=x+1
            if not ____continue then break end
        end
        ____continue = true
    until true
    i=i+1
    if not ____continue then break end
end
local d = 10

while d>0 do
    local ____continue = false
    repeat
        d=d-1
        if d>5 then
            local o = 4

            while o>0 do
                local ____continue = false
                repeat
                    o=o-1
                    if o>2 then
                        ____continue = true
                        break
                    end
                    print(o)
                    ____continue = true
                until true
                if not ____continue then break end
            end
            ____continue = true
            break
        end
        if d==2 then
            ____continue = true
            break
        end
        print(d)
        ____continue = true
    until true
    if not ____continue then break end
end
local e = 10

repeat
    local ____continue = false
    repeat
        e=e-1
        if e>5 then
            local o = 4

            repeat
                local ____continue = false
                repeat
                    o=o-1
                    if o>2 then
                        ____continue = true
                        break
                    end
                    print(o)
                    ____continue = true
                until true
                if not ____continue then break end
            until not (o>0)
            ____continue = true
            break
        end
        if e==2 then
            ____continue = true
            break
        end
        print(e)
        ____continue = true
    until true
    if not ____continue then break end
until not (e>0)
for _, i in ipairs({1,2,3,4,5,6,7,8,9,10}) do
    local ____continue = false
    repeat
        if i<3 then
            ____continue = true
            break
        end
        for _, i in ipairs({5,6,7,8}) do
            local ____continue = false
            repeat
                if i~=6 then
                    ____continue = true
                    break
                end
                print(i)
                ____continue = true
            until true
            if not ____continue then break end
        end
        if i>8 then
            ____continue = true
            break
        end
        print(i)
        ____continue = true
    until true
    if not ____continue then break end
end
for i, _ in pairs({a = 1,b = 2,c = 3,d = 4}) do
    local ____continue = false
    repeat
        if i=="a" then
            ____continue = true
            break
        end
        if i=="b" then
            ____continue = true
            break
        end
        print(i)
        ____continue = true
    until true
    if not ____continue then break end
end

The outcomes match.

I don't really know if there's a certain way that the pull requests should be made, or if there's some kind of standard for the workflow. If I've done something wrong with the presentation, then please don't hesitate to correct me. :)

lolleko · 2018-06-04T16:22:18Z

Thanks for the PR, I don't think continue is good style and should be used at all. But its probably a good thing to have support for it anyway.

Also I think it would be much cleaner to use Lua 5.2s goto statement. The output looks a bit nicer that way.
http://lua-users.org/wiki/GotoStatement

And please add at least some translation tests and if you want some functional tests.

InfiniteRain · 2018-06-04T16:34:52Z

I have been searching for this kind of project for a while, since I have some old games that I would love to mod, but they support only Lua 5.1; that was the reason for me going with this solution. I could implement the goto solution for cases when Lua 5.2 or 5.3 is used, but I would also love to support Lua 5.1. So would you mind if I added the support for both?

And you're right, I should probably add more tests with this feature.

Edit: Also I disagree with the notion that continue should not be used, I have encountered a few cases where the implementation of continue helps a lot. But I guess, that boils down to opinion.

lolleko · 2018-06-04T16:48:11Z

We dropped support for Lua versions below 5.3 a while ago because we wanted to implement all Lua features first, without having to think of backwards-compatibility all the time.

But we already have a system to transpile differently for different version, we use that for bit operations for example, so if you want to add 5.1 support please go ahead. (Unless @Perryvw has an issue with that.)

While your at it you can also look at some other features that won't work in 5.1 (e.g bit ops, can't think of more right now) and throw exceptions if those are used when the 5.1 flag is set.

Another thing make sure you don't add additional overhead if no continue is used in a loop and use goto in versions <5.1

InfiniteRain · 2018-06-04T16:58:16Z

Alright then, I think that it would be wise for me to go with the goto implementation for this PR, and implement the 5.1 compatibility in a different batch with a different PR.

On a side note, is there a list of the Lua features that are already implemented and features that are still being worked on? If I remember correctly, things such as typeof keyword don't work properly yet.

lolleko · 2018-06-04T17:03:57Z

Sounds good 👍

We have a list of things that don't work, but I don't think everything is listed there: https://github.com/Perryvw/TypescriptToLua/wiki/Limitations

InfiniteRain · 2018-06-04T17:07:51Z

And tell me if I got this right: the plan is to get rid of all the limitations, correct?

Perryvw · 2018-06-04T17:14:12Z

I would say the nicest way to do this is maintain one 'main' transpiler class and have a different extension class for other versions that override required methods. We can then simply select between them from compiler.ts

InfiniteRain · 2018-06-04T17:15:40Z

Alright, got it. Thank you.

Perryvw

A couple things, I am seeing a lot of code reuse: it would probably be a good idea to make a transpileLoopBody method.

Furthermore you should move tests that just check transpilation string result to translation tests. Furthermore I would appreciate some unit tests where you actually execute some transpiled code with continues.

Perryvw · 2018-06-04T17:56:28Z

src/Transpiler.ts


        let result = this.indent + `while ${condition} do\n`;
        this.pushIndent();
+        result += this.indent + "local ____continue = false\n";


4 _ seems a little overkill, we generally use 2 for transpiler stuff

Perryvw · 2018-06-04T17:59:51Z

test/unit/loops.spec.ts

+                ____continue = true
+            until true
+            if not ____continue then break end
+        end`;


This should be a translation test. (Just add the typescript to translation/ts and the lua to translation/lua with the same name and it will automatically include the test).

Perryvw · 2018-06-04T18:00:21Z

test/unit/while.spec.ts

+            repeat
+                a=a+1
+                ____continue = true
+            until true if not ____continue then break end


Another translation test.

InfiniteRain · 2018-06-06T13:14:06Z

Ok, I've done everything that was requested:

Since Lua 5.1 support is not considered (for now), I redone the continue functionality via the Lua 5.2's goto statements.
Removed additional overhead if continue statement is not used inside of the loop.
Created a function transpileLoop which heavily decreases code reusage.
Added translation tests to the correct places.
Added a bunch of unit tests.

Perryvw · 2018-06-06T19:27:47Z

Thanks for the update! I like the functional tests you added, this seems like a good test coverage.

I am not such a fan of the way this is implemented however. After discussing with @lolleko we came to a couple points:

You can just add a label __continue{n} at the end of each loop. Generate the n by keeping a stack of loop names. When you start transpiling a loop push and increment genVarCounter, transpile continue as goto __continue{m} where m is the loop number at the top of the stack. Pop from the stack when you're done transpiling that loop.
The structure we had before with transpileFor and transpileForOf etc was fine, what I meant with transpileLoopBody was purely refactoring the body, as indicated in the following image: https://i.imgur.com/FlsYmch.png
I think the removal of the old structure is a step in the wrong direction and should probably be reverted.
If you do the goto/label method you can get rid of hasContinue completely, we generally try to avoid getChildren(), and just labels will probably not have any performance impact anyway.

InfiniteRain · 2018-06-06T19:49:19Z

My bad, for some reason I completely missed the Body part from your initial message. Changes will follow shortly.

InfiniteRain · 2018-06-06T19:59:42Z

Question, should I be using the genVarCounter variable for keeping track of the stack? Or should I create a new variable designed to be used with loops only? Your message suggested that I should use genVarCounter, but if I do, it will interfere with switch/case statements.

Perryvw · 2018-06-06T20:05:48Z

I suggest creating a new stack to keep track of the numbers of nested loops for when you need to write a goto in one of them. You can simply populate the stack by using genVarCounter. If any time you start using a new genVarCounter value you simply increment it, it should not interfere with switch/case statements.

Perryvw · 2018-06-06T20:19:16Z

The namespace array is also used as a stack by the way, you can look at how that is used.

InfiniteRain · 2018-06-06T20:36:29Z

Requested changes are now implemented.

lolleko

Commented on some minor stuff. Other than that really good work, this looks like a solid, well tested solution now.

lolleko · 2018-06-06T22:15:08Z

src/Transpiler.ts

+        result += this.transpileStatement(node.statement);
+        this.popIndent();
+        result += this.indent + "end\n";
+        result += this.indent + `::__continue${this.loopStack[this.loopStack.length - 1]}::\n`;


You can just use this.loopStack.pop() instead of this.loopStack[this.loopStack.length - 1]. and remove the pop on the next line. pop will return the last value.

lolleko · 2018-06-06T22:17:01Z

src/Transpiler.ts

        const condition = this.transpileExpression(node.expression);

        let result = this.indent + `while ${condition} do\n`;
        this.pushIndent();


you can push the indent inside the transpileLoopBody function, this will reduce code reuse even more.

I disagree with this. If you look at the regular for loop it translates the incrementor after the loop body but inside the indent. If you move the indent inside the loop body method this no longer works. You could ofcourse move only the push inside but I think that is bad form, push and pop should be obvious in the same context.

Perryvw

Thanks for the changes, they look very good! I have only one issue and that is that none of your translation tests actually contain a continue. Also having a test with a nested loop would be nice. Once you address that and the other minor comments in the code this looks ready to merge to me!

Perryvw · 2018-06-07T06:59:19Z

src/Transpiler.ts

+            | ts.ForInStatement
+    ): string {
+        this.genVarCounter++;
+        this.loopStack.push(this.genVarCounter);


Increment genVarCounter after you use it instead of before (same as switch does)

add support for continue statement

bad7b96

Perryvw requested changes Jun 4, 2018

View reviewed changes

make us of lua goto; decrease code reusage; add tests

37f844c

implement requested changes

66cacc4

lolleko approved these changes Jun 6, 2018

View reviewed changes

Perryvw requested changes Jun 7, 2018

View reviewed changes

InfiniteRain and others added 2 commits June 7, 2018 12:25

add continue translation tests; minor requested changes

08216d6

Merge branch 'master' of https://github.com/Perryvw/TypescriptToLua

78e8bc8

Perryvw approved these changes Jun 7, 2018

View reviewed changes

fix tupleArrayUses translation test

61675ae

lolleko merged commit 7f39d57 into TypeScriptToLua:master Jun 7, 2018

YoRyan mentioned this pull request Oct 16, 2023

Support loop continue for 5.0, 5.1, and universal targets #1500

Merged

Uh oh!

Conversation

InfiniteRain commented Jun 4, 2018

Uh oh!

lolleko commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

InfiniteRain commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lolleko commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

InfiniteRain commented Jun 4, 2018

Uh oh!

lolleko commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

InfiniteRain commented Jun 4, 2018

Uh oh!

Perryvw commented Jun 4, 2018

Uh oh!

InfiniteRain commented Jun 4, 2018

Uh oh!

Perryvw left a comment

Choose a reason for hiding this comment

Uh oh!

Perryvw Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

Perryvw Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

Perryvw Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

InfiniteRain commented Jun 6, 2018

Uh oh!

Perryvw commented Jun 6, 2018

Uh oh!

InfiniteRain commented Jun 6, 2018

Uh oh!

InfiniteRain commented Jun 6, 2018

Uh oh!

Perryvw commented Jun 6, 2018

Uh oh!

Perryvw commented Jun 6, 2018

Uh oh!

InfiniteRain commented Jun 6, 2018

Uh oh!

lolleko left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lolleko Jun 6, 2018

Choose a reason for hiding this comment

Uh oh!

lolleko Jun 6, 2018

Choose a reason for hiding this comment

Uh oh!

Perryvw Jun 7, 2018

Choose a reason for hiding this comment

Uh oh!

Perryvw left a comment

Choose a reason for hiding this comment

Uh oh!

Perryvw Jun 7, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lolleko commented Jun 4, 2018 •

edited

Loading

InfiniteRain commented Jun 4, 2018 •

edited

Loading

lolleko commented Jun 4, 2018 •

edited

Loading

lolleko commented Jun 4, 2018 •

edited

Loading

lolleko left a comment •

edited

Loading