haxe icon indicating copy to clipboard operation
haxe copied to clipboard

[cpp] Use cpp::Int64 for haxe.Int64 instead of a class

Open Aidan63 opened this issue 3 years ago • 2 comments

Hello, I've made an attempt at improving the haxe.Int64 type of the cpp target by having it be treated as the existing cpp::Int64 define (__int64 on msvc, int64_t otherwise) and updating several of the hxcpp structures to play nice with this change.

https://github.com/HaxeFoundation/hxcpp/pull/932

  • haxe.Int64 is now an abstract around cpp.Int64, most of the maths helper functions have been removed but some are still used to avoid things like division operators in haxe returning a float.
  • clsIdArrayInt64 is a new object id and there is a new array store enum arrayInt64 as hxcpp arrays can now store cpp::Int64's without wrapping them in an object.
  • Int, String, and Object maps in hxcpp now have an Int64 store type and appropriate conversions as well as get and set functions for int64 values.

There are still some things I'm not sure about and could use improvements.

  • I haven't really used cppia before and while I tried to add a new array type for holding 64bit ints I couldn't get it to work and its a bit beyond me at the moment, so it uses an array of objects for 64bit ints.
  • cpp tests are failing on the ci as it needs a few functions I added in my hxcpp pr, cpp and cppia tests are passing locally for me. I also need to cleanup some of my gencpp.ml changes as I'm sure some of them aren't actually needed.

Cheers.

Aidan63 avatar Oct 28 '20 13:10 Aidan63

I've gone back and tried to update these two PRs to work with the latest dev branch. I've also added a new hash in hxcpp for int64 keys and added the cpp.Int64Map specialisation so maps with haxe.Int64 keys now work as expected. The Int64Helper extern is also used for all operations again. I think I missed that the implicit to int was why it was needed in the first place. Once thats fully gone at some point in the future operations can be definined on the cpp.Int64 type like the numeric extern abstracts for the C# target.

I'm having some trouble getting some things working as they were before. With the changes to cpp.Int64 where to and from haxe.Int64 conversions were moved into it along with the implicit to int now being a function, typing Std.isOfType(x, cpp.Int64) generates ::Std_obj::isOfType(x, ::hx::ClassOf< ::cpp::_Int64::Int64_Impl_ >()) instead of ::Std_obj::isOfType(x, ::hx::ClassOf< ::cpp::Int64 >()). I'm guessing the _impl class is only generated when an abstract has functions and since you can't usually type check with an abstract one of the metas attached to it to skip that check. Removing the functions from cpp.Int64 gets it working again but I'm not sure what the actual fix would be.

Cppia is also giving me a bit of a hard time with the new cpp.Int64Map. When creating a cppia script using that type I get the Int64Map.hx:65: characters 39-61 : Unsupported operation in cppia :CppCode error. I know you can't use untyped in cppia but I've followed the same format as the other cpp maps and added the extra Int64Map checks into gencpp but I still can't get it to work out the box. Interestingly adding --macro exclude("cpp.Int64Map") to the cppia hxml gets it compiling fine and using maps with Int64 keys also works fine in cppia.

Any advice would be greatly welcome.

Aidan63 avatar Sep 11 '21 17:09 Aidan63

Took another look at this and worked around the isTypeOf and abstract issue by intercepting it in gencpp.ml and replacing it with the correct path with CppClassOf.

Also found the HostClasses.hx file which has strings to auto exclude for cppia which fixes the Int64Map issue I was having.

I think this finishes everything feature wise, Int64Map works fine on cpp and cppia, all tests are passing as well when using hxcpp with the additions on that end.

Aidan63 avatar Oct 23 '21 16:10 Aidan63

This looks like a cool improvement. @hughsando, @Simn any thoughts or directions to get this merged?

dazKind avatar Oct 02 '22 18:10 dazKind

I think that this is a worthwhile change. It's been a little while since I've been over some of these finer points, but I'm tempted to think that if it passes the tests then it is probably better to get it in sooner rather than later.

As for making sure things always work when the versions haxe and hxcpp differ, you would ideally be able to make the hxcpp merge without the haxe merge and have things work as before. Then, you get the new native int64s when you update haxe.

You may need to update the HXCPP_API_LEVEL in the haxe generate project if you need to have hxcpp perform differently with new and old versions of haxe.

hughsando avatar Oct 10 '22 13:10 hughsando

I've just updated my PR on the hxcpp side to the latest hxcpp master and the mac tests are all passing (other platforms are failing with the same reason as the CI on master is failing).

The HxcppApiLevel define has already been bumped to 430 with the non blocking process exit code PR from a few months back so build.xml files are generated with 430 version which the new int64 stuff is guarded behind.

Aidan63 avatar Oct 15 '22 14:10 Aidan63

I added a new enum getter for int64's as they would otherwise have to go though the object getter (boxing) or the int getter (truncated).

Now that the haxe CI is working again the hxcpp CI is and my PR on the hxcpp side is all green so it seems like I've not caused any backwards compatability issues. So that should be good to merge, then we can see what the haxe tests think of all this.

Aidan63 avatar Nov 07 '22 13:11 Aidan63

nice, now that the hxcpp PR has been merged the CI is all green on the haxe side.

Aidan63 avatar Nov 08 '22 18:11 Aidan63

Thank you for your work, and sorry this took so long!

Simn avatar Nov 08 '22 19:11 Simn