Skip to content

[cpp] Un-dynamic zip implementation#12453

Merged
Simn merged 9 commits intoHaxeFoundation:developmentfrom
Aidan63:typed-zip
Dec 15, 2025
Merged

[cpp] Un-dynamic zip implementation#12453
Simn merged 9 commits intoHaxeFoundation:developmentfrom
Aidan63:typed-zip

Conversation

@Aidan63
Copy link
Copy Markdown
Contributor

@Aidan63 Aidan63 commented Dec 14, 2025

I mentioned in the marshalling type PR that cpp.ManagedType externs could be used to un-dynamic much of the cpp std implementation, so here's an example of this using the zip api since it's relatively small.

The changes here are realtively small, mostly forwarding calls into a new typed extern which contains the hxcpp implementation. I'm using the new View type in the cpp signature to allow passing any data, not just haxe arrays, to the underlying implementation.

More info on the hxcpp side can be found in it's PR: HaxeFoundation/hxcpp#1277

Again, I've changed the ci to use my fork just to show the tests, since this is a bit of a chicken and the egg problem.

Assuming this goes well slowly un-dynamicing the entire cpp std is something which can now be done.

@Aidan63
Copy link
Copy Markdown
Contributor Author

Aidan63 commented Dec 14, 2025

Those cpp tests should pass if restarted, just needed a change I forgot about on the hxcpp side.

@tobil4sk
Copy link
Copy Markdown
Member

Is it the handle being dynamic that this is aiming to solve? it seems that this replaces the indirection of dynamic with indirection of a whole other class, at least for haxe.zip.Compress. Couldn't the handle be a @:cpp.PointerType now?

@Aidan63
Copy link
Copy Markdown
Contributor Author

Aidan63 commented Dec 14, 2025

It's the removal of the dynamic handle and the removal of untyped / __global__ (which admittedly isn't used in zip, but is in other parts of the std).
That whole other class has always been there, it was just hidden behind dynamic, this just actually exposes it so we get all the usual type checking, not being able to accidentally pass in an invalid handle.
Previously when calling one of the _hx_inflate / _hx_deflate functions it would dynamic_cast the handle to the internal hxcpp zip class.

It couldn't be cpp.PointerType since the handle is a ::hx::Object subclass so write barriers wouldn't be inserted if compiling for the generational gc (which I'm assuming is why the handle was passed around as dynamic). Extern types with the cpp.ManagedType meta are assumed to be ::hx::Object subclasses and will have write barriers generated.

@tobil4sk
Copy link
Copy Markdown
Member

Thanks for explaining a bit more! That makes sense, given that the class was already there (already extending hx::Object) this does seem to improve things without too much added complexity.

@Aidan63
Copy link
Copy Markdown
Contributor Author

Aidan63 commented Dec 14, 2025

I think third time might be the charm for those cpp tests. Think I messed up the library flag in the build xml which is why it's giving weird hxcpp cache errors.

@Aidan63
Copy link
Copy Markdown
Contributor Author

Aidan63 commented Dec 14, 2025

Something very odd is going on with the hxcpp cache. I've got a cache on my machine and it was working fine, seems like the problem(?) is that the ci was setting the cache to a relative path. This meant instead of a single cache folder there was a hxcache folder next to every Build.xml file.
I have no idea if hxcpps cache is suppose to support relative paths but I can't imagine it's supposed to work in that way, but I also find it hard to believe that something in these changes in now causing it to actually break... either way I changed it to a absolute path.

I have not yet figured out why the linux builds can't find the zlib functions, "works fine on my machine".

@tobil4sk
Copy link
Copy Markdown
Member

Something very odd is going on with the hxcpp cache. I've got a cache on my machine and it was working fine, seems like the problem(?) is that the ci was setting the cache to a relative path.

We were setting it to ~/hxcache, and ~ is usually resolved by the shell which probably doesn't happen if it's set in the env section in yaml. Using a github variable seems like the correct thing to do anyway.

I have not yet figured out why the linux builds can't find the zlib functions, "works fine on my machine".

I've think I've had similar issues like this before due to linking order. Might need to add the glue files before adding the zlib sources, to stop the linker assuming that the zlib symbols are unused. hxcpp cache is a factor here, since it causes it to be linked as a static library.

@Aidan63
Copy link
Copy Markdown
Contributor Author

Aidan63 commented Dec 14, 2025

does indeed seem to be an order issue, switched the order around an the two linux tasks now pass.

@Simn Simn merged commit 447d578 into HaxeFoundation:development Dec 15, 2025
47 checks passed
PLATFORM: linux64
TEST: ${{matrix.target}}
HXCPP_COMPILE_CACHE: ~/hxcache
HXCPP_COMPILE_CACHE: ${{ github.workspace }}/hxcache
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, it slipped my mind but this change should have gone in extra/github-actions/, this file gets generated from there

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah damn, that keeps happening...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Aidan63 Aidan63 deleted the typed-zip branch December 15, 2025 16:00
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.

3 participants