Conversation
- pushing scope for try/catch/finally - some refactoring and commenting - fixed more tests
src/LuaTransformer.ts
Outdated
| return tstl.createBinaryExpression(expression, tstl.createNumericLiteral(1), tstl.SyntaxKind.AdditionOperator); | ||
| } | ||
|
|
||
| protected findScope(...scopeTypes: ScopeType[]): Scope | undefined { |
There was a problem hiding this comment.
Might be interesting to use a bitmask for this instead of an array.
There was a problem hiding this comment.
that's a good idea!
- checking variables for reference-before-declaration when they are first seen to avoid scanning later - caching nested functions in scope to avoid repeat scan later - added error for attempting to hoist when hoisting is off since it's easy to detect now - added tests for hoisting errors
|
Ok, this PR is probably worth reviewing again. There are now 3 options for hoisting:
|
src/CommandLineParser.ts
Outdated
| type: "boolean", | ||
| }, | ||
| hoisting: { | ||
| alias: "h", |
There was a problem hiding this comment.
h is usually help, maybe choice something different or don't set an alias at all.
src/LuaAST.ts
Outdated
| } | ||
| if (!node.tsOriginal) { | ||
| node.tsOriginal = parent.tsOriginal; | ||
| } |
There was a problem hiding this comment.
It is intentionally avoided to link the Lua AST node to the tsOriginal. Is this really required?
There was a problem hiding this comment.
I think I can rework things to avoid it, but I'm curious what the reason is for avoiding it? Seems like it would be quite useful.
…eed for additional scanning
| expression.text = text as string; | ||
| expression.symbol = symbol; | ||
| return expression; | ||
| } |
There was a problem hiding this comment.
Again is it possible to avoid having ts types in the Lua AST?
Sorry I haven't elaborated on why I don't think TS stuff should be in an Lua AST.
Main reason, it's a design choice: Having no TS nodes in the AST clearly separates the transform and print step. Allowing them to run independent of each other. Not having any TS nodes in the AST allows us to easily create Lua ASTs from the ground up (with no TS used to generate the AST).
I think a good compromise would be do just save the symbolID on the AST node, since that would just be a number and not a ts type. And as far as I can tell you only save the symbol to keep track on what should be hoisted? I think the ID could be enough for that purpose.
Or maybe map the tstl.Identifier to the Symbol somewhere else? For example in a Map<> in the Transformer, that way we wouldn't have to store the information on the node itself. Since you don't need that info in there print step there is no reason to save it on the node tbh.
There was a problem hiding this comment.
Ok, thanks for the explanation - that makes sense to me. All I need here is a way to differentiate unique symbols, so an ID system like you suggest should work.
No description provided.