From af531608420c7ff0274e0103107fefd02023cf7a Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 17 Feb 2025 11:00:49 +0100 Subject: [PATCH 1/4] test --- .../Nullness/NullableReferenceTypesTests.fs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index 018b81c9384..77469079409 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -146,6 +146,30 @@ let doNotWarnOnDowncastRepeatedNestedNullable(o:objnull) = o :? list<((AB | null Error 3060, Line 7, Col 51, Line 7, Col 97, "This type test or downcast will erase the provided type 'List | null' to the type 'List'"] +[] +let ``Can infer nullable type if first match handler returns null`` () = + FSharp """module TestLib + +let myFunc x = + match x with + | 0 -> null + | i -> string i + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + +[] +let ``Can infer nullable type from first branch of ifthenelse`` () = + FSharp """module TestLib + +let myFunc x = + if x = 0 then null else string x + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + [] let ``Can convert generic value to objnull arg`` () = FSharp """module TestLib From 742cd3d7d9c804066a1b348af7d489080b0c4b5b Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 17 Feb 2025 17:28:29 +0100 Subject: [PATCH 2/4] Allow first branch of if and match dictate nullness of the resulting type --- .../Checking/Expressions/CheckExpressions.fs | 2 + .../Expressions/CheckExpressionsOps.fs | 5 ++ src/Compiler/TypedTree/TypedTreeOps.fs | 13 +++-- src/Compiler/TypedTree/TypedTreeOps.fsi | 2 + .../Nullness/NullableReferenceTypesTests.fs | 57 ++++++++++++++++++- 5 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index d80470b9e53..df64c4da62e 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -10634,6 +10634,7 @@ and TcLinearExprs bodyChecker cenv env overallTy tpenv isCompExpr synExpr cont = | Some synElseExpr -> let env = { env with eContextInfo = ContextInfo.ElseBranchResult synElseExpr.Range } + TryAllowFlexibleNullnessInControlFlow (*isFirst=*) true g overallTy.Commit // tailcall TcLinearExprs bodyChecker cenv env overallTy tpenv isCompExpr synElseExpr (fun (elseExpr, tpenv) -> let resExpr = primMkCond spIfToThen m overallTy.Commit boolExpr thenExpr elseExpr @@ -10697,6 +10698,7 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC | DebugPointAtTarget.No -> resultEnv let resultExpr, tpenv = TcExprThatCanBeCtorBody cenv resultTy resultEnv tpenv synResultExpr + TryAllowFlexibleNullnessInControlFlow isFirst cenv.g resultTy.Commit let target = TTarget(vspecs, resultExpr, None) diff --git a/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs b/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs index 3552b1345f7..0e39551694e 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs @@ -18,6 +18,11 @@ open FSharp.Compiler.TypedTreeBasics open FSharp.Compiler.TypedTreeOps open FSharp.Compiler.SyntaxTreeOps +let TryAllowFlexibleNullnessInControlFlow isFirst (g: TcGlobals.TcGlobals) ty = + match isFirst, g.checkNullness, IsSupportsNullTyparTy g ty with + | true, true, ValueSome tp -> tp.SetSupportsNullFlex(true) + | _ -> () + let CopyAndFixupTypars g m rigid tpsorig = FreshenAndFixupTypars g m rigid [] [] tpsorig diff --git a/src/Compiler/TypedTree/TypedTreeOps.fs b/src/Compiler/TypedTree/TypedTreeOps.fs index f83e48a7c8f..ff6a7318689 100644 --- a/src/Compiler/TypedTree/TypedTreeOps.fs +++ b/src/Compiler/TypedTree/TypedTreeOps.fs @@ -9171,11 +9171,14 @@ let isReferenceTyparTy g ty = | ValueNone -> false -let isSupportsNullTyparTy g ty = +let IsSupportsNullTyparTy g ty = if isReferenceTyparTy g ty then - (destTyparTy g ty).Constraints |> List.exists (function TyparConstraint.SupportsNull _ -> true | _ -> false) + let tp = destTyparTy g ty + if tp.Constraints |> List.exists (function TyparConstraint.SupportsNull _ -> true | _ -> false) then + ValueSome tp + else ValueNone else - false + ValueNone let TypeNullNever g ty = let underlyingTy = stripTyEqnsAndMeasureEqns g ty @@ -9203,7 +9206,7 @@ let TypeNullIsExtraValue g m ty = | ValueNone -> // Consider type parameters - isSupportsNullTyparTy g ty + (IsSupportsNullTyparTy g ty).IsSome // Any mention of a type with AllowNullLiteral(true) is considered to be with-null let intrinsicNullnessOfTyconRef g (tcref: TyconRef) = @@ -9317,7 +9320,7 @@ let TypeNullIsExtraValueNew g m ty = | NullnessInfo.WithNull -> true) || // Check if the type has a ': null' constraint - isSupportsNullTyparTy g ty + (IsSupportsNullTyparTy g ty).IsSome /// The pre-nullness logic about whether a type uses 'null' as a true representation value let TypeNullIsTrueValue g ty = diff --git a/src/Compiler/TypedTree/TypedTreeOps.fsi b/src/Compiler/TypedTree/TypedTreeOps.fsi index 932e465b0e9..1710cbe7036 100755 --- a/src/Compiler/TypedTree/TypedTreeOps.fsi +++ b/src/Compiler/TypedTree/TypedTreeOps.fsi @@ -1829,6 +1829,8 @@ val TypeHasAllowNull: TyconRef -> TcGlobals -> range -> bool val TypeNullIsExtraValueNew: TcGlobals -> range -> TType -> bool +val IsSupportsNullTyparTy: TcGlobals -> TType -> Typar voption + val TypeNullNever: TcGlobals -> TType -> bool val TypeHasDefaultValue: TcGlobals -> range -> TType -> bool diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index 77469079409..ad4788aa0ce 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -153,7 +153,36 @@ let ``Can infer nullable type if first match handler returns null`` () = let myFunc x = match x with | 0 -> null - | i -> string i + | i -> "x" + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + +[] +let ``Can NOT infer nullable type if second match handler returns null`` () = + FSharp """module TestLib + +let myFunc x defaultValue = + match x with + | 0 -> defaultValue + | 1 -> null + | i -> "y" + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics [Error 3261, Line 7, Col 12, Line 7, Col 15, "Nullness warning: The type 'string' does not support 'null'.. See also test.fs(7,11)-(7,14)."] + +[] +let ``Can infer nullable type if first match handler returns masked null`` () = + FSharp """module TestLib + +let thisIsNull : string|null = null +let myFunc x = + match x with + | 0 -> thisIsNull + | i -> "x" """ |> asLibrary |> typeCheckWithStrictNullness @@ -164,12 +193,36 @@ let ``Can infer nullable type from first branch of ifthenelse`` () = FSharp """module TestLib let myFunc x = - if x = 0 then null else string x + if x = 0 then null else "x" """ |> asLibrary |> typeCheckWithStrictNullness |> shouldSucceed +[] +let ``Can NOT infer nullable type from second branch of ifthenelse`` () = + FSharp """module TestLib + +let myFunc x = + if x = 0 then "x" else null + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics [Error 3261, Line 4, Col 28, Line 4, Col 32, "Nullness warning: The type 'string' does not support 'null'."] + +[] +let ``Can NOT infer nullable type from second branch of nested elifs`` () = + FSharp """module TestLib + +let myFunc x = + if x = 0 then "x" elif x=1 then null else "y" + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics [Error 3261, Line 4, Col 37, Line 4, Col 41, "Nullness warning: The type 'string' does not support 'null'."] + [] let ``Can convert generic value to objnull arg`` () = FSharp """module TestLib From 7c74f1fa424d1b54884577ba76083df229116214 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 17 Feb 2025 18:14:57 +0100 Subject: [PATCH 3/4] notes --- docs/release-notes/.FSharp.Compiler.Service/9.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md index f64f090744b..0b8957725cb 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md @@ -3,6 +3,7 @@ * Fix Realsig+ generates nested closures with incorrect Generic ([Issue #17797](https://github.com/dotnet/fsharp/issues/17797), [PR #17877](https://github.com/dotnet/fsharp/pull/17877)) * Fix optimizer internal error for records with static fields ([Issue #18165](https://github.com/dotnet/fsharp/issues/18165), [PR #18280](https://github.com/dotnet/fsharp/pull/18280)) * Fix nullness warning with flexible types ([Issue #18056](https://github.com/dotnet/fsharp/issues/18056), [PR #18266](https://github.com/dotnet/fsharp/pull/18266)) +* Allow first branches of match and if expressions to return nullable results ([Issue #18015](https://github.com/dotnet/fsharp/issues/18015), [PR #18322](https://github.com/dotnet/fsharp/pull/18322)) * Fix internal error when missing measure attribute in an unsolved measure typar. ([Issue #7491](https://github.com/dotnet/fsharp/issues/7491), [PR #18234](https://github.com/dotnet/fsharp/pull/18234)== * Set `Cancellable.token` from async computation ([Issue #18235](https://github.com/dotnet/fsharp/issues/18235), [PR #18238](https://github.com/dotnet/fsharp/pull/18238)) * Fix missing nullness warning when static upcast dropped nullness ([Issue #18232](https://github.com/dotnet/fsharp/issues/18232), [PR #18261](https://github.com/dotnet/fsharp/pull/18261)) From 1d34e22dd258a88a8213bfd6f90853f33580fc68 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 21 Feb 2025 10:49:47 +0100 Subject: [PATCH 4/4] PR feedback --- src/Compiler/Checking/Expressions/CheckExpressionsOps.fs | 2 +- src/Compiler/TypedTree/TypedTreeOps.fs | 6 +++--- src/Compiler/TypedTree/TypedTreeOps.fsi | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs b/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs index 0e39551694e..4b9de82d758 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressionsOps.fs @@ -19,7 +19,7 @@ open FSharp.Compiler.TypedTreeOps open FSharp.Compiler.SyntaxTreeOps let TryAllowFlexibleNullnessInControlFlow isFirst (g: TcGlobals.TcGlobals) ty = - match isFirst, g.checkNullness, IsSupportsNullTyparTy g ty with + match isFirst, g.checkNullness, GetTyparTyIfSupportsNull g ty with | true, true, ValueSome tp -> tp.SetSupportsNullFlex(true) | _ -> () diff --git a/src/Compiler/TypedTree/TypedTreeOps.fs b/src/Compiler/TypedTree/TypedTreeOps.fs index ff6a7318689..4cabe94d952 100644 --- a/src/Compiler/TypedTree/TypedTreeOps.fs +++ b/src/Compiler/TypedTree/TypedTreeOps.fs @@ -9171,7 +9171,7 @@ let isReferenceTyparTy g ty = | ValueNone -> false -let IsSupportsNullTyparTy g ty = +let GetTyparTyIfSupportsNull g ty = if isReferenceTyparTy g ty then let tp = destTyparTy g ty if tp.Constraints |> List.exists (function TyparConstraint.SupportsNull _ -> true | _ -> false) then @@ -9206,7 +9206,7 @@ let TypeNullIsExtraValue g m ty = | ValueNone -> // Consider type parameters - (IsSupportsNullTyparTy g ty).IsSome + (GetTyparTyIfSupportsNull g ty).IsSome // Any mention of a type with AllowNullLiteral(true) is considered to be with-null let intrinsicNullnessOfTyconRef g (tcref: TyconRef) = @@ -9320,7 +9320,7 @@ let TypeNullIsExtraValueNew g m ty = | NullnessInfo.WithNull -> true) || // Check if the type has a ': null' constraint - (IsSupportsNullTyparTy g ty).IsSome + (GetTyparTyIfSupportsNull g ty).IsSome /// The pre-nullness logic about whether a type uses 'null' as a true representation value let TypeNullIsTrueValue g ty = diff --git a/src/Compiler/TypedTree/TypedTreeOps.fsi b/src/Compiler/TypedTree/TypedTreeOps.fsi index 1710cbe7036..7e98ce5bc78 100755 --- a/src/Compiler/TypedTree/TypedTreeOps.fsi +++ b/src/Compiler/TypedTree/TypedTreeOps.fsi @@ -1829,7 +1829,7 @@ val TypeHasAllowNull: TyconRef -> TcGlobals -> range -> bool val TypeNullIsExtraValueNew: TcGlobals -> range -> TType -> bool -val IsSupportsNullTyparTy: TcGlobals -> TType -> Typar voption +val GetTyparTyIfSupportsNull: TcGlobals -> TType -> Typar voption val TypeNullNever: TcGlobals -> TType -> bool