[WIP, FS-1043] Extension members visible to trait constraints

[TobyShaw]

Language feature: Make extension members visible to trait constraints

Implements related language suggestions

• Support operators in type extensions, and also operators on internal types [RFC FS-1043] fsharp/fslang-suggestions#230
• Allow Type Extensions and Extension Methods to Satisfy Constraints fsharp/fslang-suggestions#29

TODO items from the code

• TastOps.fs extSlns // TODO: do we need to remap here???
• TastPickle.fs extSlns starts empty. TODO: check the ramifications of this when inlining solved trait calls from other assemblies
• Optimizer.fs // TODO: consider what happens when the expression refers to extSlns that have become hidden
• Check use of None for TraitFreshener, e.g. https://github.com/Microsoft/visualfsharp/pull/3582/files#diff-5b9ab9dd9d7133aaf23add1048742031R576
• ConstraintSolver.fs // TODO: check the use of 'allPairs' - not all these extensions apply to each type variable.

Things to test

• explicit instantiations of all the different possible generic things
• Using C#-style extension members defined in F# to satisfy constraints
• More uses of extension constraints

Old comments by Toby

Obviously not merge-ready. Opening it up for discussion and feedback.

Seems to work in the simple examples I've tested.

I need to test it with more complex examples that originally motivated this patch.
I'm not happy with my strategy for passing around the information I need, I feel like it could be done in a more clean manner.

[abelbraaksma]

@TobyShaw, I'm curious, is this an improvement that would allow something like the following, for instance?

    module Test =
        type System.Int64 with
            static member MaxValue = 12

        let inline getMax< ^a when ^a: (static member MaxValue: ^a)> (x: ^a) = 
                let max = (^a: (static member MaxValue: ^a) ())
                max

        let testResult = getMax 12L

Currently that (righly, according to the docs) throws a compile-time exception:

BTW: this actually comes from a real-world scenario (see this stackoverflow question on MinValue and MaxValue fields, since MaxValue only exists as field and static member constraints have another limitation: they don't apply to fields or constants. The only current workaround is to create wrapper types, with added complexities and runtime + GC overhead.

So, if your fix addresses that issue, it would be HUGE improvement! Even though it would still mean, of course, adding those fields as members, but code like this (by @gusty) could well become way more readable and concise with such additions.

[TobyShaw]

I believe so yes, but I'll hold off on making any bold claims until I've done more testing. I'll update here when I have more information.

[dsyme]

@TobyShaw If you could update to master that would be great (all the conflicts will be about spaces I think)

[dsyme]

@TobyShaw Great work :) I've added links to two language design suggestions that you should review, and add extra test cases for.

We will need to add some kind of access checking to allow internal members to be used in solving constraints, possibly in this PR - I'm not certain though

[TobyShaw]

Seems like you linked the same suggestion twice? I assume you meant fsharp/fslang-suggestions#243

Fixed it up

[dsyme]

@TobyShaw I meant fsharp/fslang-suggestions#29, thanks

[saul]

I believe this project file needs reverting, see the project.json changes below. I trust @dsyme can help you integrate some compiler unit tests

[TobyShaw]

Oops, didn't mean to commit this. Good spot

[gusty]

@TobyShaw Awesome, this is a very interesting feature and I consider it another step down the road to trait or typeclasses.

@abelbraaksma I agree with you. A project like FSharpPlus will benefit with this feature, it will simplify a lot the code used there internally and the resulting signatures as well.

@dsyme An interesting test will be to compile the library as it is now with this version of the compiler to check if there is a breaking change.
Then I will try to port the code to take advantage of this feature and see how far can be used, I mean if there is any limitation.

[TobyShaw]

Okay, done some testing.

Many functions work perfectly as expected:
Append : ^A * ^A -> ^A
Show : ^A -> String
Incr : ^A -> ^A

Some functions don't: I don't get a unique overload, indicating a bug in my implementation, I reckon this can be fixed pretty easily:
Max : unit -> ^A

The three large problems which make this useless at the moment are:

• Indirections don't seem to be working currently, pretty huge problem which I need to look into.
  let inline (++) a b = Append a b // Won't detect it any more

Maybe a result of this weirdness?

• Doesn't work across Module boundaries.

Although it can find the correct extension member when constraint solving, the NameResolutionEnv I pass on to the optimiser/code generator is only the one at the top level. Ie. I need to find a more localised means of reobtaining the correct extension member in CodegenWitnessThatTypSupportsTraitConstraint. Seems rather obvious that passing around the global NRE is a dumb idea, and this shows why.

As a side: if someone could explain to me why we seem to do the constraint solving twice, that would be helpful. What I mean by this is: we record the solution to a trait constraint, and then when we enter CodegenWitnessThatTypSupportsTraitConstraint, the solution is back to None. I guess if I can resolve this, then a lot of the other problems go away.

• Doesn't currently work with extension members on generic types, causes the compiler to crash, I think this should be an easy fix though.

[dsyme]

@TobyShaw Could you merge master into your branch please?

git pull http://github.com/Microsoft/visualfsharp master + and resolve conflicts

[dsyme]

The three large problems which make this useless at the moment are...

Indirections don't seem to be working currently, pretty huge problem which I need to look into.
let inline (++) a b = Append a b // Won't detect it any more

I'm not sure what you mean here - could you give a full example please?

Doesn't work across Module boundaries. Although it can find the correct extension member when constraint solving, the NameResolutionEnv I pass on to the optimiser/code generator is only the one at the top level. Ie. I need to find a more localised means of reobtaining the correct extension member in CodegenWitnessThatTypSupportsTraitConstraint. Seems rather obvious that passing around the global NRE is a dumb idea, and this shows why.

Yes, we have to find a way to address this.

As a side: if someone could explain to me why we seem to do the constraint solving twice, that would be helpful.

Basically because we don't correctly pass witness solutions all the way through.

Doesn't currently work with extension members on generic types, causes the compiler to crash, I think this should be an easy fix though.

Yes, that should be an easy fix hopefully

[gusty]

@TobyShaw I worked before with the Constraint Solver and I also noticed that it solves twice (actually before my PRs it was solving the same stuff 4 times or more depending on the scenario).

But if my memory is not bad I stopped optimizing at the point I noticed that the CodegenWitnessThatTypSupportsTraitConstraint gives different results in some cases, that's why I didn't went further. And my feeling was that many bugs I reported regarding inconsistent Overload Resolution are caused by those differences, but I'm not 100% sure.

[TobyShaw]

@dsyme For an example of the indirection causing failures:

type System.Int32 with
  static member Append(a : System.Int32, b : System.Int32) = a + b

let inline append< ^A when ^A : (static member Append : ^A * ^A -> ^A) > (a : ^A) (b : ^A) =
  ( ^A : (static member Append : ^A * ^A -> ^A) (a,b) )

let inline append2 x y = append x y

[<EntryPoint>]
let main args =
  printfn "%A" (append 1 2)
  printfn "%A" (append2 1 2)
  1

The first line (using append) works fine. The second line (using append2) fails, despite their equivalency.

[TobyShaw]

The merge conflict was pretty massive, may take a while to resolve

[dsyme]

The first line (using append) works fine. The second line (using append2) fails, despite their equivalency.

OK, thanks, that does need to be fixed :)

[dsyme]

@TobyShaw I have merged with master and pushed to your branch. Still building and ironing out issues with that

[dsyme]

@TobyShaw OK, this is merged now, and should now build correctly. I'm going to play with it a bit and develop a list of acceptance criteria

[TobyShaw]

I'm afraid Don has taken this beyond my understanding, and I'm more focused on FS-1023 at the moment.

[realvictorprm]

OK @TobyShaw, I'll try to takeover then 🤔

[gusty]

@realvictorprm I think before merging this PR we should fix all current oddities with SRTP

[realvictorprm]

@gusty I agree on this + I'm also aware of this. If you have some time left it would be great to work together on it @gusty 😃

[gusty]

@realvictorprm I'm happy to join efforts on this.

I think the first step is to add corner case tests that will help us to see what's working and what's not, so we can work on what's not without breaking anything.

[realvictorprm]

great!

Yeah I think so too. I'm right now digging more into the compiler through just implementing the inline interfaces idea (it's not that complex).

[realvictorprm]

I would love to get the simpler SRTP syntax, inline interfaces and this PR merged for the next F# Version.

[jackfoxy]

Now that 4.5 is out and the span work is done, I hope this makes it to the next release.

[gerardtoconnor]

Just to add to edge cases under review:

I was trying to use PrintfFormat to type enforce resolution of parsers and it initially appeared to work for int but then same approach for string did not work... while float did work, so I thought was Value/Ref type issue but then tried bool and that didn't work like String.

int & float work, string & bool do not!?

(ParseApply methods are dummy implementations for now)

        type System.String  with static member inline ParseApply (path:string) (fn: string -> ^b) : ^b = fn ""
        type System.Int32   with static member inline ParseApply (path:string) (fn: int   -> ^b) : ^b = fn 0
        type System.Double  with static member inline ParseApply (path:string) (fn: float -> ^b) : ^b = fn 0.
        type System.Boolean with static member inline ParseApply (path:string) (fn: bool -> ^b) : ^b = fn true
        
        let inline parser (fmt:PrintfFormat< ^a -> ^b,_,_,^b>) (fn:^a -> ^b) (v:string) : ^b 
            when ^a : (static member ParseApply: string -> (^a -> ^b) -> ^b) =
            (^a : (static member ParseApply: string -> (^a -> ^b) -> ^b)(v,fn))

        let inline patternTest (fmt:PrintfFormat< ^a -> Action< ^T>,_,_,Action< ^T>>) (fn:^a -> Action< ^T>) v : Action< ^T> = parser fmt fn v

        let parseFn1 = patternTest "adfadf%i" (fun v -> printfn "%i" v; Unchecked.defaultof<Action<unit>> ) // works
        let parseFn2 = patternTest "adf%s245" (fun v -> printfn "%s" v; Unchecked.defaultof<Action<unit>> ) // ERROR
        let parseFn3 = patternTest "adfadf%f" (fun v -> printfn "%f" v; Unchecked.defaultof<Action<unit>> ) // works
        let parseFn4 = patternTest "adfadf%b" (fun v -> printfn "%b" v; Unchecked.defaultof<Action<unit>> ) // ERROR

The error I get on parseFn2 function format string input is The type 'string' does not support the operator 'ParseApply', similarly, parseFn4 error is The type 'bool' does not support the operator 'ParseApply'.

Seems to apply to

[KevinRansom]

@TobyShaw, @dsyme, this feels like an orphaned PR. Who will take it over, or can I close it?

[jackfoxy]

@KevinRansom please do not close. I have an interest in seeing this fixed. Unfortunately I don't have the time to come up to speed and attempt fixing it myself right now.

[KevinRansom]

@jackfoxy without someone to drive it, it is just repo noise. I am trying to clean out some of that noise. I am happy for PRs to remain open while they are being driven to completion.

Kevin

[jackfoxy]

@KevinRansom OK, I can understand that, but I think the language suggestion this references
fsharp/fslang-suggestions#29 is closed for unknown reason. I would like to keep alive "Allow Type Extensions and Extension Methods to Satisfy Constraints" as an approved language feature. I kind of consider it a bug that it does not work.

[KevinRansom]

@jack-pappas for sure re-open the language suggestion, or ask for a justification for the closure.

I will keep it open for a bit to see if we can get someone to drive this to completion. But it will close if no one steps forward.

[jackfoxy]

@KevinRansom supposedly this one fsharp/fslang-suggestions#230 includes Extension Methods to Satisfy Constraints, but I'm not so sure.

[cartermp]

I re-opened it. It was auto-closed by a tool to migrate from UserVoice, where it was marked as a duplicate of a suggestion that needed the same underlying work.

[realvictorprm]

Please don't close this. I really want to take this over as soon as the SRTP bugs are fixed (which unfortunately will take some as it's very difficult).

[dsyme]

I'll keep this one open. It's a significantly important feature and I'll integrate bit by bit

[KevinRansom]

@dsyme sweet :-)

[dsyme]

Closing in favour of #6286 which has brought this up-to-date re. master

[dsyme]

I was trying to use PrintfFormat to type enforce resolution of parsers and it initially appeared to work for int but then same approach for string did not work... while float did work, so I thought was Value/Ref type issue but then tried bool and that didn't work like String. int & float work, string & bool do not!?

@gerardtoconnor This is in reply to a really a really old comment, but just wanted to say that I double checked this and none of the four types work. However type errors are shown in two phases in the IDE, so it's not until you clear the errors for string and bool that you get shown the errors for int and float. This is related to the process of applying defaults.

[gusty]

@dsyme I think the process of applying defaults must be reviewed at some point as I'm under the impression that it can be smarter.

Sometimes the compiler applies defaults having a good candidate, I don't have an example right now, but when I was looking at the non-sealed types problem I ended up there. Just my 2 cents.