HaxeFoundation / haxe

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

Use structural equality in generic_substitute_type'. #11692

Open Apprentice-Alchemist opened 3 weeks ago

Apprentice-Alchemist commented 3 weeks ago

This fixes a bug where typed exprs coming from macros won't have their types properly substituted in generic functions. A simple example is

function main() {
    foo(0);
}

@:generic function foo<T>(val:T):T {
    return bar(val);
}

macro function bar(expr) {
    var typedExpr = haxe.macro.Context.typeExpr(expr);
    return haxe.macro.Context.storeTypedExpr(typedExpr);
}

AST dump for foo<Int> without this change:

    @:noCompletion @:noUsing @:genericInstance
    public static function foo_Int[Function:(val : Int) -> Int]
        [Arg val<5480>:Int]
        [Block:Dynamic] [Return:Dynamic] [Local val(5480):Int:foo.T]

Note there is still a foo.T left. AST dump for foo<Int> with this change:

    @:noCompletion @:noUsing @:genericInstance
    public static function foo_Int[Function:(val : Int) -> Int]
        [Arg val<5480>:Int]
        [Block:Dynamic] [Return:Dynamic] [Local val(5480):Int:Int]

Now all occurences of foo.T have been replaced by Int.

Simn commented 1 week ago

Using structural equality is almost certainly not correct here. We never want to do this for Type.t because that might end up comparing tclass and other terrible things, which can even lead to compiler hangs because of recursive data.

However, I acknowledge that the result is correct here, so we'll have to investigate why the original lookup fails. If the problem comes from a macro transformation then the solution is likely to avoid the loss of identity. We had similar issues before with tvar which has a special mechanism in place to retain its identity.