[std] Restore carray_get/length for 1.13 compat

[tobil4sk]

Hashlink 1.14 had this change:

reimplement CArray using native opcodes (requires Haxe 4.4+)

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:

/usr/bin/ld: Main_Fields_.c:(.text+0x533): undefined reference to `hl_carray_get'
/usr/bin/ld: Main_Fields_.c:(.text+0x5fe): undefined reference to `hl_carray_length'

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_ptr check so we don't try to read the header if it hasn't been allocated with hl_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 HSTRUCT carrays, this version allocates a wrapper in every hl_carray_get call. 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 old hl_carray_get, but this is ok because hl_carray_get is only for compatibility.

I have wrapped it in HL_DISABLE_COMPAT for 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:

class Object {
	public var field:Int;
}

@:struct class Struct {
	public var field:Int;
}

var success = true;
function assert(expected:Int, actual:Int, ?pos:haxe.PosInfos) {
	if (expected != actual) {
		trace('Expected $expected, got $actual', pos);
		success = false;
	}
}

function checkSuccess() {
	if (success) {
		Sys.println("Tests passed");
	} else {
		Sys.println("Tests failed");
		Sys.exit(1);
	}
}

function main() {
	final array:hl.CArray<Object> = hl.CArray.alloc(Object, 42);
	assert(42, array.length);
	assert(0, array[0].field);
	array[0].field = 42;
	assert(42, array[0].field);

	final array:hl.CArray<Struct> = hl.CArray.alloc(Struct, 42);
	assert(42, array.length);
	assert(0, array[0].field);
	array[0].field = 42;
	assert(42, array[0].field);

	checkSuccess();
}

haxe --main Main -D hl-ver=1.13.0 --hl main.hl

[yuxiaomao]

@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

[ncannasse]

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.

[tobil4sk]

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 -DHL_DISABLE_COMPAT.

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.

[Simn]

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.

[ncannasse]

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)

[tobil4sk]

Thank you for reconsidering!

I have moved the metadata to the end of the block instead of the start. This way the payload at the start of the allocation which solves the interior pointer/memory offset issue.

Even just for the length primitive metadata is necessary, because we need block_size / element_size and for struct arrays the struct size is not stored in the carray. However, I also think there is not much point in supporting length without get.

CArray is not really a core feature, it was experimental at the time.

Maybe Haxe can have an @:experimental meta for future cases like this that warns similarly to @:deprecate. If a user is shown warnings and still uses it then it is easier to justify breaking something.

Since it is publicly available in std from haxe 4.3.0 with no indication that it is experimental or not "core", then I think it is better to restore the primitives so that the api used by 4.3 is still fully supported.