From 434a56868fee74d9f4169a198ef3ae98214593a1 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 23 Jan 2025 14:40:58 +0100 Subject: [PATCH 1/3] Warn when upcast drops nullness via FindUniqueFeasibleSupertype --- src/Compiler/Checking/TypeRelations.fs | 12 ++++++++++-- .../Nullness/NullableReferenceTypesTests.fs | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Checking/TypeRelations.fs b/src/Compiler/Checking/TypeRelations.fs index d180bb778dd..2cb5dd4057a 100644 --- a/src/Compiler/Checking/TypeRelations.fs +++ b/src/Compiler/Checking/TypeRelations.fs @@ -341,5 +341,13 @@ let IteratedAdjustLambdaToMatchValReprInfo g amap valReprInfo lambdaExpr = /// "Single Feasible Type" inference /// Look for the unique supertype of ty2 for which ty2 :> ty1 might feasibly hold let FindUniqueFeasibleSupertype g amap m ty1 ty2 = - let supertypes = Option.toList (GetSuperTypeOfType g amap m ty2) @ (GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m ty2) - supertypes |> List.tryFind (TypeFeasiblySubsumesType 0 g amap m ty1 NoCoerce) + let n2 = nullnessOfTy g ty2 + let nullify t = addNullnessToTy n2 t + + let supertypes = + Option.toList (GetSuperTypeOfType g amap m ty2) @ + (GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m ty2) + + supertypes + |> List.tryFind (TypeFeasiblySubsumesType 0 g amap m ty1 NoCoerce) + |> Option.map nullify diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index 819bd475152..e56b5a55869 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -17,6 +17,20 @@ let typeCheckWithStrictNullness cu = |> withNullnessOptions |> typecheck +[] +let ``Warning on nullness hidden behind interface upcast`` () = + FSharp """module Test + +open System.IO +open System + +// This is bad - input is nullable, output is not = must warn +let whatisThis (s:Stream|null) : IDisposable = + s""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics [Error 3261, Line 8, Col 5, Line 8, Col 6, "Nullness warning: The types 'IDisposable' and 'IDisposable | null' do not have compatible nullability."] [] let ``Report warning when applying anon record to a nullable generic return value`` () = From 278e3b75a25832ddd353921103397209ba5ccc2e Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 3 Feb 2025 12:35:55 +0100 Subject: [PATCH 2/3] temporary null shutuops before `use` is softened --- src/Compiler/Utilities/Activity.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Utilities/Activity.fs b/src/Compiler/Utilities/Activity.fs index b6fafe1c1b9..2403f00d3de 100644 --- a/src/Compiler/Utilities/Activity.fs +++ b/src/Compiler/Utilities/Activity.fs @@ -93,14 +93,14 @@ module internal Activity = let activity = activitySource.CreateActivity(name, ActivityKind.Internal) match activity with - | null -> activity + | null -> !!activity //TODO change retTy to |null after PR #18262 is merged!! | activity -> for key, value in tags do activity.AddTag(key, value) |> ignore activity.Start() - let startNoTags (name: string) : IDisposable = activitySource.StartActivity name + let startNoTags (name: string) : IDisposable = !! (activitySource.StartActivity name) //TODO change retTy to |null after PR #18262 is merged!! let addEvent name = match Activity.Current with @@ -122,7 +122,7 @@ module internal Activity = let private profiledSource = new ActivitySource(ActivityNames.ProfiledSourceName) - let startAndMeasureEnvironmentStats (name: string) : IDisposable = profiledSource.StartActivity(name) + let startAndMeasureEnvironmentStats (name: string) : IDisposable = !!(profiledSource.StartActivity(name)) //TODO change retTy to |null after PR #18262 is merged!! type private GCStats = int[] From f945bea2a5a49c197470d8b823829c5552ec1855 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 3 Feb 2025 14:17:08 +0100 Subject: [PATCH 3/3] notes added --- 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 52b92207001..4224d7421fb 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md @@ -2,6 +2,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 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)) ### Added * Added missing type constraints in FCS. ([PR #18241](https://github.com/dotnet/fsharp/pull/18241))