Fix generic overloads with nullable (dotnet#10582)

xamarin · Feb 22, 2021 · 9bb4aba · 9bb4aba
1 parent b085093
commit 9bb4aba
Show file tree

Hide file tree

Showing 4 changed files with 156 additions and 3 deletions.
diff --git a/src/fsharp/MethodCalls.fs b/src/fsharp/MethodCalls.fs
@@ -209,10 +209,16 @@ let AdjustCalledArgTypeForOptionals (g: TcGlobals) enforceNullableOptionalsKnown
                 // If neither and we are at the end of overload resolution then use the Nullable
                 elif enforceNullableOptionalsKnownTypes then 
                     calledArgTy
-                // If at the beginning of inference then use a type variable
+                // If at the beginning of inference then use a type variable.
                 else 
-                    let compgenId = mkSynId range0 unassignedTyparName
-                    mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, Typar(compgenId, NoStaticReq, true), false, TyparDynamicReq.No, [], false, false))
+                    let destTy = destNullableTy g calledArgTy
+                    match calledArg.OptArgInfo with
+                    // Use the type variable from the Nullable if called arg is not optional.
+                    | NotOptional when isTyparTy g destTy ->
+                        destTy
+                    | _ ->
+                        let compgenId = mkSynId range0 unassignedTyparName
+                        mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, Typar(compgenId, NoStaticReq, true), false, TyparDynamicReq.No, [], false, false))
             else
                 calledArgTy
 

diff --git a/tests/fsharp/Compiler/Language/OptionalInteropTests.fs b/tests/fsharp/Compiler/Language/OptionalInteropTests.fs
@@ -23,6 +23,94 @@ namespace CSharpTest
     public static class Test
     {
         public static void M(FSharpOption<int> x = null) { }
+        public static int MethodTakingOptionals(int x = 3, string y = "abc", double d = 5.0)
+        {
+            return x + y.Length + (int) d;
+        }
+        public static int MethodTakingNullableOptionalsWithDefaults(int? x = 3, string y = "abc", double? d = 5.0)
+        {
+            return (x.HasValue ? x.Value : -100) + y.Length + (int) (d.HasValue ? d.Value : 0.0);
+        }
+        public static int MethodTakingNullableOptionals(int? x = null, string y = null, double? d = null)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0);
+        }
+        public static int OverloadedMethodTakingOptionals(int x = 3, string y = "abc", double d = 5.0)
+        {
+            return x + y.Length + (int) d;
+        }
+        public static int OverloadedMethodTakingOptionals(int x = 3, string y = "abc", System.Single d = 5.0f)
+        {
+            return x + y.Length + (int) d + 7;
+        }
+        public static int OverloadedMethodTakingNullableOptionalsWithDefaults(int? x = 3, string y = "abc", double? d = 5.0)
+        {
+            return (x.HasValue ? x.Value : -100) + y.Length + (int) (d.HasValue ? d.Value : 0.0);
+        }
+        public static int OverloadedMethodTakingNullableOptionalsWithDefaults(long? x = 3, string y = "abc", double? d = 5.0)
+        {
+            return (x.HasValue ? (int) x.Value : -100) + y.Length + (int) (d.HasValue ? d.Value : 0.0) + 7;
+        }
+        public static int OverloadedMethodTakingNullableOptionals(int? x = null, string y = null, double? d = null)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0);
+        }
+        public static int OverloadedMethodTakingNullableOptionals(long? x = null, string y = null, double? d = null)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? (int) x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0) + 7;
+        }
+        public static int MethodTakingNullables(int? x, string y, double? d)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0);
+        }
+
+        public static int OverloadedMethodTakingNullables(int? x, string y, double? d)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0);
+        }
+        public static int OverloadedMethodTakingNullables(long? x, string y, double? d)
+        {
+            int length;
+            if (y == null)
+                length = -1;
+            else
+                length = y.Length;
+            return (x.HasValue ? (int) x.Value : -1) + length + (int) (d.HasValue ? d.Value : -1.0) + 7;
+        }
+        public static int SimpleOverload(int? x = 3)
+        {
+            return (x.HasValue ? x.Value : 100);
+        }
+
+        public static int SimpleOverload(int x = 3)
+        {
+            return (x + 200);
+        }
     }
 }
             """
@@ -33,6 +121,29 @@ open System
 open CSharpTest
 
 Test.M(x = Some 1)
+Test.MethodTakingNullables(6, "aaaaaa", 8.0) |> ignore
+Test.MethodTakingNullables(6, "aaaaaa", Nullable 8.0) |> ignore
+Test.MethodTakingNullables(6, "aaaaaa", Nullable ()) |> ignore
+Test.MethodTakingNullables(Nullable (), "aaaaaa", 8.0) |> ignore
+Test.MethodTakingNullables(Nullable 6, "aaaaaa", 8.0) |> ignore
+
+Test.MethodTakingNullables(6, "aaaaaa", d=8.0) |> ignore
+Test.MethodTakingNullables(6, "aaaaaa", d=Nullable 8.0) |> ignore
+Test.MethodTakingNullables(6, "aaaaaa", d=Nullable ()) |> ignore
+Test.MethodTakingNullables(Nullable (), "aaaaaa", d=8.0) |> ignore
+Test.MethodTakingNullables(Nullable 6, "aaaaaa", d=8.0) |> ignore
+
+Test.MethodTakingNullables(6, y="aaaaaa", d=8.0) |> ignore
+Test.MethodTakingNullables(6, y="aaaaaa", d=Nullable 8.0) |> ignore
+Test.MethodTakingNullables(6, y="aaaaaa", d=Nullable ()) |> ignore
+Test.MethodTakingNullables(Nullable (), y="aaaaaa", d=8.0) |> ignore
+Test.MethodTakingNullables(Nullable 6, y="aaaaaa", d=8.0) |> ignore
+
+Test.MethodTakingNullables(6, y="aaaaaa", d=8.0) |> ignore
+Test.MethodTakingNullables(6, y="aaaaaa", d=Nullable 8.0) |> ignore
+Test.MethodTakingNullables(6, y="aaaaaa", d=Nullable ()) |> ignore
+Test.MethodTakingNullables(Nullable (), y="aaaaaa", d=8.0) |> ignore
+Test.MethodTakingNullables(Nullable 6, y="aaaaaa", d=8.0) |> ignore
             """
 
         let fsharpCoreAssembly =

diff --git a/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs b/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs
@@ -0,0 +1,35 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace FSharp.Compiler.UnitTests
+
+open NUnit.Framework
+open FSharp.Test.Utilities.Compiler
+
+[<TestFixture()>]
+module NullableOptionalRegressionTests =
+
+    [<Test>]
+    let ``Should compile with generic overloaded nullable methods``() =
+        Fsx """
+open System
+
+type OverloadMeths =
+    static member Map(m: 'T option, f) = Option.map f m
+    static member Map(m: 'T when 'T:null, f) = m |> Option.ofObj |> Option.map f
+    static member Map(m: 'T Nullable, f) = m |> Option.ofNullable |> Option.map f
+
+[<AllowNullLiteral>]
+type Node (child:Node)=
+    new() = new Node(null)
+    member val child:Node = child with get,set
+
+let test () =
+    let parent = Node()
+    let b1 = OverloadMeths.Map(parent.child, fun x -> x.child)
+    let c1 = OverloadMeths.Map(b1, fun x -> x.child)
+    ()
+        """
+        |> withLangVersion50
+        |> typecheck
+        |> shouldSucceed
+        |> ignore
diff --git a/tests/fsharp/FSharpSuite.Tests.fsproj b/tests/fsharp/FSharpSuite.Tests.fsproj
@@ -66,6 +66,7 @@
     <Compile Include="Compiler\Language\StringConcatOptimizationTests.fs" />
     <Compile Include="Compiler\Language\ComputationExpressionTests.fs" />
     <Compile Include="Compiler\Stress\LargeExprTests.fs" />
+    <Compile Include="Compiler\Regressions\NullableOptionalRegressionTests.fs" />
     <Compile Include="Compiler\Regressions\IndexerRegressionTests.fs" />
     <Compile Include="Compiler\Regressions\ForInDoMutableRegressionTest.fs" />
     <Compile Include="Compiler\Libraries\Core\LanguagePrimitives\CastToUnitsTests.fs" />