Skip to content
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

Codegen Lua 5.1 friendly string literals #762

Merged
merged 7 commits into from
Jul 22, 2024

Conversation

euclidianAce
Copy link
Member

This gets the job done, but is arguably inelegant. gsubing a large string literal a few times will probably have some performance implications and feels hackish. And they get run for every string literal whether it has escapes or not. I think a better solution might be to handle escapes at lex time and stash a list of byte offsets in the Token for easier translation at codegen time. So consider this a functional first draft/mvp.

Copy link

Teal Playground URL: https://762--teal-playground-preview.netlify.app

@euclidianAce
Copy link
Member Author

Ha, funnily enough that 5.1 test failure is what this is trying to solve.

@hishamhm
Copy link
Member

Nice!

gsubing a large string literal a few times will probably have some performance implications and feels hackish. And they get run for every string literal whether it has escapes or not.

I've added a check here: https://github.com/teal-language/tl/tree/euclidianAce-string-codegen-check

I think a better solution might be to handle escapes at lex time and stash a list of byte offsets in the Token for easier translation at codegen time.

I've also tried my hand implementing your suggestion, based on your code: https://github.com/teal-language/tl/tree/euclidianAce-string-codegen-escapes

I did some light benchmarking with hyperfine and I couldn't see clear performance differences (the variation caused by CPU throttling in my laptop was greater than the difference across branches).

What do think about these variations?

@euclidianAce
Copy link
Member Author

If there isn't a blatant performance degradation then this is probably fine to merge (barring any gen_compat checks that should be added).

tl.tl Outdated Show resolved Hide resolved
@hishamhm hishamhm merged commit 288966c into teal-language:master Jul 22, 2024
8 checks passed
@hishamhm
Copy link
Member

Merged!

@euclidianAce euclidianAce deleted the string-codegen branch July 22, 2024 23:59
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.

None yet

2 participants