[RFC FS-1039 Discussion] Struct representation for active patterns #230
Replies: 93 comments
-
@dsyme When you have free time could you please give me an advice? I'm going to start implementation of the feature, where should I start? Thanks! |
Beta Was this translation helpful? Give feedback.
-
@gsomix I did take a look at this once before. I tried something like this
|
Beta Was this translation helpful? Give feedback.
-
The .NET standard libraries already use the prefix Also, in my own code I'm using a |
Beta Was this translation helpful? Give feedback.
-
@mistiara @gsomix I'd like to discuss naming for StructOption. I understand the desire to use One thing that speaks against it is that the name In my own code I've been using this:
where I suppose
and so one. Also there is the more cumbersome alternative of using Of these I still tend to favour the first of There is a related question of how we might eventually add (or have the a separate library provide) versions of
Systematic options might be:
or
or
I'm not saying these should go into FSharp.Core straightaway, but we should consider them as part of naming. Please give feedback asap as I'd like to get this type into FSharp.Core so we can start to assume its existence for later language work. |
Beta Was this translation helpful? Give feedback.
-
Please thumbs-up this comment if you would be happy with |
Beta Was this translation helpful? Give feedback.
-
Please thumbs-up this comment if you would be happy with |
Beta Was this translation helpful? Give feedback.
-
I'm assuming that there won't be any support for type Foo =
{ x : struct (int * string) // struct tuple
y : string (struct option) // would this be valid? I guess not
z : string uoption } // this would be the way to do it I guess (works already) For consistency if nothing else, I wonder whether sticking with Struct prefixing might help rather than having Struct Tuples but Value Options. What naming do we already use for |
Beta Was this translation helpful? Give feedback.
-
@isaacabraham It's plausible we could do new syntax like that, but I think we wouldn't do it. For tuples, we kind of have to, since they are a form of structural type, or atl east we have the special syntax for them. For other nominal types like struct options and struct choices it is less compelling. |
Beta Was this translation helpful? Give feedback.
-
Please, guys, don't forget about new F# users. I think they could be confused by the U-naming: why is it |
Beta Was this translation helpful? Give feedback.
-
@dsyme I proposed I would personally make it:
but I'm more worried about the naming of struct-option versions of functions. Since we would already be using the But what would we do with struct-option specific functions? I do not want a |
Beta Was this translation helpful? Give feedback.
-
Can someone please clarify why you are all so familiar with Therefore I'd prefer: [<Struct>]
type StructOption<'T> =
| SNone
| SSome of 'T
and 'T soption = StructOption<'T> Which is consistent in using |
Beta Was this translation helpful? Give feedback.
-
@matthid |
Beta Was this translation helpful? Give feedback.
-
Yes, that's possible though it seems odd to use both
Yes, that is a possibility
No we don't. I think it's just from my personal code.
Good points
There's something about that I don't like. Is it that |
Beta Was this translation helpful? Give feedback.
-
Is that arguing in favour of |
Beta Was this translation helpful? Give feedback.
-
Is it possible to make it so that adding a single |
Beta Was this translation helpful? Give feedback.
-
@gsomix I agree we should do that optimization but it can be tricky to get right
|
Beta Was this translation helpful? Give feedback.
-
@gsomix I would love someone to attempt this optimization. |
Beta Was this translation helpful? Give feedback.
-
In case those messages were meant for me, I appreciate there is a few considerations, but the wins are big if we can do it, I am happy to start investigating and attempt this optimisation, if I run into any blocks can I query here? |
Beta Was this translation helpful? Give feedback.
-
@gerardtoconnor Yes I meant you :) I agree with you about the direction this should take, subject to my comment above |
Beta Was this translation helpful? Give feedback.
-
@dsyme thanks for confirming, all points above noted, will investigate this weekend 👍 |
Beta Was this translation helpful? Give feedback.
-
Right now active patterns can also be used as functions returning Option/Choice. Would this add a new kind of active pattern that took continuations as arguments? Or would this be just an optimization, not introducing any new kind of pattern? |
Beta Was this translation helpful? Give feedback.
-
@mistiara this is primarily an optimization so the usage would not change. Ideally this would remove the need to have any function that returns Option/Choice, making AP match the AP function but I need to investigate and test all the edge cases as there a many tricky considerations. Can you given me an example of where you are using an AP as a function to return Option/Choice to see if this can be optimised also? Likely case is that the default will remain the same and we try lift in optimization stage if conditions met. |
Beta Was this translation helpful? Give feedback.
-
@gerardtoconnor I don't do it that often, actually. I was just wondering if you planned on adding a different representation like struct APs would have done. But after thinking for a while, I also wonder if such an optimization can't apply more generically. We already have But the proposed optimization already has to figure out how to merge calls to the AP cases with the branches of the consuming So for this example type Temperature =
| Celsius of int
| Farenheit of int
let inline temperature (input:string) =
let n = Int32.Parse(input.Substring(0, input.Length-2)) // e.g. 24°C, 75°F
if input.EndsWith("°C") then Celsius n else Farenheit n
let app (argv:string[]) =
match temperature argv.[0] with
| Celsius n -> printfn "%d °C" n
| Farenheit n -> printfn "%d °F" n the compiler would elide the allocation of |
Beta Was this translation helpful? Give feedback.
-
I have already thought about this optimization generally, why I proposed it here, in an ideal world, with exception of persisted state DUs, we should in theory be able to lift and replace matches with direct calls, it's just easier to implement first with AP as the function bind is in place without value captures etc to worry about, as well as functions usually being small, not blowing up on inlines. |
Beta Was this translation helpful? Give feedback.
-
@gerardtoconnor Sure, that's reasonable! I just wanted to write that down; I love F# but I spend more time than usual removing unintended allocations, so I stay tuned to issues like this one. |
Beta Was this translation helpful? Give feedback.
-
@dsyme, since Also, that it's implemented is not yet clear from the rfc: https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1039-struct-representation-for-active-patterns.md (I don't know about the status of |
Beta Was this translation helpful? Give feedback.
-
The ValueOption section has its own set of RFCs, e.g. https://github.com/fsharp/fslang-design/blob/master/FSharp.Core-4.5.0.0/FS-1057-valueoption.md I'll clear out the section in the RFC for this issue, which does remain unimplemented. |
Beta Was this translation helpful? Give feedback.
-
Makes sense, and the new title certainly helps. I wasn't aware of the other RFC, tx. |
Beta Was this translation helpful? Give feedback.
-
No worries! Thanks for pinging. Hard to keep track of these sorts of things over time 🙂 |
Beta Was this translation helpful? Give feedback.
-
Discussion for https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1039-struct-representation-for-active-patterns.md
Beta Was this translation helpful? Give feedback.
All reactions