fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Oddities in statically resolved method constraints and method overloading

Open GratianPlume opened this issue 7 years ago • 13 comments

Case 1

type Test() =
     member __.Equals (_: Test) = true

let inline Equals(a: obj) (b: ^t) =
    match a with
    | :? ^t as x -> (^t: (member Equals: ^t -> bool) (b, x))
    | _-> false

let a: Test = Test()
let b:Test = Test()
//b <- null

printfn "%A" (Equals a b)

I expected to call Test.Equals: Test->unit, (Equals a b) return true but actually it call Object.Equals: obj->unit, return false

Case 2:

type X =
    static member Method (a: obj) = 1
    static member Method (a: int) = 2
    static member Method (a: int64) = 3


let inline Test< ^t, ^a when ^t: (static member Method: ^a -> int)> (value: ^a) =
    ( ^t: (static member Method: ^a -> int)(value))

let inline Test2< ^t> a = Test<X, ^t> a

printfn "%d" (Test2<int> 0)

I expected return 2, but actually it return 1

GratianPlume avatar Oct 25 '17 03:10 GratianPlume

@greatim Thanks for the clean report, I will be looking into this closely

dsyme avatar Oct 29 '17 12:10 dsyme

@greatim Please see #3967 for proposed resolution

dsyme avatar Nov 17 '17 16:11 dsyme

In case 2 there's certainly the bug that our type constrains either get lost or are ignored.

realvictorprm avatar Jun 08 '18 08:06 realvictorprm

@dsyme @jindraivanek and me noticed that if you change Case 2 to this:

type X =
    static member Method (a: int) = 2
    static member Method (a: int64) = 3


let inline Test< ^t, ^a when ^t: (static member Method: ^a -> int)> (value: ^a) =
    ( ^t: (static member Method: ^a -> int)(value))

let inline Test2< ^t> a = Test<X, ^t> a

printfn "%d" (Test2<int> 0)

Now we get super fancy weird compiler errors:

  let inline Test2< ^t> a = Test<X, ^t> a
  --------------------------^^^^

stdin(9,27): error FS0043: No overloads match for method 'Method'. The available overloads are shown below.
Possible overload: 'static member X.Method : a:int64 -> int'. Type constraint mismatch. The type 
    ''t'    
is not compatible with type
    'int64'    
.
Possible overload: 'static member X.Method : a:int -> int'. Type constraint mismatch. The type 
    ''t'    
is not compatible with type
    'int'    
.

This isn't an oddity @dsyme this is a clear bug.

realvictorprm avatar Jun 08 '18 08:06 realvictorprm

I do have a fix for this consisting mainly of your code with removing 4 lines. It also would fix Case 1. Do we want to have a fix for this?

To my understanding the problem lies in that the compiler either

  1. stops searching for a correct overload too early
  2. applies constrains on the type parameters too late so that object is chosen as solution.

Please note that this behaviour is reproducable with my branch of your fix https://github.com/Microsoft/visualfsharp/pull/5141/files

realvictorprm avatar Jun 08 '18 10:06 realvictorprm

This is unrelated to this bug.

I was investigating case 1 further and realized that this oddity is realted to overload resolution within inheritance. This example should print the same as case one but uses different code for reproduction:

type FooBase() =
     member __.Foo(_: obj) = false

type Test() =
     inherit FooBase()
     member __.Foo(_: Test) = true

let inline Foo(a: FooBase) (b: ^t) =
    match a with
    | :? ^t as x -> 
       (^t: (member Foo: ^t -> bool) (b, x))
    | x -> 
       printfn "Not what we want"
       false

let a: Test = Test()
let b:Test = Test()
//b <- null

printfn "%A" (Foo a b)

Expecting that this code prints "true" because Foo with Test as type argument would be expected here.

@dsyme Is this information helpful for finding the root cause?

realvictorprm avatar Sep 13 '18 10:09 realvictorprm

@realvictorprm Yes, helpful, thanks. The question isn't the root cause but whether we can change existing behaviour without busting existing code, and if not then what to do about it.

dsyme avatar Sep 13 '18 10:09 dsyme

From discussion with @realvictorprm

Both of these bugs are really hard to fix without breaking existing code.

  1. Fundamentally some SRTP constraints are being solved "too early" before the type variables in the signatures are fully solved. This means resolutions are reported involving the obj type (either as object/support type or argument type) and inference proceeds on that basis

  2. The first attempted fixes tried to delay resolution however that broke too much code. Attempts to moderate by delaying only "strong" resolution and proceeding on the old basis for "weak" resolution also didn't help, existing code still broke

One possible way to make progress is as follows:

  1. First, add lots of test cases. We have done some in #3967 but we really need to add all of FSharpPlus and the other test cases reported by @gusty.

  2. Second, we could first aim to report warning in the cases where the above behavior is happening. In particular we would store-aside any constraints where the arguments involved unresolved variable types, and resolution proceeded. We would then re-check the resolution of these constraints in PostInferenceChecks.fs. If the resolution of the constraints differed we would report a warning.

  3. With that in place we could consider a /langfix language update for this case. However that may still be too intrusive, we need to have enough test cases to determine this.

dsyme avatar Oct 27 '18 13:10 dsyme

Notes for @realvictorprm to add tests that would grab and build a known commit of FSharpPlus as part of the test process

In tests\fsharp\tet-frramework.fs

module Commands = 
    let dotnet workDir exec (dotNetExe: FilePath) flags srcFiles =
        let args = (sprintf "%s %s" flags (srcFiles |> Seq.ofList |> String.concat " "))

        ignore workDir 
        exec dotNetExe args
let dotnet cfg arg = Printf.ksprintf (Commands.dotnet cfg.Directory (exec cfg) cfg.DotNetExe) arg

And in tests\fsharp\tests.fs:

    [<Test>]
    let testFSharpPlusBuild () = 

        let cfg = testConfig "repos"

        git cfg "clone http://github.com/fsprojects/FSharpPlus f309032020892r30 repodir"
        
        let cfg2 = testConfig "repos/repodir"

        dotnet cfg2 "build" [ "FSharpPlus.sln" ]

dsyme avatar Oct 27 '18 13:10 dsyme

Hi @dsyme I'll work today and tomorrow many hours on this. From my recent work I need to report, that right now there isn't a correct dotnet command implemented neither it's easy to implement it such that the new compiler build will be picked up at least from a slack talk with @cartermp: Me:

Would it be best to open a PR with my changes and then discuss which steps are required to acquire a dotnet.exe which uses the freshly build fsharp compiler?

Phillip Carter:

It's very tricky, since it requires a build of the CLI itself to work. I recommend using something from FSharpPlus as source as the test case(s)

Relying on this comment I'll work on creating a build script picking the source files of FSharpPlus and invoke the compiler directly on it instead of using the sln file to build the project.

realvictorprm avatar Nov 08 '18 13:11 realvictorprm

OK another case of trying to delay overload resolution. The case 2 can be "workedaround" through using this code:

type X =
    static member Method (a: int) = 2
    static member Method (a: int64) = 3


let inline Test< ^t, ^a when (^t or ^a): (static member Method: ^a -> int)> (value: ^a) =
    ( (^t or ^a): (static member Method: ^a -> int)(value))
    
let inline Test2< ^t when (X or  ^t) : (static member Method :  ^t -> int)> a = Test<X, ^t> a

printfn "%d" (Test2<int> 0)

realvictorprm avatar Nov 22 '18 14:11 realvictorprm

Welcome to F# Interactive for .NET Core in Visual Studio. To execute code, either
  1. Use 'Send to Interactive' (Alt-Enter or right-click) from an F# script. The F# Interactive process will
     use any global.json settings associated with that script.
  2. Press 'Enter' to start. The F# Interactive process will use default settings.
> 
Microsoft (R) F# Interactive version 12.8.0.0 for F# 8.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> val it: unit = ()

> type X =
    static member Method (a: int) = 2
    static member Method (a: int64) = 3


let inline Test< ^t, ^a when ^t: (static member Method: ^a -> int)> (value: ^a) =
    ( ^t: (static member Method: ^a -> int)(value))

let inline Test2< ^t> a = Test<X, ^t> a

printfn "%d" (Test2<int> 0);;

  let inline Test2< ^t> a = Test<X, ^t> a
  --------------------------^^^^

stdin(10,27): error FS0043: No overloads match for method 'Method'.

Known return type: int

Known type parameter: < ^t >

Available overloads:
 - static member X.Method: a: int -> int // Argument 'a' doesn't match
 - static member X.Method: a: int64 -> int // Argument 'a' doesn't match

> 

Still not workable...

ingted avatar Jan 29 '24 23:01 ingted

Welcome to F# Interactive for .NET Core in Visual Studio. To execute code, either
  1. Use 'Send to Interactive' (Alt-Enter or right-click) from an F# script. The F# Interactive process will
     use any global.json settings associated with that script.
  2. Press 'Enter' to start. The F# Interactive process will use default settings.
> 
Microsoft (R) F# Interactive version 12.8.0.0 for F# 8.0
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> val it: unit = ()

> type X =
    static member Method (a: int) = 2
    static member Method (a: int64) = 3


let inline Test< ^t, ^a when ^t: (static member Method: ^a -> int)> (value: ^a) =
    ( ^t: (static member Method: ^a -> int)(value))

let inline Test2< ^t> a = Test<X, ^t> a

printfn "%d" (Test2<int> 0);;

  let inline Test2< ^t> a = Test<X, ^t> a
  --------------------------^^^^

stdin(10,27): error FS0043: No overloads match for method 'Method'.

Known return type: int

Known type parameter: < ^t >

Available overloads:
 - static member X.Method: a: int -> int // Argument 'a' doesn't match
 - static member X.Method: a: int64 -> int // Argument 'a' doesn't match

> 

Still not workable...

Nothing has been done in this area to my knowledge

vzarytovskii avatar Jan 30 '24 12:01 vzarytovskii