[std] Restore carray_get/length for 1.13 compat#907
[std] Restore carray_get/length for 1.13 compat#907tobil4sk wants to merge 2 commits intoHaxeFoundation:masterfrom
Conversation
|
@ncannasse please check this too, when you have time Hashlink's ci is failing but not related to the PR, seems that haxe ci enable the hl/c tests again, and we need haxelib dev hashlink command for hlc |
|
I'm not trying to maintain ABI compatibility between each Hashlink version, as it is quite complex to do so and does result in a lot of legacy code. HL is not only a library but a VM so dependencies correctness should be handled by the user when he deploys his application. |
|
This is not only a question of ABI compatbility, it's also about bytecode compatibility. As I mentioned, Haxe 4.3.7, which most users are using, still relies on these primitives: https://github.com/HaxeFoundation/haxe/blob/4.3.7/std/hl/CArray.hx#L20-L28. So bytecode built by 4.3.7 that uses CArray crashes with hl 1.14+. I don't see why this is complex, it only requires 2 primitives and no extra allocations, and it is easily removed using Breaking changes like this frequently affect people in the community and they are why hashlink is unreliable for usage outside of Shiro. It's disappointing that improved compatibility won't be considered, even if it comes from community contributions. |
|
I agree this should be reconsidered. This is something hxcpp has been doing right, where you can generally update to a newer hxcpp version without compatibility issues. Having to juggle dependencies is annoying, especially if it doesn't take a lot of effort to maintain compatibility. |
|
CArray is not really a core feature, it was experimental at the time. I agree we could keep the length primitive, but it will have to use the gc_block_size to calculate the length and not modify the data structure of the C Array without length prefix or memory offset. Your method will make the GC free the CArray as there is no pointer on the allocated block (if it's stored outside of the stack which allows for interior pointers) |
Hashlink 1.14 had this change:
This removed primitives which means an ABI breakage and it breaks hl/hlc programs compiled with haxe 4.3.7 that use hl.CArray, which still relies on these primitives. For hl bytecode, it gives the error:
Uncaught exception: Primitive or library is missing, and hlc gives linker errors like:This is not ideal because it was a breaking change in a minor release. It is possible to retain compatibility with the 1.13 primitives by keeping a header for hl allocated carray objects that stores the size and type. The cost of this is low because it doesn't require an additional allocation, it is just stores the metadata in front of the elements and carray is still passed as a pointer to the first element.
@yuxiaomao mentioned that sometimes carray may be used for externally managed arrays, so I added a
hl_is_gc_ptrcheck so we don't try to read the header if it hasn't been allocated withhl_alloc_carray. It gives an error instead, which should be fine because this usage wasn't supported in 1.13 anyway.Unlike the 1.13 version which stored pre-allocated dynamic wrappers for
HSTRUCTcarrays, this version allocates a wrapper in everyhl_carray_getcall. This avoids overhead from extra contents in the main carray, so normal usage with haxe 5 doesn't need to change. It is slower than the oldhl_carray_get, but this is ok becausehl_carray_getis only for compatibility.I have wrapped it in
HL_DISABLE_COMPATfor cases internal environments where haxe 4.3.7/hl 1.13 compatibility is not needed, and also to make it clear what is for compatibility only, so it is easier to remove it for when/if hl 2.0 comes out one day.I have some code I used for testing, which could be added if we add 4.3.7 to the test matrix: