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

Parsing memory fixes #63

Merged
merged 9 commits into from
Mar 19, 2024
Merged

Conversation

VisenDev
Copy link
Contributor

@VisenDev VisenDev commented Feb 21, 2024

Closes #62

@VisenDev
Copy link
Contributor Author

@natecraddock After thinking about this some more, an alternative way of managing this memory could be to have a specific toAnyScalar that just returns basic types and thus has no need for memory allocation, and having a toAny that can allocate memory for slices that returns a ziglua.Parsed(T)

Alternative names could be toAny and toAnyAlloc. toAny will warn you if you give a type that needs memory allocation to parse at compile time, in which case you can use toAnyAlloc

@VisenDev
Copy link
Contributor Author

Actually thinking about it more I think I like using a toAny and a toAnyAlloc instead of my current approach. It seems to make a lot more sense and follows zig's library memory allocator passing convention better. I am going to rework some things

@VisenDev VisenDev marked this pull request as draft February 26, 2024 22:00
@VisenDev VisenDev marked this pull request as ready for review February 27, 2024 05:14
@VisenDev
Copy link
Contributor Author

I think the checks are failing because of some sort of network error, it doesn't seem to be related to the code

@natecraddock
Copy link
Owner

I'm going to go ahead and merge this as-is. There are some small things I might want to tweak later, but that polish can come before the 0.3.0 release of ziglua (that I want to make shortly after Zig 0.12.0)

@natecraddock natecraddock merged commit 6150f0f into natecraddock:main Mar 19, 2024
3 checks 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.

Proposed fixes to memory management issues with parsing
2 participants