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

Upcast branches of `match` and `if` expressions when type is specified #792

Open
charlesroddie opened this issue Oct 2, 2019 · 10 comments

Comments

@charlesroddie
Copy link

commented Oct 2, 2019

F# should upcast return values of if and match expressions where the required type is known. For example the following expressions should compile:

type View() = class end
type Label() = inherit View()
let f b :View =
    if b
    then View()
    else Label() // FS0001 "All branches of an 'if' expression must return values of the same type as the first branch..."
let g b :View =
    match b with
    | true -> View()
    | false -> Label() // FS0001 "All branches of a pattern match expression must..."
let h b :View =
    match b with
    | true -> Label() // FS0001 "This expression was expected to have type View..."
    | false -> View()

Currently up-casting (where the intended type is specified or known) happens automatically in lists (let l:View list = [ Label() ]) and in function inputs. This doesn't happen in if and match constructs. There may be other constructs where this also doesn't happen, which should be added to this suggestion.

Pros and Cons

The advantages of making this adjustment to F# are: less manual upcasting

The disadvantages of making this adjustment to F# are: none

Extra information

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

Related suggestions: There are suggestions also to do with upcasting which are more complex: #3, #536 .

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • [ x ] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • [ x ] I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • [ x ] 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.

Please tick all that apply:

  • [ x ] This is not a breaking change to the F# language design
  • [ x ] I or my company would be willing to help implement and/or test this
@charlesroddie charlesroddie changed the title Upcast branches of match and if expressions when type is specified Upcast branches of `match` and `if` expressions when type is specified Oct 2, 2019
@alexandrehtrb

This comment has been minimized.

Copy link

commented Oct 5, 2019

Currently, in ASP.NET controllers using F#, manual upcasting is required:

[<HttpGet("{id}")>]
member this.Get(id: int): IActionResult =
    let x = this.Repository.Get(id)
    match x with 
    | null -> this.NotFound() :> IActionResult
    | _ -> this.Ok(x) :> IActionResult

It would be good to have automatic upcasting when type is specified.

@auduchinok

This comment has been minimized.

Copy link

commented Oct 5, 2019

Not an ideal suggestion but you can simplify it a bit using inferred types:

member this.Get(id: int): IActionResult =
    match this.Repository.Get(id) with 
    | null -> this.NotFound() :> _
    | _ -> this.Ok(x) :> _
@cartermp

This comment has been minimized.

Copy link
Member

commented Oct 5, 2019

I can definitely see this being controversial, since adding anything about making it easier to work with inheritance hierarchies always draws some allergic reactions. I'm generally in favor of something like this since it makes working with unavoidable and common APIs a bit easier.

@isaacabraham

This comment has been minimized.

Copy link

commented Oct 5, 2019

I would feel uncomfortable about this feature, or at least there would need to be some explicit guidance needed. Check out this old blog post from Lincoln Atkinson (ex F# compiler team) https://latkin.org/blog/2017/05/02/when-the-scala-compiler-doesnt-help/ looking at the very first sample. I definitely don't want that in F# with the compiler trying to be helpful but picking some arbitrary common type.

@cartermp

This comment has been minimized.

Copy link
Member

commented Oct 5, 2019

@heronbpv

This comment has been minimized.

Copy link

commented Oct 5, 2019

Wouldn't it be kind of a special case of a flexible type, but in the return definition instead of the parameter one? Maybe we could use that to properly signal that we want the return type to be exactly this one.

Wonder how much we could reuse of the current flexible type implementation for this, if any.

F# docs reference: https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/flexible-types

@isaacabraham

This comment has been minimized.

Copy link

commented Oct 5, 2019

@cartermp as long as the behaviour would be predictable, understandable and easy to reason about I'd probably be interested in this eg if there was a type hint to explicitly tell the compiler what the expected type was.

@isaacabraham

This comment has been minimized.

Copy link

commented Oct 6, 2019

In fact I remember when writing GPWF having to explain this exact scenario. So perhaps doing this in a specific scope has some value after all 😊

@charlesroddie

This comment has been minimized.

Copy link
Author

commented Oct 6, 2019

This suggestion would just extend current list behaviour to if and match:

let l1 = [View(); Label()] // Not OK
let l2:View list = [View(); Label()] // OK
let f(_:View list) = ()
f [View(); Label()] // OK

A more complete solution would be to upcast whenever the required type is known:

let l3:View list = [Label(); Label()] // OK
let v1:View = if true then Label() else Label() // This suggestion would make this OK
let v2:View = Label() // Not OK. Should it be OK?
@isaacabraham

This comment has been minimized.

Copy link

commented Oct 6, 2019

@charlesroddie mmm good question. The latter would I assume be logical to permit if the first two are legal, yet that's nothing that's been allowed this far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.