From f939050eb1b6cde83eb71780127d38810d3d28d6 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 21 Feb 2025 12:05:10 +0100 Subject: [PATCH] Bugfix :: Nullness :: Allow nullable return type for first branches of match and ifthenelse expressions (#18322) --- .../.FSharp.Compiler.Service/9.0.300.md | 1 + .../Checking/Expressions/CheckExpressions.fs | 2 + .../Expressions/CheckExpressionsOps.fs | 5 ++ src/Compiler/TypedTree/TypedTreeOps.fs | 13 ++-- src/Compiler/TypedTree/TypedTreeOps.fsi | 2 + .../Nullness/NullableReferenceTypesTests.fs | 77 +++++++++++++++++++ 6 files changed, 95 insertions(+), 5 deletions(-) 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)) 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..4b9de82d758 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, GetTyparTyIfSupportsNull 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..4cabe94d952 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 GetTyparTyIfSupportsNull 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 + (GetTyparTyIfSupportsNull 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 + (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 932e465b0e9..7e98ce5bc78 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 GetTyparTyIfSupportsNull: 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 018b81c9384..ad4788aa0ce 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -146,6 +146,83 @@ 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 -> "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 + |> shouldSucceed + +[] +let ``Can infer nullable type from first branch of ifthenelse`` () = + FSharp """module TestLib + +let myFunc 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