-
-
Notifications
You must be signed in to change notification settings - Fork 185
Hoisting #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Hoisting #369
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
4290749
initial hoisting implementation (needs tests)
tomblind 968b12a
fixed issues with switch and loop continue
tomblind 73929f9
Merge remote-tracking branch 'origin/ts-to-lua-ast' into hoisting
tomblind 86c7bd4
dealing with hoisting of imports and exports
tomblind 5bc6764
Merge remote-tracking branch 'origin/ts-to-lua-ast' into hoisting
tomblind e2374c5
hoisting tests
tomblind 7833a65
fixed namespace hoisting and added more tests
tomblind f378a01
Merge remote-tracking branch 'origin/master' into hoisting
tomblind 23cde6a
rebuilt translation tests with hoisting
tomblind 5fff23c
fixed tests
tomblind fbf7072
Merge remote-tracking branch 'origin/master' into hoisting
tomblind 4927c88
More Hoisting Adjustments
tomblind 77303dc
reverting accidentally changed test files
tomblind e167812
renamed some things for clarity
tomblind 3000e3b
working on possible smart-hoisting detection
tomblind 8592d4a
Merge remote-tracking branch 'origin/master' into hoisting
tomblind b4157b3
addressed comments
tomblind 086465f
Merge remote-tracking branch 'origin/hoisting' into hoisting-v2
tomblind 27bb3b5
smart-hoisting working
tomblind 58b5237
fixed tests
tomblind 6fabe14
refactored hoisting detection and added command line options to contr…
tomblind baf226e
Another full refactor
tomblind 67c44b6
change ScopeType to bitfields
tomblind cd47351
Merge remote-tracking branch 'origin/master' into hoisting
tomblind 31c6890
rewrote hoisting to defer all logic to popScope, so that there's no n…
tomblind 1a846b8
reworked hoisting to avoid printer hacks
tomblind 798bb27
removing unnecessary symbol lookups
tomblind eadfa75
Merge remote-tracking branch 'origin/master' into hoisting
tomblind c9c564b
removing accidentally added import
tomblind d6e83e2
replaced ts.Symbol stored in identifiers with generic ids
tomblind 922380f
split out stuff to separate functions
tomblind 9edd0bc
Merge remote-tracking branch 'origin/master' into hoisting
tomblind 01191da
reverted bad logic which broke referencing exports inside functions
tomblind File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.