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

feature/impl-luau-lib: Add support for luau.compile and luau.load. #82

Merged
merged 24 commits into from
Aug 10, 2023
Merged

feature/impl-luau-lib: Add support for luau.compile and luau.load. #82

merged 24 commits into from
Aug 10, 2023

Conversation

4x8Matrix
Copy link
Contributor

@4x8Matrix 4x8Matrix commented Aug 9, 2023

Context

Lune at the moment has no way to optimally run source/bytecode, this PR aims to try and implement a new luau library that allows developers to create Luau closures in their code.

This PR attempts to bring together some of the points made in [RFC] Lune built-in luau library, however, discards the seySafeEnv function call as i'm not certain how we'd go about doing that under mlua

(Would be happy to discuss this though!)

This PR

  • Implement a luau Lune library.
  • Implement luau.compile function to compile a string into bytecode.
    • Implement an optional options table allowing developers to set optimization_level, coverage_level and debug_level
  • Implement luau.load function to load either bytecode or a source string into a function.
    • Implement an optional options table allowing developers to set debug_name.
  • Implement tests for the luau Lune library
  • Add documentation types for luau Lune library

Yet to-do

  • N/A

@4x8Matrix 4x8Matrix marked this pull request as ready for review August 10, 2023 00:10
Copy link
Collaborator

@filiptibell filiptibell left a comment

Choose a reason for hiding this comment

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

Left a few comments - I think everything looks pretty good, not sure exactly how we would expand the testing suite here, but if you think of anything feel free to add some more tests since this is such a low level feature.

src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved

assert(type(luau.compile) == "function", "expected `luau.compile` to be a function")

assert(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any way to verify that this is actually bytecode and not an arbitrary string? Does Luau bytecode start with any specific magic header or somesuch?

Copy link
Contributor Author

@4x8Matrix 4x8Matrix Aug 10, 2023

Choose a reason for hiding this comment

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

From what I can tell - Luau bytecode does start off with the version of the bytecode, but the bytes after that go into the content of that bytecode.

<bytecode-ver> ...

I've found this from both of these sources;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Edit: Additionally, if a compilation fails - instead of producing an error, the luau.compile function will return a message where the first byte is 0, and the error is attached afterwards. (luau.load accounts for this, but i'll get it tested none-the-less!)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Additionally, if a compilation fails - instead of producing an error, the luau.compile function will return a message where the first byte is 0, and the error is attached afterwards

I think it would probably be a good idea to check for this on the rust end of things for the compile function and throw a lua error then, too. We don't really have any other APIs that behave like this in Lune where they return an error message, if the user wants to handle errors/fallible functions pcall should be the go-to for consistency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the above changes in b5906b2

types/Luau.luau Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved
src/lune/builtins/luau.rs Outdated Show resolved Hide resolved

assert(type(luau.compile) == "function", "expected `luau.compile` to be a function")

assert(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Additionally, if a compilation fails - instead of producing an error, the luau.compile function will return a message where the first byte is 0, and the error is attached afterwards

I think it would probably be a good idea to check for this on the rust end of things for the compile function and throw a lua error then, too. We don't really have any other APIs that behave like this in Lune where they return an error message, if the user wants to handle errors/fallible functions pcall should be the go-to for consistency

@filiptibell filiptibell merged commit 0a8773d into lune-org:main Aug 10, 2023
1 check passed
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.

2 participants