fslang-suggestions icon indicating copy to clipboard operation
fslang-suggestions copied to clipboard

Allow returning `int` from active patterns that only classing the input

Open Tangent-90 opened this issue 6 months ago • 3 comments

I propose we allow returning int from active patterns that only classing the input, instead of Choice<unit, unit, ..., unit>, for better performance

Sample:

// Marking the return type as `int`
// only allow returning the cases defined by the name of the active pattern, no explicit numbers like 1, 2, 3, ...
let (|XmlDoc|SingleLine|Else|) (line: string): int =
    if line.StartsWith("///") then XmlDoc
    elif line.StartsWith("//") then SingleLine
    else Else

// Marking the return type of (|A|B|C|) as `int`
let f ((|A|B|C|) : _ -> int) = 
    match "some thing" with
    | A -> "A"
    | B -> "B"
    | C -> "C"

For a n-cases active pattern, its int representation will be:

  • case 1: 1
  • case 2: 2 ...
  • case (n - 1): (n - 1)
  • case n: exclude above

The existing way of approaching this problem in F# is returning Choice<unit, unit, ..., unit> from those active patterns.

Pros and Cons

The advantages of making this adjustment to F# are

  • Fewer memory allocations and better performance

The disadvantages of making this adjustment to F# are ...

  • Adding one more way to do the same thing.

Extra information

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

Related suggestions: (put links to related suggestions here) #1041 #612

Affidavit (please submit!)

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

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

For Readers

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

Tangent-90 avatar Dec 29 '23 11:12 Tangent-90

The original suggestion of https://github.com/fsharp/fslang-suggestions/issues/612 included struct total active patterns which was not implemented.

// Marking the return type as `int`
// only allow returning the cases defined by the name of the active pattern, no explicit numbers like 1, 2, 3, ...
let [<return: Struct>] (|XmlDoc|SingleLine|Else|) (line: string) =
    if line.StartsWith("///") then XmlDoc
    elif line.StartsWith("//") then SingleLine
    else Else
error FS3385: The use of '[<Struct>]' on values, functions and methods is only allowed on partial active pattern definitions

In the implementation PR (https://github.com/dotnet/fsharp/pull/10338), the original message had this "Not sure what's the value counterpart for Choice?" which was ignored in the whole thread later. ValueChoices might be usable but I am not sure if the .NET Runtime can merge generic fields of the same type into the same field for the same memory efficiency as int.

Happypig375 avatar Dec 29 '23 12:12 Happypig375

I think a disadvantage to call out is that to make this non-breaking, it would require the type annotation, which would make it much less likely to be used. That said, I would prefer a more efficient representation by default, and I think the impact of breaking changes is likely fairly low if we were to change it from a FSharpChoice<Unit, Unit, ..., Unit> to an int under the covers.

cartermp avatar Dec 29 '23 17:12 cartermp

Maybe it is not suitable to pass these kind of active pattern by parameters like let f ((|A|B|C|) : _ -> int) = ..., because the active pattern function may return any integers that not match to the labels...

Or maybe we can add a wildcard case to it, making it to be a multiple cases partial active pattern (|A|B|C|_|): _ -> int?

Tangent-90 avatar Dec 30 '23 10:12 Tangent-90