From 10b812b91e05da57c264660b1fe45a18cba3a38a Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 4 Feb 2025 14:03:01 +0100 Subject: [PATCH] Bugfix :: Flexible types should subsume nullable version of equivalent CoarcesTo constraints (#18266) --- .../.FSharp.Compiler.Service/9.0.300.md | 1 + src/Compiler/Checking/ConstraintSolver.fs | 18 ++++++++- .../Nullness/NullableReferenceTypesTests.fs | 40 ++++++++++++++++++- .../Signatures/SeqTests.fs | 16 ++++++++ 4 files changed, 73 insertions(+), 2 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 683c8dc0f5c..de19496f010 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md @@ -1,6 +1,7 @@ ### Fixed * 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)) * 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/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index 82141080743..902bf331d85 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -1609,7 +1609,10 @@ and SolveTyparSubtypeOfType (csenv: ConstraintSolverEnv) ndeep m2 trace tp ty1 = elif isSealedTy g ty1 then SolveTypeEqualsTypeKeepAbbrevs csenv ndeep m2 trace (mkTyparTy tp) ty1 else - AddConstraint csenv ndeep m2 trace tp (TyparConstraint.CoercesTo(ty1, csenv.m)) + if SubtypeConstraintImplied g tp.Constraints ty1 then + CompleteD + else + AddConstraint csenv ndeep m2 trace tp (TyparConstraint.CoercesTo(ty1, csenv.m)) and DepthCheck ndeep m = if ndeep > 300 then @@ -2490,6 +2493,19 @@ and CheckConstraintImplication (csenv: ConstraintSolverEnv) tpc1 tpc2 = and CheckConstraintsImplication csenv existingConstraints newConstraint = existingConstraints |> List.exists (fun tpc2 -> CheckConstraintImplication csenv tpc2 newConstraint) +and SubtypeConstraintImplied g existingConstraints newCoarceToTy = + if g.checkNullness then + let canBeNull t = (nullnessOfTy g t).Evaluate() = NullnessInfo.WithNull + let newTyIsWithoutNull = canBeNull newCoarceToTy |> not + let typeCoversNewConstraint existingTy = + typeEquiv g existingTy newCoarceToTy + && not (newTyIsWithoutNull && canBeNull existingTy) // :> T? cannot imply :>T, since non-nullable is a stricter constraint. + + existingConstraints + |> List.exists (function | TyparConstraint.CoercesTo(ty2,_) when typeCoversNewConstraint ty2 -> true | _ -> false) + else + false + // Ensure constraint conforms with existing constraints // NOTE: QUADRATIC and EnforceConstraintSetConsistency csenv ndeep m2 trace retry allCxs i cxs = diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index 7dbe3527fbf..85b872d01a2 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -1003,7 +1003,45 @@ looseFunc(maybeTuple2) |> ignore Error 3261, Line 29, Col 12, Line 29, Col 19, "Nullness warning: The type 'MyDu | null' supports 'null' but a non-null type is expected." Error 3261, Line 30, Col 12, Line 30, Col 21, "Nullness warning: The type 'MyRecord | null' supports 'null' but a non-null type is expected." Error 43, Line 40, Col 36, Line 40, Col 40, "The type 'Maybe' does not have 'null' as a proper value"] - + + +[] +let ``Nullness support for flexible types`` () = + FSharp """module MyLibrary +open System +let dispose (x: IDisposable | null) : unit = + match x with + | null -> () + | d -> d.Dispose() + +let useThing (thing: #IDisposable) = + try + printfn "%O" thing + finally + dispose thing // Warning used to be here, should not warn! """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + +[] +let ``Nullness support for flexible types - opposite`` () = + FSharp """module MyLibrary +open System +let dispose (x: IDisposable) : unit = + x.Dispose() + +let useThing (thing: #IDisposable | null) = + try + printfn "%O" thing + finally + dispose thing // Warning should be here, because 'thing' can be null! + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics [Error 3261, Line 10, Col 17, Line 10, Col 22, "Nullness warning: The types 'IDisposable' and ''a | null' do not have compatible nullability."] + + [] let ``Static member on Record with null arg`` () = FSharp """module MyLibrary diff --git a/tests/FSharp.Compiler.ComponentTests/Signatures/SeqTests.fs b/tests/FSharp.Compiler.ComponentTests/Signatures/SeqTests.fs index 10a0d0b3a25..fdca6035783 100644 --- a/tests/FSharp.Compiler.ComponentTests/Signatures/SeqTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Signatures/SeqTests.fs @@ -2,6 +2,8 @@ open Xunit open Signatures.TestHelpers +open FSharp.Test +open FSharp.Test.Compiler [] let ``int seq`` () = @@ -14,3 +16,17 @@ let ``tuple seq`` () = assertSingleSignatureBinding "let s = seq { yield (1, 'b', 2.) }" "val s: (int * char * float) seq" + +[] +let ``seq transpose`` () = + let encodeFs = + FsSource """module Program +let transpose (source: seq<#seq<'T>>) = + source |> Seq.collect Seq.indexed |> Seq.groupBy fst |> Seq.map (snd >> (Seq.map snd))""" + + Fsi """module Program +val transpose: source: seq<'Collection> -> seq> when 'Collection :> seq<'T>""" + |> withAdditionalSourceFile encodeFs + |> withLangVersionPreview + |> compile + |> shouldSucceed