HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.19k stars 656 forks source link

Allow implementing/overriding Dynamic parameters with any type. #8884

Closed haath closed 5 years ago

haath commented 5 years ago

Problem

So let's say I have the following interface:

interface Point
{
    public function onOverlap(other: Dynamic): Void;
}

Trying to implement it the following way:

class MyPoint implements Point
{
    public function onOverlap(other: MyPoint): Void { }
}

Gives me the following error: Field onOverlap has different type than in Point

The same issue also exists when extending a class, where overriding a method while changing the type of originally Dynamic parameters gives the error: Field onOverlap overrides parent class with different or incomplete type

Suggestion

The above behavior should be allowed with an implicit cast at the time the method is invoked.

Rationale

The behavior on this matter should be such that it is consistent with local functions and function-typed parameters, where this exact behavior is actually allowed. For example the following works without any errors:

function foo(callback: Dynamic->Void) { }
var localFoo = function(point: Point) { };

foo(localFoo);
RealyUniqueName commented 5 years ago

What you describe in "Problem" section is actually an intended behavior because function arguments are contravariant. What you describe in "Rationale" section should be fixed to also emit an error. Here is why:

function foo(callback: Dynamic->Void) {
    callback(123); // this is allowed because `Int` can be passed to `Dynamic`
}
var localFoo = function(point:Point) {
    point.onOverlap(null); // this is allowed, because `Point` has `onOverlap` method
};
foo(localFoo); // Exception: 123 does not have `onOverlap` method
back2dos commented 5 years ago

I don't think this is a bug. Dynamic works as a wildcard:

var a:Array<Int> = [];
var b:Array<Dynamic> = a;
b.push('foo');

The basic problem is that Dynamic conflates a whole range of concepts, but every time Nicolas is pressed on the matter, his stance is this:

image


Not sure what that means for the original feature request. Allowing it would certainly be more consistent, although in this case I'm not sure that's a good thing.

haath commented 5 years ago

@RealyUniqueName I see your point, but what you are describing is a runtime error, and frankly the possibility of these will always be there by default when using the Dynamic or Any types. At that point it is anyway up to the developer regardless of what the compiler allows or not.


To maybe try to expand a bit further on why I think this should be allowed:

interface I
    function foo(x: Dynamic): Void;
class A implements I
    public function foo(x: Int): Void;

Interface I: I require a function named foo which accepts a single argument of any type.

Now, does A not fulfill this requirement? I think it does...

kLabz commented 5 years ago

Now, does A not fulfill this requirement? I think it does...

It doesn't. It cannot handle an argument of a type different than Int, so it doesn't accept a single argument of any type.

haath commented 5 years ago

It doesn't. It cannot handle an argument of a type different than Int, so it doesn't accept a single argument of any type.

Ok I do see your point. I was looking at it more in terms of being compliant with the interface definition, rather than being compliant with the users of the interface.


So I would only like to bring to your attention how HaxeFlixel uses this feature on callback functions. Its collide() method accepts a callback for detected collisions which has Dynamic arguments, this way, when you know which objects collide with which, you can save yourself a cast and do the following:

FlxG.collide( listOfCars, listOfObstacles, onCarCollidedWithObstacle );
function onCarCollidedWithObstacle( car: Car, obstacle: Obstacle )
{
    // ...
}

So if to remove this feature for consistency, the example above would just need two extra casts and two extra local variables to hold the objects after the cast. And the runtime type-safety would be exactly where it was before.

RealyUniqueName commented 5 years ago

That could be done with type parameters without any casts and without Dynamic:

function collide<T1, T2>(list1:Array<T1>, list2:Array<T2>, callback:(T1, T2)->Void) : {...}
Simn commented 5 years ago

This isn't sound because it violates variance. You can implement with a more general argument type, but not with a more specific one.