Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impove type inference for yields in computation expression to let them be aware of overloads with function parameters #1388

Open
6 tasks done
Lanayx opened this issue Oct 8, 2024 · 3 comments

Comments

@Lanayx
Copy link

Lanayx commented Oct 8, 2024

This suggestion is based on issue dotnet/fsharp#17837

The following code is not currently allowed, it requires specifying int type for y explicitly.

open System

type Test() =
    member inline _.Yield(value: int -> unit) = ()
    member inline _.Yield(value: string) = () // without this overload issue is not observed
    member inline _.Zero() = ()

let x =
    Test() {
        yield (fun y -> Console.WriteLine(y)) // error, the type of y is unknown
    }

another case:

open System

type Person = {| Name: string |}

type Test() =
    member inline _.Yield(value: Person -> unit) = ()
    member inline _.Yield(value: string) = () // without this overload issue is not observed
    member inline _.Zero() = ()

let x =
    Test() {
        yield (fun y -> Console.WriteLine(y.Name)) // error, the type of y is unknown
    }

I suggest that this is undesired behavior and typechecker should be able to try function overload before giving up and saying that type is unknown. The suggested changes in typechecker by @T-Gro:

-The callsite is known to be passing in a function (not yet fully resolved, but known to be a function)
-There is exactly one overload with a function type argument (although I'm not sure why this is needed, I hope that typechecker can go through several overloads with function arguments and still resolve if there is only one match without error)

Pros and Cons

The advantage of making this adjustment to F# is less verbose code and consistent auto-inference behaviour.

The disadvantage of making this adjustment to F# is more work for typechecker

Extra information

Estimated cost (XS, S, M, L, XL, XXL): L

Affidavit (please submit!)

Please tick these items by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
  • This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@vzarytovskii
Copy link

vzarytovskii commented Oct 9, 2024

Estimated cost (XS, S, M, L, XL, XXL): S

It isn't, likely L

This is not only about yields in CE, this is more fundamental problem, we need to choose yield overload in this case, to do that we need to know the type of its argument, which we can't know because we can't typecheck it (we can't choose WriteLine overload, otherwise we would need to specialized it). In other words we can't infer type from specific Yield, because we don't know which one we want to use yet.

Let's say we make a change (special case the function somehow) to do type inference differently in certain cases, what happens when another overload is added?

open System

type Test() =
    member inline _.Yield(value: int -> unit) = ()
    member inline _.Yield(value: bool -> unit) = ()
    member inline _.Yield(value: string) = ()
    member inline _.Zero() = ()

let x =
    Test() {
        yield (fun y -> Console.WriteLine(y)) 
    }

Which overload should be chosen in this case, and which type should be inferred for y?

@T-Gro
Copy link

T-Gro commented Oct 9, 2024

As soon as the "exactly one function overload" stops being true, it would go back to being a method resolution error needing additional type annotations.

This is not unlike other method resolution situations where adding a new overload causes a compilation error downstream (looking at BCL adding overloads).

@Lanayx
Copy link
Author

Lanayx commented Oct 9, 2024

Let's say we make a change (special case the function somehow) to do type inference differently in certain cases, what happens when another overload is added?

My understanding (as soon as I don't know current implementation details) are the following:

1st case: it's even simpler to me, since we have a finite set of possible overloads in Console.WriteLine and finite number of possible overloads for Yield. What I expect compiler to do is to find a valid intersection of two sets. If we get 0 matches - show an error, if we get more than 1 match - show an error (your question), if we get exactly 1 match then apply that.

2nd case is more complex to me, since there is no set of possible overloads, since the type is totally uknown, we only know that y should be of some record with field Name. In that case rather than just matching types compiler should look if whether if any of those is a record with field Name and then (just like in the first case) fail if there are 0 matches or more than one

It isn't, likely L

Will change, no problem

As soon as the "exactly one function overload" stops being true, it would go back to being a method resolution error needing additional type annotations.

As I said, my thoughts above are very naive since I don't know details of how typechecker implementation works today, so they easily might be impossible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants