[cppia] Add tests for missing retyping in function calls - #12321
Conversation
|
Indeed, we have the same error with super constructor calls, which requires are not covered by the current patch: // template function converts to dynamic array
function wrap<T>(a:Array<T>):Array<T> {
return a;
}
class Parent {
public function new(a:Array<Int>) {
a.copy();
}
}
class Child extends Parent {
public function new(a:Array<Int>) {
super(wrap(a));
}
}
function main() {
new Child([1,2,3]);
}@Aidan63 I'm hoping you might have some insight into the cpp generator since you refactored it not long ago. Any chance you might know what the best solution here is? I'm confused with the difference between |
|
Some of the retypers logic can be a bit difficult to follow.
I interpreted those "todo proper retyping" comments to refer to the args being retyped to There could well be some missing conversions for cppia in here, I can take a closer look if need be. |
These were solved in: HaxeFoundation@4714cc3 The following cases from that patch do not affect cppia: - FuncFromStaticFunction - this is not supported no cppia - is_array_splice_call - this case has the explicit `not forCppia` condition - _hx_create_array_length - this is not used directly on cppia - cpp_is_templated_call - templated calls are not supported on cppia There are also some cases where I have added tests, but they were working correctly before the patch: - testMapSet - technically the patch makes a difference with regards to the values stored in the map, but using map.get always converts to the correct type anyway. - testDynamicFieldCall - cppia always converts dynamic method arguments to the correct type. See: https://github.com/HaxeFoundation/hxcpp/blob/8600e4d49852a3e915c32f97c249ff7111936103/src/hx/cppia/CppiaFunction.cpp#L821 - testObjectDeclaration - similar to map set, it affects the stored data but the conversion always occurs on dynamic field access.
351fdc2 to
4070954
Compare
|
@Aidan63 I went through each line in your patch and added a test for each case that affected cppia with this array conversion issue. This includes #9990, and some other similar cases. As I mentioned in the commit message, some lines don't affect cppia, and some don't have an observable difference because the |
This fixes #9990, which crashes due to a missing
TODATAARRAYinstruction, which is required to convert the resulting dynamic array back into an int array.Before:
vs after:
I imagine the same issue can occur with other types of calls, so I will look into that before marking this as ready.
Some of those
(* TODO - proper re-typing *)comments also look a bit worrying...