Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullness - option<> must be indicated as nullable in IL for C# consumers #17528

Merged
merged 13 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)

### 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
Loading