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

Auto implement partial active recognizers for DU cases #766

Open
4 tasks done
gusty opened this issue Jul 7, 2019 · 14 comments
Open
4 tasks done

Auto implement partial active recognizers for DU cases #766

gusty opened this issue Jul 7, 2019 · 14 comments

Comments

@gusty
Copy link

gusty commented Jul 7, 2019

I propose we add a way to auto-implement partial active recognizers for DU cases. This will provide a way to treat patterns as first class functions.

The existing way of approaching this problem in F# is to implement some functions by hand:

type Cases = CaseA of int | CaseB of float | CaseC of (int * string)
let fCaseA = function | CaseA x -> Some x | _ -> None
let fCaseB = function | CaseB x -> Some x | _ -> None
let fCaseC = function | CaseC x -> Some x | _ -> None

Pros and Cons

The advantages of making this adjustment to F# are: less typing, less code that adds nothing but noise that hides the intent in the end.

The disadvantages of making this adjustment to F# are that it's work (RFC, PR, testing).

Extra information

Estimated cost (S):

Related suggestions: #611 specifically this suggestion was mentioned by @mexx comment there.

Affidavit (please submit!)

Please tick this 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
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • 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:

  • [?] This is not a breaking change to the F# language design (it will depend on the implementation we choose)
  • I or my company would be willing to help implement and/or test this

Some context

The case exposed in #611 is a good example of how useful this is and how it relates to treat patterns as first class functions.

I also find this boiler-plate code while defining CODECs for discriminated unions to Json in Fleece by using alternatives:

type Shape =
    | Rectangle of width : float * length : float
    | Circle of radius : float
    | Prism of width : float * float * height : float
    with 
        static member JsonObjCodec =
            jchoice
                [
                    Rectangle <!> jreq "rectangle" (function Rectangle (x, y) -> Some (x, y) | _ -> None)
                    Circle    <!> jreq "radius"    (function Circle x -> Some x | _ -> None)
                    Prism     <!> jreq "prism"     (function Prism (x, y, z) -> Some (x, y, z) | _ -> None)
                ]

And I'm sure that there might be more cases around.

Proposed syntax

The most obvious syntax I can think of is just the case enclosed in banana parens, ie (|CaseA|_|) which matches the syntax for partial active patterns, that's why I choose the title of this suggestion as auto-generate partial active patterns.

So it will be as if these functions:

let (|Rectangle|_|) = function | Rectangle x -> Some x | _ -> None
let (|Circle|_|) = function | Circle x -> Some x | _ -> None
let (|Prism|_|) = function | Prism x -> Some x | _ -> None

were generated automatically after the type declaration, then the above code could be reduced to:

type Shape =
    | Rectangle of width : float * length : float
    | Circle of radius : float
    | Prism of width : float * float * height : float
    with 
        static member JsonObjCodec =
            jchoice
                [
                    Rectangle <!> jreq "rectangle" (|Rectangle|_|)
                    Circle    <!> jreq "radius"    (|Circle|_|)
                    Prism     <!> jreq "prism"     (|Prism|_|)
                ]

Alternative syntax

Another possibility is to use a different syntax, not related to partial active patterns, but the essence of this suggestion is not the syntax itself but the possibility to auto-generate those functions automatically.

@realvictorprm
Copy link
Member

I really like this! This is a good addition in my opinion and should be easy to do :)

@pblasucci
Copy link

@gusty just to clarify...

Did you mean to specify partial active patterns? Because that seems like a better fit here. Emitting a single-case total active pattern seems like it wouldn’t really serve the highlighted use case.

@gusty gusty changed the title Auto implement active recognizers for DU cases Auto implement partial active recognizers for DU cases Jul 7, 2019
@gusty
Copy link
Author

gusty commented Jul 7, 2019

Thanks @pblasucci I updated the suggestion title and also updated the content to reflect it.

@cartermp
Copy link
Member

cartermp commented Jul 8, 2019

This is a very interesting suggestion. We would have to consider compatibility here, likely only generating these functions if there isn't such an active recognizer name already in scope. But what if only a subset is in scope, do we still generate the remainders?

@gusty
Copy link
Author

gusty commented Jul 8, 2019

@cartermp good catch, I will update the breaking change box, as it will depend on the implementation.

I would say that either:

  1. We always generate, it would be a breaking change when the same active recognizer identifiers were already in scope, but we can use the language switch

  2. We do as you say, we detect if they are in scope and either don't generate any active recognizer, or we generate only those that are not in scope.

  3. We use a different syntax than the proposed here, namely a syntax which is currently invalid, this way it's not a breaking change at all.

@charlesroddie
Copy link

charlesroddie commented Jul 9, 2019

An alternative to creating many new methods and names:

trymatch du with CaseA x -> x
tryfunction CaseA x -> x

Equivalent to:

match du with CaseA x -> Some x | _ -> None
function CaseA x -> Some x | _ -> None

Pros: Doesn't create so many new methods and names. Deals with user-defined patterns, and more than one if needed. Fewer characters than current syntax.
Cons: more characters than proposed syntax (although the proposed syntax may need to be made slightly more verbose for clarity).

@thinkbeforecoding
Copy link

This is a very interesting suggestion. We would have to consider compatibility here, likely only generating these functions if there isn't such an active recognizer name already in scope. But what if only a subset is in scope, do we still generate the remainders?

There are two cases here, the Active pattern is declared before:

module Mod
let (|Case1|_|) x = if test x then Some value else None
type DU = Case1 of int | Case 2 of string

In this case there is no way that the first (|Case1|_|) takes DU as an input. So references to (|Case1|_|) will be shadowed by the DU version of (|Case1|_|) that take a DU and it will not compile anymore. Both can be distinguished by using Mod.Case1 / Mod.DU.Case1 in a match, or Mod.(|Case1|_|) / Mod.DU.(|Case1|_|) when used as a function.

The other case is when it's defined after:

module Mod
type DU = Case1 of int | Case 2 of string
let (|Case1|_|) x = match DU with Case1 x -> Some (x+1) | _ -> None

Here the new (|Case1|_|) will shadow DU.(|Case1|_|) which is expected. And disambiguation can still be done if necessary (Mod.(|Case1|_|) vs Mod.DU.(|Case1|_|) )

Anyway, having something like that would help a lot with the code I write!

@gusty
Copy link
Author

gusty commented Jul 16, 2021

@thinkbeforecoding maybe we need to change a bit the syntax in order to disambiguate.

Something like _Case1 instead of Case1 ( _ stands for the parameter).

But somehow, having not write these functions/active patterns by hand is something I find useful frequently both, in libraries and also in Business Logic code.

@Happypig375
Copy link
Contributor

Is this option only? What about voption and bool (#1041)?

@dsyme
Copy link
Collaborator

dsyme commented Jun 16, 2022

#222 and #506 are approved which I believes gives:

type Shape =
    | Rectangle of width : float * length : float
    | Circle of radius : float
    | Prism of width : float * float * height : float
    static member JsonObjCodec =
        jchoice
            [
                Rectangle <!> jreq "rectangle" _.IsRectangle
                Circle    <!> jreq "radius"    _.IsCircle
                Prism     <!> jreq "prism"     _.IsPrism
            ]

@dsyme dsyme closed this as completed Jun 16, 2022
@Happypig375
Copy link
Contributor

@dsyme Not equivalent because there is no deconstruction when using Is* properties.

@dsyme
Copy link
Collaborator

dsyme commented Jun 16, 2022

I see, yes, you're correct

@dsyme dsyme reopened this Jun 16, 2022
@abelbraaksma
Copy link
Member

abelbraaksma commented Feb 2, 2023

What would sensible naming here be? It seems counter intuitive to use the same name as the original cases, unless that doesn’t lead to conflicts. I mean, does the inference work correctly? What if you want to use a match statement with the partial active pattern? What if you want to use the partial active pattern and the DU case names in a single match?

I think fCase1 x as in the suggestion seems to non-idiomatic, plus that it violates the leading uppercase rule for (partial) patterns. Perhaps TryCase1 makes sense (prefix case-name with Try)? Or HasCase1, somewhat synonymous with IsCase1 from the referenced issue, but for deconstruction?

Other ideas? Maybe prefix with With? The latter seems to flow nicely with the example code above.

@gusty
Copy link
Author

gusty commented Feb 5, 2023

It seems counter intuitive to use the same name as the original cases, unless that doesn’t lead to conflicts. I mean, does the inference work correctly? What if you want to use a match statement with the partial active pattern? What if you want to use the partial active pattern and the DU case names in a single match?

Well, they will shadow the cases but what's wrong with it as they are kind of equivalent?

Here's an issue -> Exhaustive cases analysis. When doing:

    match x with
    | Rectangle _ -> ()
    | Circle _ -> ()
    | Prism _ -> ()

The compiler will use the generated partial active recognizers instead of the actual DU cases. So the exhaustive check warning will be triggered.
That's a drawback, but I think we can add some logic to save this case.

On a more general solution, but this probably deserves another suggestion: It would be cool if in general the F# compiler could perform exhaustive cases with partial recognizers like these ones.

And finally here's another way. Instead of generating the code, what if it simply works? I mean any DU case can be used as a partial active recognizer, without creating internally a separate binding and shadowing.

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

No branches or pull requests

9 participants