From 7ed9adcfad9c463de673e6dad9d1dd91568d35ca Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 1 Dec 2020 17:42:55 -0800 Subject: [PATCH 1/5] Trying to fix generic overloads with nullable --- src/fsharp/MethodCalls.fs | 1 + .../NullableOptionalRegressionTests.fs | 35 +++++++++++++++++++ tests/fsharp/FSharpSuite.Tests.fsproj | 1 + 3 files changed, 37 insertions(+) create mode 100644 tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs diff --git a/src/fsharp/MethodCalls.fs b/src/fsharp/MethodCalls.fs index 5339231db31..b16074a609b 100644 --- a/src/fsharp/MethodCalls.fs +++ b/src/fsharp/MethodCalls.fs @@ -213,6 +213,7 @@ let AdjustCalledArgTypeForOptionals (g: TcGlobals) enforceNullableOptionalsKnown else let compgenId = mkSynId range0 unassignedTyparName mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, Typar(compgenId, NoStaticReq, true), false, TyparDynamicReq.No, [], false, false)) + |> mkNullableTy g else calledArgTy diff --git a/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs b/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs new file mode 100644 index 00000000000..8d7af7479a9 --- /dev/null +++ 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 + +[] +module NullableOptionalRegressionTests = + + [] + 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 + +[] +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 index 0006517fd76..c67895f0e24 100644 --- a/tests/fsharp/FSharpSuite.Tests.fsproj +++ b/tests/fsharp/FSharpSuite.Tests.fsproj @@ -58,6 +58,7 @@ + From 1ee002116d1488f90fee9a3e0f035f1838dddaf1 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 1 Dec 2020 17:46:51 -0800 Subject: [PATCH 2/5] Return the nullable if we are not optional --- src/fsharp/MethodCalls.fs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/fsharp/MethodCalls.fs b/src/fsharp/MethodCalls.fs index b16074a609b..7312d5cb67a 100644 --- a/src/fsharp/MethodCalls.fs +++ b/src/fsharp/MethodCalls.fs @@ -209,11 +209,14 @@ 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 or use the Nullable if we are not optional. else - let compgenId = mkSynId range0 unassignedTyparName - mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, Typar(compgenId, NoStaticReq, true), false, TyparDynamicReq.No, [], false, false)) - |> mkNullableTy g + match calledArg.OptArgInfo with + | NotOptional -> + calledArgTy + | _ -> + let compgenId = mkSynId range0 unassignedTyparName + mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, Typar(compgenId, NoStaticReq, true), false, TyparDynamicReq.No, [], false, false)) else calledArgTy From 174ff1a460948433683c9263ea841dae2fbc0a36 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 1 Dec 2020 17:54:24 -0800 Subject: [PATCH 3/5] Update test --- .../Compiler/Regressions/NullableOptionalRegressionTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs b/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs index 8d7af7479a9..93970223ce5 100644 --- a/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs +++ b/tests/fsharp/Compiler/Regressions/NullableOptionalRegressionTests.fs @@ -26,7 +26,7 @@ type Node (child:Node)= let test () = let parent = Node() let b1 = OverloadMeths.Map(parent.child, fun x -> x.child) - // let c1 = OverloadMeths.Map(b1, fun x -> x.child) + let c1 = OverloadMeths.Map(b1, fun x -> x.child) () """ |> withLangVersion50 From 61df0745c6dc0442c516a8f060a2404325ec6164 Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 1 Dec 2020 20:06:08 -0800 Subject: [PATCH 4/5] Get dest of nullable since the caller argument type is not nullable --- src/fsharp/MethodCalls.fs | 4 +- .../Compiler/Language/OptionalInteropTests.fs | 89 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/fsharp/MethodCalls.fs b/src/fsharp/MethodCalls.fs index 7312d5cb67a..c6c30d2b2bc 100644 --- a/src/fsharp/MethodCalls.fs +++ b/src/fsharp/MethodCalls.fs @@ -209,11 +209,11 @@ 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 or use the Nullable if we are not optional. + // If at the beginning of inference then use a type variable or use this if we are not optional. else match calledArg.OptArgInfo with | NotOptional -> - calledArgTy + destNullableTy g calledArgTy | _ -> let compgenId = mkSynId range0 unassignedTyparName mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, Typar(compgenId, NoStaticReq, true), false, TyparDynamicReq.No, [], false, false)) diff --git a/tests/fsharp/Compiler/Language/OptionalInteropTests.fs b/tests/fsharp/Compiler/Language/OptionalInteropTests.fs index a4e4d498400..2f661e78515 100644 --- 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 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,7 @@ open System open CSharpTest Test.M(x = Some 1) +Test.MethodTakingNullables(6, "aaaaaa", 8.0) |> ignore """ let fsharpCoreAssembly = From 6613cc40ef3f42fecefdfab4f29547a8106c357f Mon Sep 17 00:00:00 2001 From: Will Smith Date: Tue, 1 Dec 2020 21:00:51 -0800 Subject: [PATCH 5/5] Trying to pass tests --- src/fsharp/MethodCalls.fs | 8 ++++--- .../Compiler/Language/OptionalInteropTests.fs | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/fsharp/MethodCalls.fs b/src/fsharp/MethodCalls.fs index c6c30d2b2bc..2c643411d90 100644 --- a/src/fsharp/MethodCalls.fs +++ b/src/fsharp/MethodCalls.fs @@ -209,11 +209,13 @@ 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 or use this if we are not optional. + // If at the beginning of inference then use a type variable. else + let destTy = destNullableTy g calledArgTy match calledArg.OptArgInfo with - | NotOptional -> - destNullableTy g calledArgTy + // 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)) diff --git a/tests/fsharp/Compiler/Language/OptionalInteropTests.fs b/tests/fsharp/Compiler/Language/OptionalInteropTests.fs index 2f661e78515..eda7a75e31d 100644 --- a/tests/fsharp/Compiler/Language/OptionalInteropTests.fs +++ b/tests/fsharp/Compiler/Language/OptionalInteropTests.fs @@ -122,6 +122,28 @@ 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 =