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

[dsyme]

RFC https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1043-extension-members-for-operators-and-srtp-constraints.md

This is work by @TobyShaw and myself to implement RFC FS-1043. This PR brings #3582 up-to-date with master

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

Things from previous PR

• [WIP, FS-1043] Extension members visible to trait constraints #3582 (comment)

[dsyme]

How would this work with the Expression Tree? Right now the TraitCall pattern only gives you the method name and little information else (like whether it is an instance or a static member). Fable has to resolve the exact overload by itself, for example, which can be tricky sometimes. I'm not sure we could locate an extension member if the TraitCall doesn't gives you a reference to it.

@alfonsogarciacaro It is critical that we update the TAST (and the FCS API) to include solutions (witnesses) for trait constraints. This has long been a flaw in the TAST and our code generation, leading to horrors where we rediscover the solution during code generation and likewise where we rediscover solutions using reflection in FSHarp.Core.

I need to finally address this problem and record solutions to trait constraints in the TAST itself, and possibly pass them as witnesses in the underlying generic code used for reflection calls and quotation evaluation. This is very tricky and awkward as we can't break compat, I need to prototype carefully. Much of the compiler cleanup and documentation I've been doing has been preparing to storm this particular castle, or die doing it.

[realvictorprm]

@dsyme @cartermp can we discuss that point about easy accessability to experimental compiler features?

[kevmal]

testfiles\test.fs does not compile:

> .\artifacts\bin\fsc\Release\net46\fsc.exe .\testfiles\test.fs
Microsoft (R) F# Compiler version 10.2.3 for F# 4.5
Copyright (c) Microsoft Corporation. All Rights Reserved.

testfiles\test.fs(11,20): error FS0043: The type 'int' does not support the operator '++'

testfiles\test.fs(32,20): error FS0043: The type 'int' does not support the operator '++'

testfiles\test.fs(40,20): error FS0043: The type 'int' does not support the operator '++'

testfiles\test.fs(80,23): error FS0001: The type 'MyType' does not support the operator '+'

testfiles\test.fs(80,21): error FS0043: The type 'MyType' does not support the operator '+'

....

[dsyme]

@dsyme @cartermp can we discuss that point about easy accessability to experimental compiler features?

My reply was "In this case, compile the branch and use it" :)

(We could also make the links to the VSIX artifacts from the CI builds more apparent)

[kevmal]

@dsyme Not sure what the best way is to help with this one. So I'll just outline what I found here:

In ConstraintSolver.fs GetRelevantExtensionMethodsForTrait is never called. Looks like GetRelevantMethodsForTrait should be

/// Only consider overload resolution if canonicalizing or all the types are now nominal. 
/// That is, don't perform resolution if more nominal information may influence the set of available overloads 
and GetRelevantMethodsForTrait (csenv:ConstraintSolverEnv) permitWeakResolution nm (TTrait(tys, _, memFlags, argtys, rty, soln, extSlns, ad) as traitInfo) : MethInfo list =
    let results = 
        if permitWeakResolution || MemberConstraintSupportIsReadyForDeterminingOverloads csenv traitInfo then
            let m = csenv.m
            let minfos = 
                match memFlags.MemberKind with
                | MemberKind.Constructor ->
                    tys |> List.map (GetIntrinsicConstructorInfosOfType csenv.SolverState.InfoReader m)
                | _ ->
                    tys |> List.map (GetIntrinsicMethInfosOfType csenv.SolverState.InfoReader (Some nm, AccessibleFromSomeFSharpCode, AllowMultiIntfInstantiations.Yes) IgnoreOverrides m) 

            // Merge the sets so we don't get the same minfo from each side 
            // We merge based on whether minfos use identical metadata or not. 
            let minfos = List.reduce (ListSet.unionFavourLeft MethInfo.MethInfosUseIdenticalDefinitions) minfos
            
            // Get the extension method that may be relevant to solving the constraint as MethInfo objects.
            // Extension members are not used when canonicalizing prior to generalization (permitWeakResolution=true)
            let extMInfos = 
                if MemberConstraintSupportIsReadyForDeterminingOverloads csenv traitInfo then 
                    GetRelevantExtensionMethodsForTrait csenv.m csenv.amap traitInfo
                else []

            let extMInfos = extMInfos |> ListSet.setify MethInfo.MethInfosUseIdenticalDefinitions 

            let minfos = minfos @ extMInfos

            /// Check that the available members aren't hiding a member from the parent (depth 1 only)
            let relevantMinfos = minfos |> List.filter(fun minfo -> not minfo.IsDispatchSlot && not minfo.IsVirtual && minfo.IsInstance)
            minfos
            |> List.filter(fun minfo1 ->
                not(minfo1.IsDispatchSlot && 
                    relevantMinfos
                    |> List.exists (fun minfo2 -> MethInfosEquivByNameAndSig EraseAll true csenv.g csenv.amap m minfo2 minfo1)))
        else 
            []

    // The trait name "op_Explicit" also covers "op_Implicit", so look for that one too.
    if nm = "op_Explicit" then 
        results @ GetRelevantMethodsForTrait csenv permitWeakResolution "op_Implicit" (TTrait(tys, "op_Implicit", memFlags, argtys, rty, soln, extSlns, ad))
    else
        results 

With this change the testfile now has three errors:

testfiles\test.fs(66,63): error FS0001: Type constraint mismatch. The type
    'int'
is not compatible with type
    'MyType'


testfiles\test.fs(67,63): error FS0001: Type constraint mismatch. The type
    'int'
is not compatible with type
    'MyType'


testfiles\test.fs(68,63): error FS0001: Type constraint mismatch. The type
    'int'
is not compatible with type
    'MyType'

[kevmal]

The three remaining errors can be reproduced by

type MyType =
    | MyType of int
/// Locally extending an F# type with a wide range of standard operators
module FSharpTypeWithExtrinsicOperators = 

    [<AutoOpen>]
    module Extensions = 
        type MyType with
            static member (|||)(MyType (x : int), MyType (y : int)) = MyType (x ||| y)
            static member (&&&)(MyType x, MyType y) = MyType (x &&& y)
            static member (^^^)(MyType x, MyType y) = MyType (x ^^^ y)

Here, SolveMemberConstraint matches on minfos, tys, memFlags.IsInstance, nm, argtys and though some branches have a wildcard match on minfos (for example | _, _, false, ("op_Addition" | "op_Subtraction" | "op_Modulus"), [argty1;argty2]) the branch of interest matches on minfos being empty | [], _, false, ("op_BitwiseAnd" | "op_BitwiseOr" | "op_ExclusiveOr"), [argty1;argty2] which it is not due to the extension method. Perhaps this could also just be a wildcard match which allows the above to compile.

Going back to the test file this leaves one last error on

/// Extending a .NET primitive type with new operator
module DotNetPrimtiveWithAmbiguousNewOperator = 
    [<AutoOpen>]
    module Extensions = 
        type System.Int32 with
            static member (++)(a: int, b: int) = a 

    [<AutoOpen>]
    module Extensions2 = 
        type System.Int32 with
            static member (++)(a: int, b: string) = a 

    let f x = 1 ++ x

Where x (last line) is inferred to have type obj producing the error

testfiles\test.fs(25,17): error FS0071: Type constraint mismatch when applying the default type 'obj' for a type inference variable. internal error: Exception of type 'FSharp.Compiler.ConstraintSolver+LocallyAbortOperationThatFailsToResolveOverload' was thrown.

With a type annotation (let f (x : string) = 1 ++ x) the test file compiles.

[kevmal]

Using Extension attribute does not work:

open System.Runtime.CompilerServices
[<Extension>]
type Ext2() = 
    [<Extension>]
    static member inline (!@)(a : string) = a.Length

!@"324234"

Error FS0043 op_BangAt is not a static method

[dsyme]

@kevmal Super work, thanks. Could you submit the first change as a PR to my branch? Thanks

[kevmal]

ExtensionAttribute members work fine. For example

open System.Runtime.CompilerServices
[<Extension>]
type Ext2() = 
    [<Extension>]
    static member Bleh(a : string) = a.Length

let inline bleh s = (^a : (member Bleh : unit -> int) s)

works. The last operator example, as the error states, is an instance member and shouldn't work. Currently I think that's okay because

type System.Int32 with 
    static member (!@)(a:string) = a.Length

!@"32423"

also works. My only concern is it would make sense to restrict the above (you must extend string with string operators). But it's important to me to be able to do something like (not exactly this, just staying on the same theme)

type System.Int32 with 
    static member inline (!@)(a) = (^a : (member Length : int) a)

where, in similar cases, I'd usually have to use the ExtensionAttribute.

With that said there seems to be an issue:

type System.Int32 with 
    static member inline (+)(a, b) = Array.map2 (+) a b

[|1;2;3|] + [|2;3;4|] //Okay
[|TimeSpan.Zero|] + [|TimeSpan.Zero|] //Okay
[|1m|] + [|2m|] //Okay
[|1uy|] + [|2uy|] //Okay
[|1L|] + [|2L|] //Okay
[|1I|] + [|2I|] //Okay
[| [|1 ; 1|]; [|2|] |] + [| [|2; 2|]; [|3|] |] //Okay
[|"1"|] + [|"2"|] //error FS0001
[|1.f|] + [|2.f|] //error FS0001
[|1.0|] + [|2.0|] //error FS0001

Where the errors (for float/single/string) are:

error FS0001: Type constraint mismatch. The type
    'float'
is not compatible with type
    ''a []

[dsyme]

Here, SolveMemberConstraint matches on minfos, tys, memFlags.IsInstance, nm, argtys and though some branches have a wildcard match on minfos (for example | _, _, false, ("op_Addition" | "op_Subtraction" | "op_Modulus"), [argty1;argty2]) the branch of interest matches on minfos being empty | [], _, false, ("op_BitwiseAnd" | "op_BitwiseOr" | "op_ExclusiveOr"), [argty1;argty2] which it is not due to the extension method. Perhaps this could also just be a wildcard match which allows the above to compile.

Yes, I remember reverting this during integration, thanks, I will fix it now

[dsyme]

Re this:

[|"1"|] + [|"2"|] //error FS0001

I think this should be an error. + constrains to ( ^a or ^b) : (static member ( + ) : ^a * ^b -> ^c). This means that the type providing the witness must be either the left or right hand operand type.

[dsyme]

@kevmal Thanks for your help! I've pushed the required fixes to the branch and testfiles\test.fs now compiles.

If you'd like to contribute a bit of code, then moving test.fs into a new tests\fsharp\core\... test file would be great, and adding execution tests that check the right values are computed.

[kevmal]

@dsyme With that change (wildcard match on minfos) all solution tests passed. I think the overall issue relates to the last TODO you listed. GetRelevantExtensionMethodsForTrait is returning extension methods that are not actually valid (in this case the one that's being defined).

So in the context of the previous issue I brought up... With integertypes we hit this branch

      | _, _, false, ("op_Addition" | "op_Subtraction" | "op_Modulus"), [argty1;argty2]
          when // Ignore any explicit +/- overloads from any basic integral types
               (minfos |> List.forall (fun minfo -> isIntegerTy g minfo.ApparentEnclosingType ) &&

Since the check minfos passes for integer types it's fine but for double/float/string the invalid method (array extension op) causes this branch to skip and error on the available method. In the case of TimeSpan there's a valid method in minfos and it's fine.

Though with that said I'm not sure why the above would not be List.forall (fun minfo -> IsNumericOrIntegralEnumType g minfo.ApparentEnclosingType )

[kevmal]

With

    type System.Int32 with 
        static member inline (+)(struct(a,b), struct(c,d)) = struct(a + c, b + d)
        static member inline (+)((a,b), (c,d)) = (a + c, b + d)

(1,2) + (3,4) compiles and runs fine but struct(1,2) + struct(3,4) compiles and fails at runtime with

Unhandled Exception: System.TypeInitializationException: The type initializer for 'AdditionDynamicImplTable`3' threw an exception. ---> System.NotSupportedException: Dynamic invocation of op_Addition involving coercions is not supported.
at [email protected](Unit unitVar0)
at Microsoft.FSharp.Core.LanguagePrimitives.AdditionDynamicImplTable`3..cctor()
--- End of inner exception stack trace ---
at Microsoft.FSharp.Core.LanguagePrimitives.AdditionDynamicImplTable`3.get_Impl()
at Microsoft.FSharp.Core.LanguagePrimitives.AdditionDynamic[T1,T2,TResult](T1 x, T2 y)
at <StartupCode$test>.$Test$fsx.main@()

It compiles to

      $Test$fsx.x@1 = new ValueTuple<int, int>(1, 2);
      $Test$fsx.x@1-1 = Test.TupleOps.x@1;
      $Test$fsx.y@1 = new ValueTuple<int, int>(3, 4);
      $Test$fsx.y@1-1 = Test.TupleOps.y@1;
      $Test$fsx.v1@310-3 = LanguagePrimitives.AdditionDynamic<ValueTuple<int, int>, ValueTuple<int, int>, ValueTuple<int, int>>(Test.TupleOps.x@1, Test.TupleOps.y@1);

[dsyme]

Closed in favour of #6805