From d5b274f7c13aba8f2a4c3c01418451f32ac21521 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 22 Apr 2024 11:19:49 +0200 Subject: [PATCH 1/9] Bugfix - matching aliased nullable should strip nullness Eliminating nullness after pattern matching null (that is , for subsequent patterns) must visit contents of abbreviations as well. Otherwise it does not work with the Maybe type whcih we use in the compiler. --- src/Compiler/Checking/CheckExpressions.fs | 4 +++- .../Language/NullableReferenceTypesTests.fs | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 328b3510c38..2c7c13da4b5 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -10531,7 +10531,9 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC let target = TTarget(vspecs, resultExpr, None) let inputTypeForNextPatterns= - let removeNull t = replaceNullnessOfTy KnownWithoutNull t + let removeNull t = + let stripped = stripTyEqns cenv.g t + replaceNullnessOfTy KnownWithoutNull stripped let rec isWild (p:Pattern) = match p with | TPat_wild _ -> true diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs index 7639c8ca208..bdb21a9dc42 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs @@ -185,6 +185,21 @@ let myFunction (input1 : string | null) (input2 : string | null): (string*string |> typeCheckWithStrictNullness |> shouldFail |> withErrorCode 3261 + +[] +let ``Eliminate aliased nullness after matching`` () = + FSharp $"""module MyLibrary + +type Maybe<'T> = 'T | null + +let myFunction (input : string Maybe) : string = + match input with + | null -> "" + | nonNullString -> nonNullString +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed [] let ``WithNull used on anon type`` () = From 3eed6b3bfb0f0f5c444eb6a3bae13d921f2817d7 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 22 Apr 2024 11:31:59 +0200 Subject: [PATCH 2/9] Making 'obj' work with new 'not null' constraints in fslib functions Bugfix: obj cannot be passed to generic typars which require T: not null, such as the NonNull active pattern. This commit fixes it. --- src/Compiler/TypedTree/TypedTreeOps.fs | 5 +---- .../Language/NullableReferenceTypesTests.fs | 21 ++++++++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/Compiler/TypedTree/TypedTreeOps.fs b/src/Compiler/TypedTree/TypedTreeOps.fs index 29d41b8e543..1a4ff99f8e5 100644 --- a/src/Compiler/TypedTree/TypedTreeOps.fs +++ b/src/Compiler/TypedTree/TypedTreeOps.fs @@ -9104,10 +9104,7 @@ let nullnessOfTy g ty = /// The new logic about whether a type admits the use of 'null' as a value. let TypeNullIsExtraValueNew g m ty = let sty = stripTyparEqns ty - - // Check if the type is 'obj' - isObjTy g sty - || + // Check if the type has AllowNullLiteral (match tryTcrefOfAppTy g sty with | ValueSome tcref -> diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs index bdb21a9dc42..687789dae7a 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs @@ -264,8 +264,7 @@ strictFunc(null:(string|null)) |> ignore |> typeCheckWithStrictNullness |> shouldFail |> withDiagnostics - [ Error 3261, Line 6, Col 12, Line 6, Col 20, "Nullness warning: The type 'obj' supports 'null' but a non-null type is expected." - Error 3261, Line 7, Col 18, Line 7, Col 26, "Nullness warning: The type 'obj' supports 'null' but a non-null type is expected." + [ Error 3261, Line 6, Col 12, Line 6, Col 16, "Nullness warning: The type 'obj' does not support 'null'." Error 3261, Line 7, Col 12, Line 7, Col 27, "Nullness warning: The type 'obj | null' supports 'null' but a non-null type is expected." Error 3261, Line 8, Col 12, Line 8, Col 30, "Nullness warning: The type 'string | null' supports 'null' but a non-null type is expected."] @@ -472,4 +471,20 @@ let mapped2 = |> withDiagnostics [ Error 3262, Line 6, Col 7, Line 6, Col 24, "Value known to be without null passed to a function meant for nullables: You can remove this |NonNullQuick| pattern usage." Error 3262, Line 10, Col 6, Line 10, Col 10, "Value known to be without null passed to a function meant for nullables: You can remove this |Null|NonNull| pattern usage." - Error 3262, Line 11, Col 6, Line 11, Col 15, "Value known to be without null passed to a function meant for nullables: You can remove this |Null|NonNull| pattern usage."] \ No newline at end of file + Error 3262, Line 11, Col 6, Line 11, Col 15, "Value known to be without null passed to a function meant for nullables: You can remove this |Null|NonNull| pattern usage."] + +[] +let ``Obj can be passed to not null constrained methods`` () = + FSharp """module MyLibrary + +let objVal:(obj | null) = box 42 + + +let mappableFunc = + match objVal with + |Null -> 42 + |NonNull o -> o.GetHashCode() +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed \ No newline at end of file From e222d106f8aee9deb56e34d158ee562ed0b3a899 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 22 Apr 2024 12:38:11 +0200 Subject: [PATCH 3/9] Bugfix - false 'useless null' warning in nested applications Error fixed: Error on useless null checkwith nullness constraint propagation in code like this:let meTry = Option.ofObj (Path.GetDirectoryName "")`. The warning about 'useless Option.ofObj' points to the string literal, ignoring the string literal is first passed to an API which may return null --- src/Compiler/Checking/CheckExpressions.fs | 6 ++++-- src/Compiler/Checking/ConstraintSolver.fs | 9 +++++++-- .../Language/NullableReferenceTypesTests.fs | 12 ++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/Compiler/Checking/CheckExpressions.fs b/src/Compiler/Checking/CheckExpressions.fs index 2c7c13da4b5..d1879119973 100644 --- a/src/Compiler/Checking/CheckExpressions.fs +++ b/src/Compiler/Checking/CheckExpressions.fs @@ -8462,8 +8462,10 @@ and TcApplicationThen (cenv: cenv) (overallTy: OverallTy) env tpenv mExprAndArg | ApplicableExpr(expr=Expr.Val (valRef=vref)) | ApplicableExpr(expr=Expr.App (funcExpr=Expr.Val (valRef=vref))) -> match TryFindLocalizedFSharpStringAttribute g g.attrib_WarnOnWithoutNullArgumentAttribute vref.Attribs with - | Some _ as msg -> env,{ cenv with css.WarnWhenUsingWithoutNullOnAWithNullTarget = msg} - | None -> env,cenv + | Some _ as msg -> env,{ cenv with css.WarnWhenUsingWithoutNullOnAWithNullTarget = msg} + | None when cenv.css.WarnWhenUsingWithoutNullOnAWithNullTarget <> None -> + env, { cenv with css.WarnWhenUsingWithoutNullOnAWithNullTarget = None} + | None -> env,cenv | _ -> env,cenv TcExprFlex2 cenv domainTy env false tpenv synArg, cenv diff --git a/src/Compiler/Checking/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index a5b123ac937..281e794e444 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -1021,6 +1021,11 @@ and SolveTypMeetsTyparConstraints (csenv: ConstraintSolverEnv) ndeep m2 trace ty SolveMemberConstraint csenv false PermitWeakResolution.No ndeep m2 trace traitInfo |> OperationResult.ignore } +and shouldWarnUselessNullCheck (csenv:ConstraintSolverEnv) = + csenv.g.checkNullness && + csenv.IsSpeculativeForMethodOverloading = false && + csenv.SolverState.WarnWhenUsingWithoutNullOnAWithNullTarget.IsSome + // nullness1: actual // nullness2: expected and SolveNullnessEquiv (csenv: ConstraintSolverEnv) m2 (trace: OptionalTrace) ty1 ty2 nullness1 nullness2 = @@ -1044,7 +1049,7 @@ and SolveNullnessEquiv (csenv: ConstraintSolverEnv) m2 (trace: OptionalTrace) ty | NullnessInfo.WithNull, NullnessInfo.WithNull -> CompleteD | NullnessInfo.WithoutNull, NullnessInfo.WithoutNull -> CompleteD // Warn for 'strict "must pass null"` APIs like Option.ofObj - | NullnessInfo.WithNull, NullnessInfo.WithoutNull when csenv.g.checkNullness && csenv.SolverState.WarnWhenUsingWithoutNullOnAWithNullTarget.IsSome -> + | NullnessInfo.WithNull, NullnessInfo.WithoutNull when shouldWarnUselessNullCheck csenv -> WarnD(Error(FSComp.SR.tcPassingWithoutNullToANullableExpectingFunc (csenv.SolverState.WarnWhenUsingWithoutNullOnAWithNullTarget.Value),m2)) // Allow expected of WithNull and actual of WithoutNull // TODO NULLNESS: this is not sound in contravariant cases etc. It is assuming covariance. @@ -1078,7 +1083,7 @@ and SolveNullnessSubsumesNullness (csenv: ConstraintSolverEnv) m2 (trace: Option | NullnessInfo.WithNull, NullnessInfo.WithNull -> CompleteD | NullnessInfo.WithoutNull, NullnessInfo.WithoutNull -> CompleteD // Warn for 'strict "must pass null"` APIs like Option.ofObj - | NullnessInfo.WithNull, NullnessInfo.WithoutNull when csenv.g.checkNullness && csenv.SolverState.WarnWhenUsingWithoutNullOnAWithNullTarget.IsSome -> + | NullnessInfo.WithNull, NullnessInfo.WithoutNull when shouldWarnUselessNullCheck csenv -> WarnD(Error(FSComp.SR.tcPassingWithoutNullToANullableExpectingFunc (csenv.SolverState.WarnWhenUsingWithoutNullOnAWithNullTarget.Value),m2)) // Allow target of WithNull and actual of WithoutNull | NullnessInfo.WithNull, NullnessInfo.WithoutNull -> diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs index 687789dae7a..e7984f50cfb 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs @@ -451,6 +451,18 @@ let mappedMaybe = nonNull maybeNull |> shouldFail |> withDiagnostics [Error 3262, Line 4, Col 25, Line 4, Col 39, "Value known to be without null passed to a function meant for nullables: You can remove this `nonNull` assertion."] +[] +let ``Regression: Useless usage in nested calls`` () = + FSharp """module MyLibrary +open System.IO + +let meTry = Option.ofObj (Path.GetDirectoryName "") +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + + [] let ``Useless usage of null active patterns from fscore`` () = FSharp """module MyLibrary From 6f22d8a1423d870d2bcc5832e4aafedda0f58cea Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 22 Apr 2024 14:51:03 +0200 Subject: [PATCH 4/9] Fix import for C# extension methods Bugfix for: C# extension methods which put "?" on the this argument are wrongly interpreted by moving the nullability elsewhere. See AsMemory from System.Memory.dll , this treats byteArray.ToMemory() as resulting in a Memory which is clearly wrong. Also, this now allows to call C# extension methods with ?this to be invoked on a nullable value. --- src/Compiler/Checking/ConstraintSolver.fs | 6 +++++- src/Compiler/Checking/import.fs | 7 +++++-- .../Language/NullableCsharpImportTests.fs | 20 +++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Checking/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index 281e794e444..7a708eb3f3a 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -2932,7 +2932,11 @@ and CanMemberSigsMatchUpToCheck ErrorD(Error (FSComp.SR.csMemberIsNotInstance(minfo.LogicalName), m)) else // The object types must be non-null - let nonNullCalledObjArgTys = calledObjArgTys |> List.map (replaceNullnessOfTy g.knownWithoutNull) + let nonNullCalledObjArgTys = + if not calledMeth.Method.IsExtensionMember then + calledObjArgTys |> List.map (replaceNullnessOfTy g.knownWithoutNull) + else + calledObjArgTys MapCombineTDC2D subsumeTypes nonNullCalledObjArgTys callerObjArgTys let! usesTDC3 = diff --git a/src/Compiler/Checking/import.fs b/src/Compiler/Checking/import.fs index b3935c5e46c..e2d5419e71e 100644 --- a/src/Compiler/Checking/import.fs +++ b/src/Compiler/Checking/import.fs @@ -186,7 +186,10 @@ module Nullness = | 2uy -> knownNullable | _ -> dprintfn "%i was passed to Nullness mapping, this is not a valid value" byteValue - knownAmbivalent + knownAmbivalent + + let isByte (g:TcGlobals) (ilgType:ILType) = + g.ilg.typ_Byte.BasicQualifiedName = ilgType.BasicQualifiedName let tryParseAttributeDataToNullableByteFlags (g:TcGlobals) attrData = match attrData with @@ -194,7 +197,7 @@ module Nullness = | Some ([ILAttribElem.Byte 0uy],_) -> ValueSome arrayWithByte0 | Some ([ILAttribElem.Byte 1uy],_) -> ValueSome arrayWithByte1 | Some ([ILAttribElem.Byte 2uy],_) -> ValueSome arrayWithByte2 - | Some ([ILAttribElem.Array(byteType,listOfBytes)],_) when byteType = g.ilg.typ_Byte -> + | Some ([ILAttribElem.Array(byteType,listOfBytes)],_) when isByte g byteType -> listOfBytes |> Array.ofList |> Array.choose(function | ILAttribElem.Byte b -> Some b | _ -> None) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs index 65e3a5c85bb..b1558d35c9a 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs @@ -34,6 +34,26 @@ let doSomethingAboutIt (ilg:ILGenerator) = |> typeCheckWithStrictNullness |> shouldSucceed +[] +let ``Consuming C# extension methods which allow nullable this`` () = + FSharp """module MyLibrary + +open System + +let asMemoryOnNonNull : Memory = + let bytes = [|0uy..11uy|] + let memory = bytes.AsMemory() + memory + +let asMemoryOnNull : Memory = + let bytes : (byte[]) | null = null + let memory = bytes.AsMemory() + memory +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + [] let ``Nullable directory info show warn on prop access`` () = FSharp """module MyLibrary From 35feeebaae37f855dcdaba7e1756465df971d5cf Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 22 Apr 2024 15:02:37 +0200 Subject: [PATCH 5/9] LinkedList First,Last bugfix There was a bug of LinkedList .First and .Last properties not returning nullable nodes. This was fixed by improved byte import in previous commit, adding a regression test for guarding this. --- .../Language/NullableCsharpImportTests.fs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs index b1558d35c9a..7c138db4e47 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableCsharpImportTests.fs @@ -54,6 +54,23 @@ let asMemoryOnNull : Memory = |> typeCheckWithStrictNullness |> shouldSucceed +[] +let ``Consuming LinkedList First and Last should warn about nullness`` () = + FSharp """module MyLibrary + +let ll = new System.Collections.Generic.LinkedList() +let x:System.Collections.Generic.LinkedListNode = ll.Last +let y:System.Collections.Generic.LinkedListNode = ll.First +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics + [ Error 3261, Line 4, Col 59, Line 4, Col 66, "Nullness warning: The types 'System.Collections.Generic.LinkedListNode' and 'System.Collections.Generic.LinkedListNode | null' do not have compatible nullability." + Error 3261, Line 4, Col 59, Line 4, Col 66, "Nullness warning: The types 'System.Collections.Generic.LinkedListNode' and 'System.Collections.Generic.LinkedListNode | null' do not have equivalent nullability." + Error 3261, Line 5, Col 59, Line 5, Col 67, "Nullness warning: The types 'System.Collections.Generic.LinkedListNode' and 'System.Collections.Generic.LinkedListNode | null' do not have compatible nullability." + Error 3261, Line 5, Col 59, Line 5, Col 67, "Nullness warning: The types 'System.Collections.Generic.LinkedListNode' and 'System.Collections.Generic.LinkedListNode | null' do not have equivalent nullability."] + [] let ``Nullable directory info show warn on prop access`` () = FSharp """module MyLibrary From 4b6a72524bf6197f38c7228de3111a053c3f902e Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 23 Apr 2024 11:11:17 +0200 Subject: [PATCH 6/9] Pipe null inference - failing test --- .../EmittedIL/Nullness/CustomPipe.fs | 3 + .../Nullness/CustomPipe.fs.il.net472.bsl | 123 ++++++++++++++++++ .../Nullness/CustomPipe.fs.il.netcore.bsl | 58 +++++++++ .../EmittedIL/Nullness/NullnessMetadata.fs | 5 + .../Language/NullableReferenceTypesTests.fs | 30 +++++ 5 files changed, 219 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl create mode 100644 tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs new file mode 100644 index 00000000000..73fe562b29f --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs @@ -0,0 +1,3 @@ +module MyTestModule + +let inline myCustomPipeFunc arg func = func arg \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl new file mode 100644 index 00000000000..7118dc61712 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl @@ -0,0 +1,123 @@ + + + + + +.assembly extern runtime { } +.assembly extern FSharp.Core { } +.assembly assembly +{ + .hash algorithm 0x00008004 + .ver 0:0:0:0 +} +.module assembly.dll + +.imagebase {value} +.file alignment 0x00000200 +.stackreserve 0x00100000 +.subsystem 0x0003 +.corflags 0x00000001 + + + + + +.class public abstract auto ansi sealed MyTestModule + extends [runtime]System.Object +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) + .custom instance void System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = ( 01 00 01 00 00 ) + .method public specialname static !!b op_PipeRight(!!a arg, + class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2 func) cil managed + { + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationArgumentCountsAttribute::.ctor(int32[]) = ( 01 00 02 00 00 00 01 00 00 00 01 00 00 00 00 00 ) + .param type a + .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 00 00 00 ) + .param type b + .custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.1 + IL_0001: ldarg.0 + IL_0002: tail. + IL_0004: callvirt instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2::Invoke(!0) + IL_0009: ret + } + +} + +.class private abstract auto ansi sealed ''.$MyTestModule + extends [runtime]System.Object +{ +} + +.class private auto ansi beforefieldinit System.Runtime.CompilerServices.NullableAttribute + extends [runtime]System.Attribute +{ + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .field public uint8[] NullableFlags + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [runtime]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) + .method public specialname rtspecialname instance void .ctor(uint8 scalarByteValue) cil managed + { + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [runtime]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [runtime]System.Attribute::.ctor() + IL_0006: ldarg.0 + IL_0007: ldc.i4.1 + IL_0008: newarr [runtime]System.Byte + IL_000d: dup + IL_000e: ldc.i4.0 + IL_000f: ldarg.1 + IL_0010: stelem.i1 + IL_0011: stfld uint8[] System.Runtime.CompilerServices.NullableAttribute::NullableFlags + IL_0016: ret + } + + .method public specialname rtspecialname instance void .ctor(uint8[] NullableFlags) cil managed + { + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [runtime]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [runtime]System.Attribute::.ctor() + IL_0006: ldarg.0 + IL_0007: ldarg.1 + IL_0008: stfld uint8[] System.Runtime.CompilerServices.NullableAttribute::NullableFlags + IL_000d: ret + } + +} + +.class private auto ansi beforefieldinit System.Runtime.CompilerServices.NullableContextAttribute + extends [runtime]System.Attribute +{ + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .field public uint8 Flag + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [runtime]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) + .method public specialname rtspecialname instance void .ctor(uint8 Flag) cil managed + { + .custom instance void [runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( 01 00 00 00 ) + .custom instance void [runtime]System.Diagnostics.DebuggerNonUserCodeAttribute::.ctor() = ( 01 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.0 + IL_0001: call instance void [runtime]System.Attribute::.ctor() + IL_0006: ldarg.0 + IL_0007: ldarg.1 + IL_0008: stfld uint8 System.Runtime.CompilerServices.NullableContextAttribute::Flag + IL_000d: ret + } + +} + + + + + + diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl new file mode 100644 index 00000000000..526371a1dcc --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl @@ -0,0 +1,58 @@ + + + + + +.assembly extern runtime { } +.assembly extern FSharp.Core { } +.assembly assembly +{ + .hash algorithm 0x00008004 + .ver 0:0:0:0 +} +.module assembly.dll + +.imagebase {value} +.file alignment 0x00000200 +.stackreserve 0x00100000 +.subsystem 0x0003 +.corflags 0x00000001 + + + + + +.class public abstract auto ansi sealed MyTestModule + extends [runtime]System.Object +{ + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) + .custom instance void [runtime]System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = ( 01 00 01 00 00 ) + .method public specialname static !!b op_PipeRight(!!a arg, + class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2 func) cil managed + { + .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationArgumentCountsAttribute::.ctor(int32[]) = ( 01 00 02 00 00 00 01 00 00 00 01 00 00 00 00 00 ) + .param type a + .custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 00 00 00 ) + .param type b + .custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 00 00 00 ) + + .maxstack 8 + IL_0000: ldarg.1 + IL_0001: ldarg.0 + IL_0002: tail. + IL_0004: callvirt instance !1 class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2::Invoke(!0) + IL_0009: ret + } + +} + +.class private abstract auto ansi sealed ''.$MyTestModule + extends [runtime]System.Object +{ +} + + + + + + diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/NullnessMetadata.fs b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/NullnessMetadata.fs index c7bc6a47755..46672e0208c 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/NullnessMetadata.fs +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/NullnessMetadata.fs @@ -79,6 +79,11 @@ let ``Nullable inheritance`` compilation = compilation |> verifyCompilation DoNotOptimize +[] +let ``Custom pipe`` compilation = + compilation + |> verifyCompilation DoNotOptimize + module Interop = open System.IO diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs index e7984f50cfb..78e9495ba17 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs @@ -422,6 +422,36 @@ let whatIsThis = Option.ofObj "abc123" |> shouldFail |> withErrorCodes [3262] +[] +let ``Option ofObj with inner nonnull expression`` () = + FSharp """module MyLibrary +open System.IO + +//let dirName = Path.GetDirectoryName "" + +//let whatIsThis1 = Option.ofObj dirName +//let whatIsThis2 = Option.ofObj ( Path.GetDirectoryName "" ) +let whatIsThis3 = Option.ofObj ("" |> Path.GetDirectoryName ) // Warnings were happening at this line only +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnostics [] + +[] +let ``Option ofObj with inner nullable expression`` () = + FSharp """module MyLibrary +open System.IO + +let nullSupportiveFunc (x: _ | null) = x + +let maybePath : string | null = null +let whatIsThis3 = Option.ofObj (maybePath |> nullSupportiveFunc) +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + [] let ``Useless null pattern match`` () = FSharp """module MyLibrary From 4de851f534ae8a5cc803faa3f64ec25d09086bd6 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 23 Apr 2024 16:37:58 +0200 Subject: [PATCH 7/9] IL base --- .../EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl | 4 ++-- .../EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl index 7118dc61712..f057362bee0 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.net472.bsl @@ -27,8 +27,8 @@ { .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) .custom instance void System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = ( 01 00 01 00 00 ) - .method public specialname static !!b op_PipeRight(!!a arg, - class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2 func) cil managed + .method public static !!b myassemblyFunc(!!a arg, + class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2 func) cil managed { .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationArgumentCountsAttribute::.ctor(int32[]) = ( 01 00 02 00 00 00 01 00 00 00 01 00 00 00 00 00 ) .param type a diff --git a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl index 526371a1dcc..cb9e545989f 100644 --- a/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl +++ b/tests/FSharp.Compiler.ComponentTests/EmittedIL/Nullness/CustomPipe.fs.il.netcore.bsl @@ -27,8 +27,8 @@ { .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 ) .custom instance void [runtime]System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = ( 01 00 01 00 00 ) - .method public specialname static !!b op_PipeRight(!!a arg, - class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2 func) cil managed + .method public static !!b myassemblyFunc(!!a arg, + class [FSharp.Core]Microsoft.FSharp.Core.FSharpFunc`2 func) cil managed { .custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationArgumentCountsAttribute::.ctor(int32[]) = ( 01 00 02 00 00 00 01 00 00 00 01 00 00 00 00 00 ) .param type a From a948c819180e6230d22b641a5ab34c8923b851f9 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 25 Apr 2024 14:37:36 +0200 Subject: [PATCH 8/9] Bugfix: Solve nullness for typars This fixes a bug where `not null` generic constraint was incorrectly passed between two typars:`T1 | null` with not null constraint on T1, and T2 without constraints. This occured when calling Option.ofObj(..) when the inner expression caused solving of generic type arguments, e.g. after (|>) or (id) function. This uses additional inference variable to unify them. --- src/Compiler/Checking/ConstraintSolver.fs | 14 ++++++++- .../Language/NullableReferenceTypesTests.fs | 29 ++++++++++++------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/Compiler/Checking/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index 7a708eb3f3a..88ee62bb71c 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -1232,6 +1232,12 @@ and SolveTypeEqualsType (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTr // Unifying 'T1? and 'T2? | ValueSome NullnessInfo.WithNull, ValueSome NullnessInfo.WithNull -> SolveTyparEqualsType csenv ndeep m2 trace sty1 (TType_var (tp2, g.knownWithoutNull)) + | ValueSome NullnessInfo.WithNull, ValueSome NullnessInfo.WithoutNull -> + let tpNew = NewCompGenTypar(TyparKind.Type, TyparRigidity.Flexible, TyparStaticReq.None, TyparDynamicReq.No, false) + trackErrors { + do! SolveTypeEqualsType csenv ndeep m2 trace cxsln sty2 (TType_var(tpNew, g.knownWithNull)) + do! SolveTypeEqualsType csenv ndeep m2 trace cxsln (TType_var(tpNew, g.knownWithoutNull)) sty1 + } //// Unifying 'T1 % and 'T2 % //| ValueSome NullnessInfo.AmbivalentToNull, ValueSome NullnessInfo.AmbivalentToNull -> // SolveTyparEqualsType csenv ndeep m2 trace sty1 (TType_var (tp2, g.knownWithoutNull)) @@ -1248,6 +1254,12 @@ and SolveTypeEqualsType (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTr // Unifying 'T1? and 'T2? | ValueSome NullnessInfo.WithNull, ValueSome NullnessInfo.WithNull -> SolveTyparEqualsType csenv ndeep m2 trace sty2 (TType_var (tp1, g.knownWithoutNull)) + | ValueSome NullnessInfo.WithNull, ValueSome NullnessInfo.WithoutNull -> + let tpNew = NewCompGenTypar(TyparKind.Type, TyparRigidity.Flexible, TyparStaticReq.None, TyparDynamicReq.No, false) + trackErrors { + do! SolveTypeEqualsType csenv ndeep m2 trace cxsln sty2 (TType_var(tpNew, g.knownWithNull)) + do! SolveTypeEqualsType csenv ndeep m2 trace cxsln (TType_var(tpNew, g.knownWithoutNull)) sty1 + } //// Unifying 'T1 % and 'T2 % //| ValueSome NullnessInfo.AmbivalentToNull, ValueSome NullnessInfo.AmbivalentToNull -> // SolveTyparEqualsType csenv ndeep m2 trace sty2 (TType_var (tp1, g.knownWithoutNull)) @@ -1264,7 +1276,7 @@ and SolveTypeEqualsType (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTr match nullness1.TryEvaluate(), (nullnessOfTy g sty2).TryEvaluate() with // Unifying 'T1? and 'T2? | ValueSome NullnessInfo.WithNull, ValueSome NullnessInfo.WithNull -> - SolveTyparEqualsType csenv ndeep m2 trace sty1 (replaceNullnessOfTy g.knownWithoutNull sty2) + SolveTyparEqualsType csenv ndeep m2 trace sty1 (replaceNullnessOfTy g.knownWithoutNull sty2) // Unifying 'T1 % and 'T2 % //| ValueSome NullnessInfo.AmbivalentToNull, ValueSome NullnessInfo.AmbivalentToNull -> // SolveTyparEqualsType csenv ndeep m2 trace sty1 (replaceNullnessOfTy g.knownWithoutNull sty2) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs index 78e9495ba17..f546c8e8d59 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/NullableReferenceTypesTests.fs @@ -423,28 +423,24 @@ let whatIsThis = Option.ofObj "abc123" |> withErrorCodes [3262] [] -let ``Option ofObj with inner nonnull expression`` () = +let ``Option ofObj for PathGetDirectoryName`` () = FSharp """module MyLibrary open System.IO -//let dirName = Path.GetDirectoryName "" - -//let whatIsThis1 = Option.ofObj dirName -//let whatIsThis2 = Option.ofObj ( Path.GetDirectoryName "" ) +let dirName = Path.GetDirectoryName "" +let whatIsThis1 = Option.ofObj dirName +let whatIsThis2 = Option.ofObj ( Path.GetDirectoryName "" ) let whatIsThis3 = Option.ofObj ("" |> Path.GetDirectoryName ) // Warnings were happening at this line only """ |> asLibrary |> typeCheckWithStrictNullness - |> shouldFail - |> withDiagnostics [] + |> shouldSucceed [] -let ``Option ofObj with inner nullable expression`` () = +let ``Option ofObj with fully annotated nullsupportive func`` () = FSharp """module MyLibrary -open System.IO - -let nullSupportiveFunc (x: _ | null) = x +let nullSupportiveFunc (x: string | null) : string | null = x let maybePath : string | null = null let whatIsThis3 = Option.ofObj (maybePath |> nullSupportiveFunc) """ @@ -452,6 +448,17 @@ let whatIsThis3 = Option.ofObj (maybePath |> nullSupportiveFunc) |> typeCheckWithStrictNullness |> shouldSucceed +[] +let ``Option ofObj with calling id inside`` () = + FSharp """module MyLibrary + +let maybePath : string | null = null +let whatIsThis5 = Option.ofObj (id maybePath) +""" + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldSucceed + [] let ``Useless null pattern match`` () = FSharp """module MyLibrary From f6e8e2e8fc16072ea0b14a9ffaa441001e1971ec Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 25 Apr 2024 14:42:15 +0200 Subject: [PATCH 9/9] remove trailing white space --- src/Compiler/Checking/ConstraintSolver.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Checking/ConstraintSolver.fs b/src/Compiler/Checking/ConstraintSolver.fs index 88ee62bb71c..37bb43a28cd 100644 --- a/src/Compiler/Checking/ConstraintSolver.fs +++ b/src/Compiler/Checking/ConstraintSolver.fs @@ -1276,7 +1276,7 @@ and SolveTypeEqualsType (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTr match nullness1.TryEvaluate(), (nullnessOfTy g sty2).TryEvaluate() with // Unifying 'T1? and 'T2? | ValueSome NullnessInfo.WithNull, ValueSome NullnessInfo.WithNull -> - SolveTyparEqualsType csenv ndeep m2 trace sty1 (replaceNullnessOfTy g.knownWithoutNull sty2) + SolveTyparEqualsType csenv ndeep m2 trace sty1 (replaceNullnessOfTy g.knownWithoutNull sty2) // Unifying 'T1 % and 'T2 % //| ValueSome NullnessInfo.AmbivalentToNull, ValueSome NullnessInfo.AmbivalentToNull -> // SolveTyparEqualsType csenv ndeep m2 trace sty1 (replaceNullnessOfTy g.knownWithoutNull sty2)