HaxeFoundation / haxe

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

cache server + inline +multiType #8611

Closed nadako closed 5 years ago

nadako commented 5 years ago

This one is "interesting" (extracted from OpenFL):

Vector.hx

@:multiType(T)
abstract Vector<T>(Array<T>) {
    function new();

    public inline static function ofArray<T>(a:Array<T>):Vector<T> {
        return new Vector<T>();
    }

    @:to static function toIntVector(t:Array<Int>) return null;
    @:to static function toObjectVector<T>(t:Array<T>) return null;
}

Main.hx

class Main {
    static function main() {
        trace(Vector.ofArray([1]));
    }
}

First build will produce:

console.log("src/Main.hx:3:",_$Vector_Vector_$Impl_$.toIntVector(null));

(which is kind of what we'd expect, but is actually debatable, because ofArray.T is not generally known to be an Int, but due to inlining this just happens)

However if we touch Main.hx and build again we will get:

console.log("src/Main.hx:3:",_$Vector_Vector_$Impl_$.toObjectVector(null));
nadako commented 5 years ago

Good news is that adding @:generic to the ofArray function fixes the issue and gives the desired behaviour (toIntVector). \o/

nadako commented 5 years ago

@jgranick you might be interested in this ^

Simn commented 5 years ago

First compilation:

source/Main.hx: 47-66: find_multitype_specialization [TMono (Some (TMono (Some (TAbstract(Int, [])))))]
Raised by primitive operation at file "_build/src/context/abstractCast.ml", line 205, characters 205-231
Called from file "_build/src/optimization/dce.ml", line 237, characters 22-81
Called from file "_build/src/optimization/dce.ml", line 491, characters 1-26
Called from file "list.ml", line 100, characters 12-15
Called from file "list.ml", line 100, characters 12-15

Second:

source/Main.hx: 47-66: find_multitype_specialization [TAbstract(Int, [])]
Raised by primitive operation at file "_build/src/context/abstractCast.ml", line 205, characters 205-231
Called from file "_build/src/optimization/dce.ml", line 237, characters 22-81
Called from file "_build/src/optimization/dce.ml", line 491, characters 1-26
Called from file "list.ml", line 100, characters 12-15
Called from file "_build/src/optimization/dce.ml" (inlined), line 288, characters 50-53
Called from file "_build/src/optimization/dce.ml", line 742, characters 4-29

source/Main.hx: 47-66: find_multitype_specialization [TMono (Some (TMono (Some (TAbstract(Int, [])))))]
Raised by primitive operation at file "_build/src/context/abstractCast.ml", line 205, characters 205-231
Called from file "_build/src/optimization/dce.ml", line 237, characters 22-81
Called from file "_build/src/optimization/dce.ml", line 491, characters 1-26
Called from file "list.ml", line 100, characters 12-15
Called from file "list.ml", line 100, characters 12-15

So there's a second find_multitype_specialization here. Though that doesn't answer why we end up with a different result because both of these type parameters looks quite Intish.

Simn commented 5 years ago

I think there's a secondary problem here because your @:to functions don't have a return type. This probably doesn't matter in practice, but we should make sure that they unify with Vector<Something> at least. Otherwise we risk leaking their return type monomorphs and that could lead to very weird inference down the line.

That actually reminds me of something @RealyUniqueName has been working on, so it might be related to that, too.

Simn commented 5 years ago

Something else to note: If we change Vector.hx we go back to toIntVector being generated. The issue here is likely related to cf_expr_unoptimized being used for inlining, but it's not yet clear to me what exactly the problem is.

nadako commented 5 years ago

I'm also not sure if having toIntVector there is correct, because it depends on whether the ofArray method is inlined or not, which shouldn't be the case...

RealyUniqueName commented 5 years ago

I think there's a secondary problem here because your @:to functions don't have a return type. This probably doesn't matter in practice, but we should make sure that they unify with Vector<Something> at least. Otherwise we risk leaking their return type monomorphs and that could lead to very weird inference down the line.

That actually reminds me of something @RealyUniqueName has been working on, so it might be related to that, too.

Yeah, that's what happened to hl.ByteAccess in https://github.com/HaxeFoundation/haxe/issues/8348

nadako commented 5 years ago

FWIW, In OpenFL the return type for @:to is specified, though not as Vector<Something> but as the underlying type. Which is also what we do for haxe.ds.Map.

Simn commented 5 years ago

I'm also not sure if having toIntVector there is correct, because it depends on whether the ofArray method is inlined or not, which shouldn't be the case...

That's similar to some cases we have with native Vector/Array types which cannot be generalized unless a function is made inline or @:generic. I don't think this is a problem.

Simn commented 5 years ago

The issue was actually pretty wide: Any AST change that happened during post-processing before cf_expr_unoptimized was saved would influence the next compilation. I think this might have been a big source of compilation server instability. We now save the unoptimized expression very early, which means that any inlining operation should give the same result.

The test for this is really retarded but I didn't want to over-engineer this at the moment.