Move allocation functions into javy lib #461
Merged
Conversation
This file contains 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
jeffcharles
commented
Aug 18, 2023
| @@ -69,6 +71,7 @@ pub fn generate(js: &JS, exports: Vec<Export>) -> Result<Vec<u8>> { | |||
|
|
|||
| // We no longer need these exports so remove them. | |||
| module.exports.delete(realloc_export); | |||
| module.exports.delete(free_export); | |||
There was a problem hiding this comment.
I'm doing this for consistency with us hiding the realloc function
| @@ -40,6 +40,7 @@ pub use config::Config; | |||
| pub use quickjs_wasm_rs as quickjs; | |||
| pub use runtime::Runtime; | |||
|
|
|||
| pub mod alloc; | |||
There was a problem hiding this comment.
Why don't we instead add [cfg(feature = "export_alloc_fns")] here and remove the cfg directive from each of the function definitions?
There was a problem hiding this comment.
One small reason is if they don't enable the export_alloc_fns feature, they can still add exported functions in their own crate to call canonical_abi_realloc and canonical_abi_free but export them with a name of their own choosing instead of with the export name we chose.
Co-authored-by: Saúl Cabrera <saulecabrera@gmail.com>
saulecabrera
approved these changes
Sep 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This does two things:
javycrate under the publicly exportedallocmoduleexport_alloc_fnswhich, when enabled, will export the realloc and free functions from the produced Wasm moduleThis will allow users of the Javy crate to share memory allocation and free functions instead of potentially having to write their own. I opted to put the function exports behind a cargo feature because someone using the Javy crate may want to expose their own allocation and free functions instead with their allocation logic and wouldn't want someone calling our functions or if they want to export them under a different name.