Skip to content

Disallow usage of goto and label in Lua 5.1#124

Merged
Perryvw merged 5 commits intoTypeScriptToLua:masterfrom
zengjie:master
Jun 25, 2018
Merged

Disallow usage of goto and label in Lua 5.1#124
Perryvw merged 5 commits intoTypeScriptToLua:masterfrom
zengjie:master

Conversation

@zengjie
Copy link
Copy Markdown
Contributor

@zengjie zengjie commented Jun 25, 2018

Lua 5.1 does not support goto and label. Using goto and label to simulate continue is not possible for Lua 5.1 target.

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Jun 25, 2018

Will merge if you fix the tests.

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Jun 25, 2018

Actually, as @lolleko pointed out to me, this would mess up our inheritance tree. You should move the current goto/continue implementation to the 5.2 implementation and then put this 'default' version in the general transpiler.ts. We will add some documentation to the wiki about this.

@zengjie
Copy link
Copy Markdown
Contributor Author

zengjie commented Jun 25, 2018

OK. I will fix the tests. But I don't understand why this would mess up the inheritance tree. The current goto/continue implementation is valid for Lua 5.2/5.3/JIT and should be the 'default' version. Lua 5.1 is the special case for this problem.

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Jun 25, 2018

@zengjie We aim to implement versions as described here: https://github.com/Perryvw/TypescriptToLua/wiki/Different-Lua-Versions

The different versions of transpilers are currently chained so we have as little code duplication as possible. If the current loop version is implemented in 5.2 it will also be used for later versions.

@zengjie
Copy link
Copy Markdown
Contributor Author

zengjie commented Jun 25, 2018

Brilliant Idea! I got it.

@zengjie
Copy link
Copy Markdown
Contributor Author

zengjie commented Jun 25, 2018

@Perryvw I have finished the adjustment.

Copy link
Copy Markdown
Member

@Perryvw Perryvw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for contributing! I have added a comment regarding the yarn.lock you comitted. Once that is resolved I will merge.

import * as ts from "typescript";

export class LuaTranspiler51 extends LuaTranspiler {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary

yarn.lock Outdated
@@ -0,0 +1,2070 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't commit this please (I would be okay with a commit adding this to .gitignore)

@Perryvw Perryvw merged commit bbaf3ae into TypeScriptToLua:master Jun 25, 2018
@Janne252
Copy link
Copy Markdown
Contributor

Is this available in the 0.5.0 release? I'm still getting generated ::__continue0:: labels with "luaTarget": "5.1".

@Perryvw
Copy link
Copy Markdown
Member

Perryvw commented Jul 26, 2018

It was included in the 0.6.0 release 11 days ago.

@Janne252
Copy link
Copy Markdown
Contributor

It appears that the npm version is still 0.5.0. I suppose I'll have to work with a local clone of the repository until the npm gets updated. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants