dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.92k stars 786 forks source link

SRTP bug in type inference #10519

Open RealmPlume opened 3 years ago

RealmPlume commented 3 years ago

I think it's serious:

    type Ah =
        static member plus (a: Ah, b: Ah) = "Ah"

    type TestPlus =

        static member plus(a: int, b: int) = a + b 
        static member plus(a: string, b: string) = a + b
        static member plus (a: 'T, b: 'T) = "TestPlus"

    let inline plus'< ^t,^x, ^r when (^t or ^x or ^r):(static member plus: ^x * ^x -> ^r)> a b =
            ((^t or ^x or ^r):(static member plus: ^x * ^x -> ^r) (a, b))        

    let inline plus a b = plus'<TestPlus, _, _> a b

    let foo2 (a: int) (b: int) = plus a b

the code compile to :

public static int foo2(int a) (int b)
{
    //IL_0006: Expected I4, but got O
    return (int)"TestPlus";
}

foo2 1 2 result is -464137512

the type inference should do something here.

charlesroddie commented 3 years ago

What is expected behavior, a compile error or a result?

RealmPlume commented 3 years ago

What is expected behavior, a compile error or a result?

return a string and don't cast it to int with a magic value, and let it be a compile error .


I have seen the funtion plus compile to:

public static b plus<a, b>(a a, a b)
{
    return (b)TestPlus.plus(a, b);
}

why cast to b ?

cartermp commented 3 years ago

I think this is expected. In your code, you're telling the compiler to infer to the types for plus' and there are two candidates that could satisfy the constraint. It happens to pick the last one because that's generally how things in F# work - the F# compiler will infer the last item it sees that will satisfy the constraints. In this case int is also a 'T, hence it picks that particular implementation.

RealmPlume commented 3 years ago

I think this is expected. In your code, you're telling the compiler to infer to the types for plus' and there are two candidates that could satisfy the constraint. It happens to pick the last one because that's generally how things in F# work - the F# compiler will infer the last item it sees that will satisfy the constraints. In this case int is also a 'T, hence it picks that particular implementation.

I am clear now.

    .maxstack 8

    // return (int)"TestPlus";
    IL_0000: ldstr "TestPlus"
    IL_0005: ret

It is honest in the procedur,but the result beyond my prediction of .Net.

The chosen is expected, but -464137512 seems to a pointer of string "TestPlus"? This is not expected.

It looks hack,,, hacker,,, hackest ,,,,,,

En3Tho commented 3 years ago

@cartermp any change of redesining type inference to pick up best match (like int in this case) or is to too breaking?

charlesroddie commented 3 years ago

@cartermp I think this is expected. In your code, you're telling the compiler to infer to the types for plus' and there are two candidates that could satisfy the constraint. It happens to pick the last one because that's generally how things in F# work - the F# compiler will infer the last item it sees that will satisfy the constraints. In this case int is also a 'T, hence it picks that particular implementation.

The severity is very minor, but there is a bug here surely. The code has managed to confuse the compiler. If the compiler just picked the last implementation, then it would return "TestPlus", not a nondeterministic integer.

Also let x = plus'<TestPlus, _, _> 1 2 returns 3, which is neither the "TestPlus" not a nondeterministic integer. So assuming you are right that the last one should be picked, then this is another bug since it should return "TestPlus".

cartermp commented 3 years ago

Sure, there might be a bug in here. But the mixing of overloads, SRTP, and inference is so complex that it's just unsurprising that a seemingly unintuitive result would get picked.

@En3Tho highly unlikely to happen. There are just too many subtle interactions that could break.