HaxeFoundation / haxe

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

[hl] Mess with type parameters and macro/inline functions. #11344

Open pecheny opened 1 year ago

pecheny commented 1 year ago

I discovered that simple signal implementation i've always used, passes garbage instead of numbers to callback on hl target. In the original implementation i have macro dispatch() function for listeners call. Trying to minimize reproduction of the bug i've found that inline function may work the same.

class ObsTest {
    public static var onChange(default, null):Signal<Int> = new Signal();
    public static function main() {
        onChange.listen(v -> trace("Received value: " + v));
        onChange.dispatch(30); //ObsTest.hx:4: Received value: 907739328, the actual number may vary
    }
}

class Signal<T> {
    var listeners:Array<T->Void> = [];
    public function new() {}

    public inline function listen(listener:T->Void) {
        listeners.push(listener);
    }

    public function dispatch(a) {
        for (l in listeners)
            l(a);
    }
}

if both listen() and dispatch() are inline, the code works fine. If dispatch() only is inline, the code fails in runtime with Uncaught exception: Access violation (Hl only again).

Minimal example with macro function works fine, bun on my codebase passes garbage as well.

class Signal<T> {
    var listeners:Array<T> = [];
    public function new() {
    }

    public inline function listen(listener:T) {
        listeners.push(listener);
    }

    public macro function dispatch(signal, args:Array<haxe.macro.Expr>) {
        return macro {
            for (listener in $signal.asArray()) listener($a{args});
        }
    }

    public inline function asArray() return listeners;
}

https://try.haxe.org/#6Bfb676e

One more curious effect, this sample fails to compile on last release with no dce enabled, but works on nightly.

kLabz commented 1 year ago

One more curious effect, this sample fails to compile on last release with no dce enabled, but works on nightly.

Yeah, that hl bug has been fix on development and the fix will be included in 4.3.3

pecheny commented 1 year ago

@kLabz, BTW would 4.3.3 include https://github.com/HaxeFoundation/haxe/issues/11293 fix ? I'm curious about testing Armory on a phone, and this one breaks compilation. And when the release planned?

kLabz commented 12 months ago

@kLabz, BTW would 4.3.3 include #11293 fix ? I'm curious about testing Armory on a phone, and this one breaks compilation. And when the release planned?

Yep, just released 4.3.3 and that fix is included https://github.com/HaxeFoundation/haxe/releases/tag/4.3.3

Regarding your issue, it might be related to hlopt. Can you try disabling it with -D hl-no-opt? (replacing the loop with a lambda call works, whichever function is inline; sounds like the look is getting a wrong optimization https://try.haxe.org/#d885c2df)