Skip to content

Commit

Permalink
Nullness - option<> must be indicated as nullable in IL for C# consum…
Browse files Browse the repository at this point in the history
…ers (#17528)
  • Loading branch information
T-Gro authored Aug 19, 2024
1 parent d37a8ae commit d90885c
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 22 deletions.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.100.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Fix `function` implicit conversion. ([Issue #7401](https://github.com/dotnet/fsharp/issues/7401), [PR #17487](https://github.com/dotnet/fsharp/pull/17487))
* Compiler fails to recognise namespace in FQN with enabled GraphBasedChecking. ([Issue #17508](https://github.com/dotnet/fsharp/issues/17508), [PR #17510](https://github.com/dotnet/fsharp/pull/17510))
* Fix missing message for type error (FS0001). ([Issue #17373](https://github.com/dotnet/fsharp/issues/17373), [PR #17516](https://github.com/dotnet/fsharp/pull/17516))
* Nullness export - make sure option<> and other UseNullAsTrueValue types are properly annotated as nullable for C# and reflection consumers [PR #17528](https://github.com/dotnet/fsharp/pull/17528)
* MethodAccessException on equality comparison of a type private to module. ([Issue #17541](https://github.com/dotnet/fsharp/issues/17541), [PR #17548](https://github.com/dotnet/fsharp/pull/17548))

### Added
Expand Down
17 changes: 12 additions & 5 deletions src/Compiler/CodeGen/EraseUnions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ open FSharp.Compiler.IlxGenSupport
open System.Collections.Generic
open System.Reflection
open Internal.Utilities.Library
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeOps
open FSharp.Compiler.Features
open FSharp.Compiler.TcGlobals
Expand Down Expand Up @@ -955,7 +956,14 @@ let convAlternativeDef
&& g.langFeatureNullness
&& repr.RepresentAlternativeAsNull(info, alt)
then
GetNullableAttribute g [ FSharp.Compiler.TypedTree.NullnessInfo.WithNull ]
let noTypars = td.GenericParams.Length

GetNullableAttribute
g
[
yield NullnessInfo.WithNull // The top-level value itself, e.g. option, is nullable
yield! List.replicate noTypars NullnessInfo.AmbivalentToNull
] // The typars are not (i.e. do not change option<string> into option<string?>
|> Array.singleton
|> mkILCustomAttrsFromArray
else
Expand Down Expand Up @@ -1199,7 +1207,7 @@ let convAlternativeDef

let attrs =
if g.checkNullness && g.langFeatureNullness then
GetNullableContextAttribute g :: debugAttrs
GetNullableContextAttribute g 1uy :: debugAttrs
else
debugAttrs

Expand Down Expand Up @@ -1365,8 +1373,7 @@ let mkClassUnionDef
match nullableIdx with
| None ->
existingAttrs
|> Array.append
[| GetNullableAttribute g [ FSharp.Compiler.TypedTree.NullnessInfo.WithNull ] |]
|> Array.append [| GetNullableAttribute g [ NullnessInfo.WithNull ] |]
| Some idx ->
let replacementAttr =
match existingAttrs[idx] with
Expand Down Expand Up @@ -1619,7 +1626,7 @@ let mkClassUnionDef
customAttrs =
if cud.IsNullPermitted && g.checkNullness && g.langFeatureNullness then
td.CustomAttrs.AsArray()
|> Array.append [| GetNullableAttribute g [ FSharp.Compiler.TypedTree.NullnessInfo.WithNull ] |]
|> Array.append [| GetNullableAttribute g [ NullnessInfo.WithNull ] |]
|> mkILCustomAttrsFromArray
|> storeILCustomAttrs
else
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/CodeGen/IlxGen.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1912,7 +1912,7 @@ type TypeDefBuilder(tdef: ILTypeDef, tdefDiscards) =
if attrsBefore |> TryFindILAttribute g.attrib_AllowNullLiteralAttribute then
yield GetNullableAttribute g [ NullnessInfo.WithNull ]
if (gmethods.Count + gfields.Count + gproperties.Count) > 0 then
yield GetNullableContextAttribute g
yield GetNullableContextAttribute g 1uy
|]
|> mkILCustomAttrsFromArray
else
Expand Down
9 changes: 7 additions & 2 deletions src/Compiler/CodeGen/IlxGenSupport.fs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ let GetDynamicDependencyAttribute (g: TcGlobals) memberTypes (ilType: ILType) =
/// Nested items not being annotated with Nullable attribute themselves are interpreted as being withoutnull
/// Doing it that way is a heuristical decision supporting limited usage of (| null) annotations and not allowing nulls in >50% of F# code
/// (if majority of fields/parameters/return values would be nullable, this heuristic would lead to bloat of generated metadata)
let GetNullableContextAttribute (g: TcGlobals) =
let GetNullableContextAttribute (g: TcGlobals) flagValue =
let tref = g.attrib_NullableContextAttribute.TypeRef

g.TryEmbedILType(
Expand All @@ -329,7 +329,7 @@ let GetNullableContextAttribute (g: TcGlobals) =
mkLocalPrivateAttributeWithPropertyConstructors (g, tref.Name, fields, PublicFields))
)

mkILCustomAttribute (tref, [ g.ilg.typ_Byte ], [ ILAttribElem.Byte 1uy ], [])
mkILCustomAttribute (tref, [ g.ilg.typ_Byte ], [ ILAttribElem.Byte flagValue ], [])

let GetNotNullWhenTrueAttribute (g: TcGlobals) (propNames: string array) =
let tref = g.attrib_MemberNotNullWhenAttribute.TypeRef
Expand Down Expand Up @@ -407,6 +407,11 @@ let rec GetNullnessFromTType (g: TcGlobals) ty =
else if isValueType then
// Generic value type: 0, followed by the representation of the type arguments in order including containing types
yield NullnessInfo.AmbivalentToNull
else if
IsUnionTypeWithNullAsTrueValue g tcref.Deref
|| TypeHasAllowNull tcref g FSharp.Compiler.Text.Range.Zero
then
yield NullnessInfo.WithNull
else
// Reference type: the nullability (0, 1, or 2), followed by the representation of the type arguments in order including containing types
yield nullness.Evaluate()
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/CodeGen/IlxGenSupport.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ val GenAdditionalAttributesForTy: g: TcGlobals -> ty: TypedTree.TType -> ILAttri
val GetReadOnlyAttribute: g: TcGlobals -> ILAttribute
val GetIsUnmanagedAttribute: g: TcGlobals -> ILAttribute
val GetNullableAttribute: g: TcGlobals -> nullnessInfos: TypedTree.NullnessInfo list -> ILAttribute
val GetNullableContextAttribute: g: TcGlobals -> ILAttribute
val GetNullableContextAttribute: g: TcGlobals -> byte -> ILAttribute
val GetNotNullWhenTrueAttribute: g: TcGlobals -> string array -> ILAttribute
10 changes: 6 additions & 4 deletions src/Compiler/TypedTree/TypedTreeOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9198,16 +9198,18 @@ let reqTyForArgumentNullnessInference g actualTy reqTy =
changeWithNullReqTyToVariable g reqTy
| _ -> reqTy

let TypeHasAllowNull (tcref:TyconRef) g m =
not tcref.IsStructOrEnumTycon &&
not (isByrefLikeTyconRef g m tcref) &&
(TryFindTyconRefBoolAttribute g m g.attrib_AllowNullLiteralAttribute tcref = Some true)

/// 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 has AllowNullLiteral
(match tryTcrefOfAppTy g sty with
| ValueSome tcref ->
not tcref.IsStructOrEnumTycon &&
not (isByrefLikeTyconRef g m tcref) &&
(TryFindTyconRefBoolAttribute g m g.attrib_AllowNullLiteralAttribute tcref = Some true)
| ValueSome tcref -> TypeHasAllowNull tcref g m
| _ -> false)
||
// Check if the type has a nullness annotation
Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/TypedTree/TypedTreeOps.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -1809,6 +1809,8 @@ val TypeNullIsTrueValue: TcGlobals -> TType -> bool

val TypeNullIsExtraValue: TcGlobals -> range -> TType -> bool

val TypeHasAllowNull: TyconRef -> TcGlobals -> range -> bool

val TypeNullIsExtraValueNew: TcGlobals -> range -> TType -> bool

val TypeNullNever: TcGlobals -> TType -> bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags,
int32) = ( 01 00 08 00 00 00 00 00 00 00 00 00 )
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )

.maxstack 8
IL_0000: ldnull
Expand Down Expand Up @@ -188,7 +188,7 @@
.property class TestModule/MyNullableOption`1<!T>
MyNone()
{
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )
.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 )
.custom instance void [runtime]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [runtime]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 )
Expand Down Expand Up @@ -266,7 +266,7 @@
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags,
int32) = ( 01 00 08 00 00 00 00 00 00 00 00 00 )
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )

.maxstack 8
IL_0000: ldnull
Expand Down Expand Up @@ -382,7 +382,7 @@
.property class TestModule/MyOptionWhichCannotHaveNullInTheInside`1<!T>
MyNotNullNone()
{
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )
.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 )
.custom instance void [runtime]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [runtime]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 )
Expand Down Expand Up @@ -422,6 +422,10 @@
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.param type b
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )
.param [2]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )

.maxstack 4
.locals init (class TestModule/MyNullableOption`1<!!a> V_0,
Expand Down Expand Up @@ -458,6 +462,10 @@
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param type b
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param [0]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )
.param [2]
.custom instance void System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )

.maxstack 4
.locals init (class TestModule/MyOptionWhichCannotHaveNullInTheInside`1<!!a> V_0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags,
int32) = ( 01 00 08 00 00 00 00 00 00 00 00 00 )
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )

.maxstack 8
IL_0000: ldnull
Expand Down Expand Up @@ -188,7 +188,7 @@
.property class TestModule/MyNullableOption`1<!T>
MyNone()
{
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )
.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 )
.custom instance void [runtime]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [runtime]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 )
Expand Down Expand Up @@ -266,7 +266,7 @@
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags,
int32) = ( 01 00 08 00 00 00 00 00 00 00 00 00 )
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )

.maxstack 8
IL_0000: ldnull
Expand Down Expand Up @@ -382,7 +382,7 @@
.property class TestModule/MyOptionWhichCannotHaveNullInTheInside`1<!T>
MyNotNullNone()
{
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 00 00 00 )
.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 )
.custom instance void [runtime]System.Diagnostics.DebuggerBrowsableAttribute::.ctor(valuetype [runtime]System.Diagnostics.DebuggerBrowsableState) = ( 01 00 00 00 00 00 00 00 )
Expand Down Expand Up @@ -422,6 +422,10 @@
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.param type b
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 02 00 00 )
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )
.param [2]
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )

.maxstack 4
.locals init (class TestModule/MyNullableOption`1<!!a> V_0,
Expand Down Expand Up @@ -458,6 +462,10 @@
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param type b
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8) = ( 01 00 01 00 00 )
.param [0]
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )
.param [2]
.custom instance void [runtime]System.Runtime.CompilerServices.NullableAttribute::.ctor(uint8[]) = ( 01 00 02 00 00 00 02 01 00 00 )

.maxstack 4
.locals init (class TestModule/MyOptionWhichCannotHaveNullInTheInside`1<!!a> V_0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,39 @@ module Interop =
|> File.ReadAllText
|> fsharpLibCreator

[<Fact>]
let ``Csharp understands option like type using UseNullAsTrueValue`` () =
let csharpCode = """
using System;
using static TestModule;
using static Microsoft.FSharp.Core.FuncConvert;
#nullable enable
public class C {
// MyNullableOption has [<CompilationRepresentation(CompilationRepresentationFlags.UseNullAsTrueValue)>] applied on it
public void M(MyNullableOption<string> customOption) {
Console.WriteLine(customOption.ToString()); // should not warn
var thisIsNone = MyNullableOption<string>.MyNone;
Console.WriteLine(thisIsNone.ToString()); // !! should warn !!
var mapped = mapPossiblyNullable<string,string>(ToFSharpFunc<string,string>(x => x.ToString()), customOption); // should not warn, because null will not be passed
var mapped2 = mapPossiblyNullable<string,string>(ToFSharpFunc<string,string>(x => x + ".."), thisIsNone); // should NOT warn for passing in none, this is allowed
if(thisIsNone != null)
Console.WriteLine(thisIsNone.ToString()); // should NOT warn
if(customOption != null)
Console.WriteLine(customOption.ToString()); // should NOT warn
Console.WriteLine(MyOptionWhichCannotHaveNullInTheInside<string>.NewMyNotNullSome("").ToString()); // should NOT warn
}
}"""
csharpCode
|> csharpLibCompile (FsharpFromFile "NullAsTrueValue.fs")
|> withDiagnostics [ Warning 8602, Line 12, Col 27, Line 12, Col 37, "Dereference of a possibly null reference."]

[<Fact>]
let ``Csharp understands Fsharp-produced struct unions via IsXXX flow analysis`` () =
let csharpCode = """
Expand Down
Loading

0 comments on commit d90885c

Please sign in to comment.