[cs] Wrong generic type definition

Neverbirth commented 8 years ago

Having for example the following piece of code:

interface ITest<K, V>
{
    function keys():Array<K>;
    function values():Array<V>;
}

interface ISubTest extends ITest<String, String>
{
}

Generates the following C# code for ISubTest:

public interface ISubTest : global::haxe.lang.IHxObject, global::ITest<object, object> {

}

As you can see it completely loses the types we want to use. I tried different types, and I was always getting object for the generic type. Furthermore, this one:

interface ITest<K, V>
{
    function keys():Array<K>;
    function values():Array<V>;
}

interface ISubTest extends ITest<String, Dynamic>
{
}

class TestClass implements ISubTest
{
    public function keys():Array<String>
    {
        return null;
    }

    public function values():Array<Dynamic>
    {
        return null;
    }
}

Doesn't compile at all, because it generates some invalid implementation of the interface in TestClass:

    global::Array global::ITest<object, object>.keys() {
        return ((global::Array) (this.keys()) );
    }

    public virtual global::Array<object> keys() {
        return null;
    }

    public virtual global::Array values() {
        return null;
    }

I would say this is a rather important issue.

nadako commented 8 years ago

The second one is a real issue.

As for the object - that's how the c# target currently works. As @waneck explained before, since the memory layout is the same, there's no real benefit of working around the haxe/c# type system differences to make the code look more "native", and by some tests it even worked faster. I'd like to review/check that again though, because this topic is brought quite often.

Neverbirth commented 8 years ago

? Would love to see those tests. While what you say is true for some cases, it's not for all of them. Think for example of a generic that in the end is creating an array for ints, with the code used for c# you'll end creating an array of objects, in that case the memory layout is not the same and haxe code will be much slower for some uses. Also, what if you use constraints for the generic type? I didn't check this use case, but do you rely on reflection to access known methods and properties? in that case it's clear it will also be slower.

There are other cases too, but I'd say that in a lot of cases the produced code will be slower (admittedly in a lot of them the difference will be minimal), maybe those tests were not taking into account GC garbage, starting up overload and other things that can take an effect into the final results?

EDIT: Tested it, I see that constraints are not converted over to C# code, and that in fact reflection is used unless we define the exact type to be used in the interface or class declaration.

nadako commented 8 years ago

Well, for basic types (int,double,bool) it generates proper type, it's just all others that are pointers anyway, but yeah, we should look into it at some point in future.

Neverbirth commented 8 years ago

I see, I thought the object replacement was used everywhere, my bad. There may be some cases where it can still be a problem, like arrays of primitive types, but I think this one would be very uncommon.

At any rate, that bug when using Dynamic is way more important, it's for example making hexMachina not usable when targetting C#, a real shame as it's already working for a few targets.

vroad commented 8 years ago

Is there any way to avoid this issue? Haxe falls back to reflection when iterating through Map values like this.

https://github.com/openfl/starling/blob/8d3f19b9594870db8f406442ba5cff476395393a/starling/rendering/BatchProcessor.hx#L211

vroad commented 7 years ago

I'm not sure why losing generic parameter makes program faster either. That will cause lots of casts, and makes code size bigger. Compilers cannot generate optimized code from such big code.

vroad commented 7 years ago

Even when reflection is not used it could be bad for performance because of lots of casts.

Also, losing type information is bad if you want to make a library for existing .NET application, or if you want to use that library from Haxe with -net-lib. You need to do cast everywhere to get an instance of correct type from arrays/maps.

Neverbirth commented 7 years ago

Also, losing type information is bad if you want to make a library for existing .NET application, or if you want to use that library from Haxe with -net-lib. You need to do cast everywhere to get an instance of correct type from arrays/maps.

That's a good point... I didn't try it, but what if you have a library that requires to get for example some GenericClass<CustomClass> instance? AFAIK GenericClass<Object> won't work in the cases where variance is not defined, will it?

vroad commented 7 years ago

You mean we should support casting to GenericClass<object> from GenericClass<CustomClass> in some way? Then, should all generic classes have wrapper object like arrays/maps to support variance? That may work, but makes generated code complex, and creates many objects for casting.

Could that be the other way around? Just create class instances with type parameter(SomeList<CustomClass>), and automatically upcast to CustomClass if you want to push some element, and downcast to `object' when you get some element from object. This way we don't need wrappers, but list cannot contain any kind of object anymore.

Should a List be able to contain any kind of object? Then it's not possible to support variance in this way. but you can't insert objects to Array<Int> already, can you?

Neverbirth commented 7 years ago

You mean we should support casting to GenericClass

HaxeFoundation / haxe

[cs] Wrong generic type definition #5434