extern + inline + @:overload: method not inlined

[clemos]

Consider the following code :

class Extern {
    @:overload( function (a:Int) : Void {})
    inline public static function test(a:String):Void {
        return untyped Extern['my-test'](a);
    }
}

class RegressionTest {
    static function main(){
        Extern.test("coucou");
        Extern.test(1);
    }
}

On latest haxe/development, it will generate this js code:

RegressionTest.main = function() {
    Extern["my-test"]("coucou");
    Extern.test(1);
};

As you can see the second call, which correspond to the overloaded signature, won't be inlined.

I use this trick a lot to workaround JS using invalid identifiers in Haxe.
It seems it used to work with previous versions (3.1.3), so it's likely a regression.

As a side note, it might be interresting to allow function bodies in @:overload in this situation:
It would allow to "translate" different signatures into different calls, including cases where the number of arguments matter.
Of course in JS we can always use apply/arguments to forward calls, but it's not ideal.

[clemos]

I just tested and can confirm it works properly on master

[Simn]

@waneck: Are the overloaded fields supposed to have a cf_expr or not? We create them before following the original cf_type which means the TLazy is not called in time and the created copy ends up with cf_expr = None, which then cancels inlining.

[Simn]

Actually we run init_meta_overloads before bind_type so there's not even a TLazy yet.

[waneck]

@Simn, overloads on all platforms but C# and Java cannot have any body, and their cf_expr is None

[Simn]

I know that's what comes out of the syntax, my question is if it should be like this after type loading. How is this supposed to work alongside inlining? Should we really inline the original expression using the overloaded arguments? I don't think that makes any sense in the general case...

[Simn]

After talking with Caue we agree that this behavior is silly, but I'll work on restoring it for 3.2 somehow to avoid "regressions".

In the long run we should disallow inline on fields that have @:overload(function ...) because it simply doesn't make any sense. Instead we should support @:overload inline function on all targets.

[Simn]

This is now "supported". Leaving the issue open because we should talk about this crime against type systems after 3.2.

[Simn]

I missed this one for the breaking-haxe4 run, but I still think we should break this. Having this kind of overload declaration on an inline function is simply nonsensical, so I consider this a bug.

[Simn]

This breaks HL because of this thing in Reflect.hx:

	@:overload(function(f:Array<Dynamic>->Void):Dynamic {})
	extern public inline static function makeVarArgs(f:Array<Dynamic>->Dynamic):Dynamic {
		return _makeVarArgs(f);
	}

@ncannasse Could this be implemented differently so we don't have to allow inline + @:overload?

[ncannasse]

Sadly I'm not sure how to avoid this : we need the overload to match Reflect definition and I think we need the inline so it does not cause issue with HL strictly typed system.

[Simn]

Surely this can just be rewritten to a different expression in the generator...

[Simn]

See linked commit. Note that I didn't change etype because real_type already investigates the exact nature of TField expressions anyway.

[nadako]

@ncannasse please review

[ncannasse]

If CI passes it's ok for me.