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

Feature nullness :: Bugfixes #17102

Merged
merged 20 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 3 additions & 1 deletion src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5264,6 +5264,7 @@ and TcExprFlex (cenv: cenv) flex compat (desiredTy: TType) (env: TcEnv) tpenv (s

if flex then
let argTy = NewInferenceType g
(destTyparTy g argTy).SetSupportsNullFlex(true)
if compat then
(destTyparTy g argTy).SetIsCompatFlex(true)

Expand Down Expand Up @@ -9522,7 +9523,8 @@ and TcEventItemThen (cenv: cenv) overallTy env tpenv mItem mExprAndItem objDetai
| None, false -> error (Error (FSComp.SR.tcEventIsNotStatic nm, mItem))
| _ -> ()

let delTy = einfo.GetDelegateType(cenv.amap, mItem)
// The F# wrappers around events are null safe (impl is in FSharp.Core). Therefore, from an F# perspective, the type of the delegate can be considered Not Null.
let delTy = einfo.GetDelegateType(cenv.amap, mItem) |> replaceNullnessOfTy KnownWithoutNull
let (SigOfFunctionForDelegate(delInvokeMeth, delArgTys, _, _)) = GetSigOfFunctionForDelegate cenv.infoReader delTy mItem ad
let objArgs = Option.toList (Option.map fst objDetails)
MethInfoChecks g cenv.amap true None objArgs env.eAccessRights mItem delInvokeMeth
Expand Down
19 changes: 18 additions & 1 deletion src/Compiler/Checking/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ type ConstraintSolverEnv =
// Is this speculative, with a trace allowing undo, and trial method overload resolution
IsSpeculativeForMethodOverloading: bool

// Can this ignore the 'must support null' constraint, e.g. in a mutable assignment scenario
IsSupportsNullFlex: bool

/// Indicates that when unifying ty1 = ty2, only type variables in ty1 may be solved. Constraints
/// can't be added to type variables in ty2
MatchingOnly: bool
Expand Down Expand Up @@ -344,6 +347,7 @@ let MakeConstraintSolverEnv contextInfo css m denv =
EquivEnv = TypeEquivEnv.Empty
DisplayEnv = denv
IsSpeculativeForMethodOverloading = false
IsSupportsNullFlex = false
ExtraRigidTypars = emptyFreeTypars
}

Expand Down Expand Up @@ -953,6 +957,13 @@ let rec SolveTyparEqualsTypePart1 (csenv: ConstraintSolverEnv) m2 (trace: Option
// Record the solution before we solve the constraints, since
// We may need to make use of the equation when solving the constraints.
// Record a entry in the undo trace if one is provided

//let ty1AllowsNull = r.Constraints |> List.exists (function | TyparConstraint.SupportsNull _ -> true | _ -> false )
//let tyAllowsNull() = TypeNullIsExtraValueNew csenv.g m2 ty
//if ty1AllowsNull && not (tyAllowsNull()) then
// trace.Exec (fun () -> r.typar_solution <- Some (ty |> replaceNullnessOfTy csenv.g.knownWithNull)) (fun () -> r.typar_solution <- None)
//else
// trace.Exec (fun () -> r.typar_solution <- Some ty) (fun () -> r.typar_solution <- None)
trace.Exec (fun () -> r.typar_solution <- Some ty) (fun () -> r.typar_solution <- None)
}

Expand Down Expand Up @@ -1006,7 +1017,8 @@ and SolveTypMeetsTyparConstraints (csenv: ConstraintSolverEnv) ndeep m2 trace ty
AddConstraint csenv ndeep m2 trace destTypar (TyparConstraint.DefaultsTo(priority, dty, m))

| TyparConstraint.NotSupportsNull m2 -> SolveTypeUseNotSupportsNull csenv ndeep m2 trace ty
| TyparConstraint.SupportsNull m2 -> SolveTypeUseSupportsNull csenv ndeep m2 trace ty
| TyparConstraint.SupportsNull _ when csenv.IsSupportsNullFlex -> CompleteD
| TyparConstraint.SupportsNull m2 -> SolveTypeUseSupportsNull csenv ndeep m2 trace ty
| TyparConstraint.IsEnum(underlyingTy, m2) -> SolveTypeIsEnum csenv ndeep m2 trace ty underlyingTy
| TyparConstraint.SupportsComparison(m2) -> SolveTypeSupportsComparison csenv ndeep m2 trace ty
| TyparConstraint.SupportsEquality(m2) -> SolveTypeSupportsEquality csenv ndeep m2 trace ty
Expand Down Expand Up @@ -1222,6 +1234,11 @@ and SolveTypeEqualsType (csenv: ConstraintSolverEnv) ndeep m2 (trace: OptionalTr
let sty1 = stripTyEqnsA csenv.g canShortcut ty1
let sty2 = stripTyEqnsA csenv.g canShortcut ty2

let csenv =
match ty1 with
T-Gro marked this conversation as resolved.
Show resolved Hide resolved
| TType.TType_var(r,_) when r.typar_flags.IsSupportsNullFlex -> { csenv with IsSupportsNullFlex = true}
| _ -> csenv

match sty1, sty2 with
// type vars inside forall-types may be alpha-equivalent
| TType_var (tp1, nullness1), TType_var (tp2, nullness2) when typarEq tp1 tp2 || (match aenv.EquivTypars.TryFind tp1 with | Some tpTy1 when typeEquiv g tpTy1 ty2 -> true | _ -> false) ->
Expand Down
36 changes: 27 additions & 9 deletions src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ type TyparRigidity =
[<Struct>]
type TyparFlags(flags: int32) =

new (kind: TyparKind, rigidity: TyparRigidity, isFromError: bool, isCompGen: bool, staticReq: TyparStaticReq, dynamicReq: TyparDynamicReq, equalityDependsOn: bool, comparisonDependsOn: bool) =
new (kind: TyparKind, rigidity: TyparRigidity, isFromError: bool, isCompGen: bool, staticReq: TyparStaticReq, dynamicReq: TyparDynamicReq, equalityDependsOn: bool, comparisonDependsOn: bool, supportsNullFlex: bool) =
TyparFlags((if isFromError then 0b00000000000000010 else 0) |||
(if isCompGen then 0b00000000000000100 else 0) |||
(match staticReq with
Expand All @@ -321,7 +321,11 @@ type TyparFlags(flags: int32) =
| TyparDynamicReq.No -> 0b00000000000000000
| TyparDynamicReq.Yes -> 0b00000010000000000) |||
(if equalityDependsOn then
0b00000100000000000 else 0))
0b00000100000000000 else 0) |||
// 0b00001000100000000 is being checked by x.Kind, but never set in this version of the code
// 0b00010000000000000 is taken by compat flex
(if supportsNullFlex then
0b00100000000000000 else 0))

/// Indicates if the type inference variable was generated after an error when type checking expressions or patterns
member x.IsFromError = (flags &&& 0b00000000000000010) <> 0x0
Expand Down Expand Up @@ -380,8 +384,20 @@ type TyparFlags(flags: int32) =
else
TyparFlags(flags &&& ~~~0b00010000000000000)

/// Indicates that whether this type parameter is flexible for 'supports null' constraint, e.g. in the case of assignment to a mutable value
member x.IsSupportsNullFlex =
(flags &&& 0b00100000000000000) <> 0x0

member x.WithSupportsNullFlex b =
if b then
TyparFlags(flags ||| 0b00100000000000000)
else
TyparFlags(flags &&& ~~~0b00100000000000000)



member x.WithStaticReq staticReq =
TyparFlags(x.Kind, x.Rigidity, x.IsFromError, x.IsCompilerGenerated, staticReq, x.DynamicReq, x.EqualityConditionalOn, x.ComparisonConditionalOn)
TyparFlags(x.Kind, x.Rigidity, x.IsFromError, x.IsCompilerGenerated, staticReq, x.DynamicReq, x.EqualityConditionalOn, x.ComparisonConditionalOn, x.IsSupportsNullFlex)

/// Get the flags as included in the F# binary metadata. We pickle this as int64 to allow for future expansion
member x.PickledBits = flags
Expand Down Expand Up @@ -2320,6 +2336,8 @@ type Typar =
/// Set whether this type parameter is a compat-flex type parameter (i.e. where "expr :> tp" only emits an optional warning)
member x.SetIsCompatFlex b = x.typar_flags <- x.typar_flags.WithCompatFlex b

member x.SetSupportsNullFlex b = x.typar_flags <- x.typar_flags.WithSupportsNullFlex b

/// Indicates whether a type variable can be instantiated by types or units-of-measure.
member x.Kind = x.typar_flags.Kind

Expand Down Expand Up @@ -2424,12 +2442,12 @@ type Typar =
/// Sets the rigidity of a type variable
member x.SetRigidity b =
let flags = x.typar_flags
x.typar_flags <- TyparFlags(flags.Kind, b, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, flags.DynamicReq, flags.EqualityConditionalOn, flags.ComparisonConditionalOn)
x.typar_flags <- TyparFlags(flags.Kind, b, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, flags.DynamicReq, flags.EqualityConditionalOn, flags.ComparisonConditionalOn, flags.IsSupportsNullFlex)

/// Sets whether a type variable is compiler generated
member x.SetCompilerGenerated b =
let flags = x.typar_flags
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, b, flags.StaticReq, flags.DynamicReq, flags.EqualityConditionalOn, flags.ComparisonConditionalOn)
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, b, flags.StaticReq, flags.DynamicReq, flags.EqualityConditionalOn, flags.ComparisonConditionalOn, flags.IsSupportsNullFlex)

/// Sets whether a type variable has a static requirement
member x.SetStaticReq b =
Expand All @@ -2438,17 +2456,17 @@ type Typar =
/// Sets whether a type variable is required at runtime
member x.SetDynamicReq b =
let flags = x.typar_flags
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, b, flags.EqualityConditionalOn, flags.ComparisonConditionalOn)
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, b, flags.EqualityConditionalOn, flags.ComparisonConditionalOn, flags.IsSupportsNullFlex)

/// Sets whether the equality constraint of a type definition depends on this type variable
member x.SetEqualityDependsOn b =
let flags = x.typar_flags
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, flags.DynamicReq, b, flags.ComparisonConditionalOn)
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, flags.DynamicReq, b, flags.ComparisonConditionalOn, flags.IsSupportsNullFlex)

/// Sets whether the comparison constraint of a type definition depends on this type variable
member x.SetComparisonDependsOn b =
let flags = x.typar_flags
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, flags.DynamicReq, flags.EqualityConditionalOn, b)
x.typar_flags <- TyparFlags(flags.Kind, flags.Rigidity, flags.IsFromError, flags.IsCompilerGenerated, flags.StaticReq, flags.DynamicReq, flags.EqualityConditionalOn, b, flags.IsSupportsNullFlex)

[<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
member x.DebugText = x.ToString()
Expand Down Expand Up @@ -6117,7 +6135,7 @@ type Construct() =
Typar.New
{ typar_id = id
typar_stamp = newStamp()
typar_flags= TyparFlags(kind, rigid, isFromError, isCompGen, staticReq, dynamicReq, eqDep, compDep)
typar_flags= TyparFlags(kind, rigid, isFromError, isCompGen, staticReq, dynamicReq, eqDep, compDep, false)
typar_solution = None
typar_astype = Unchecked.defaultof<_>
typar_opt_data =
Expand Down
9 changes: 8 additions & 1 deletion src/Compiler/TypedTree/TypedTree.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ type TyparFlags =
staticReq: Syntax.TyparStaticReq *
dynamicReq: TyparDynamicReq *
equalityDependsOn: bool *
comparisonDependsOn: bool ->
comparisonDependsOn: bool *
supportsNullFlex: bool ->
TyparFlags

new: flags: int32 -> TyparFlags
Expand All @@ -232,6 +233,9 @@ type TyparFlags =
/// Indicates if the type inference variable was generated after an error when type checking expressions or patterns
member IsFromError: bool

/// Indicates whether this type parameter is flexible for 'supports null' constraint, e.g. in the case of assignment to a mutable value
member IsSupportsNullFlex: bool

/// Indicates whether a type variable can be instantiated by types or units-of-measure.
member Kind: TyparKind

Expand Down Expand Up @@ -1552,6 +1556,9 @@ type Typar =
/// Set whether this type parameter is a compat-flex type parameter (i.e. where "expr :> tp" only emits an optional warning)
member SetIsCompatFlex: b: bool -> unit

/// Set whether this type parameter is flexible for 'supports null' constraint, e.g. in the case of assignment to a mutable value
member SetSupportsNullFlex: b:bool -> unit

/// Sets the rigidity of a type variable
member SetRigidity: b: TyparRigidity -> unit

Expand Down
7 changes: 6 additions & 1 deletion src/Compiler/TypedTree/TypedTreeBasics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,12 @@ let addNullnessToTy (nullness: Nullness) (ty:TType) =
| _ ->
match ty with
| TType_var (tp, nullnessOrig) -> TType_var (tp, combineNullness nullnessOrig nullness)
| TType_app (tcr, tinst, nullnessOrig) -> TType_app (tcr, tinst, combineNullness nullnessOrig nullness)
| TType_app (tcr, tinst, nullnessOrig) ->
let tycon = tcr.Deref
if tycon.IsStructRecordOrUnionTycon || tycon.IsStructOrEnumTycon then
ty
else
TType_app (tcr, tinst, combineNullness nullnessOrig nullness)
| TType_fun (d, r, nullnessOrig) -> TType_fun (d, r, combineNullness nullnessOrig nullness)
//| TType_ucase _ -> None // TODO NULLNESS
//| TType_tuple _ -> None // TODO NULLNESS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ let typeCheckWithStrictNullness cu =
|> withOptions ["--warnaserror+"]
|> compile


[<FactForNETCOREAPP>]
let ``Passing null to IlGenerator BeginCatchBlock is fine`` () =
FSharp """module MyLibrary
Expand All @@ -34,6 +35,42 @@ let doSomethingAboutIt (ilg:ILGenerator) =
|> typeCheckWithStrictNullness
|> shouldSucceed

[<FactForNETCOREAPP>]
let ``Consuming C# generic API which allows struct and yet uses question mark on the typar`` () =
FSharp """module MyLibrary
let ec =
{ new System.Collections.Generic.IEqualityComparer<int> with
member this.Equals(x, y) = (x+0) = (y+0)
member this.GetHashCode(obj) = obj * 2}
"""
|> asLibrary
|> typeCheckWithStrictNullness
|> shouldSucceed

[<FactForNETCOREAPP>]
let ``TypeBuilder CreateTypeInfo with an upcast`` () =
FSharp """module MyLibrary
open System
open System.Reflection.Emit

let createType (typB:TypeBuilder) : Type=
typB.CreateTypeInfo() :> Type
"""
|> asLibrary
|> typeCheckWithStrictNullness
|> shouldSucceed

[<FactForNETCOREAPP>]
let ``CurrentDomain ProcessExit add to event`` () =
FSharp """module MyLibrary
open System

do System.AppDomain.CurrentDomain.ProcessExit |> Event.add (fun args -> failwith $"{args.GetHashCode()}")
"""
|> asLibrary
|> typeCheckWithStrictNullness
|> shouldSucceed

[<FactForNETCOREAPP>]
let ``Consuming C# extension methods which allow nullable this`` () =
FSharp """module MyLibrary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,37 @@ let nonStrictFunc(x:string | null) = strictFunc(x)
|> withDiagnostics [
Error 3261, Line 4, Col 49, Line 4, Col 50, "Nullness warning: The types 'string' and 'string | null' do not have equivalent nullability."]

[<Fact>]
let ``Mutable binding initially assigned to null should not need type annotation``() =
FSharp """
module MyLib
open System.Collections.Concurrent
open System

let mkCacheInt32 () =
let mutable cache = null

fun f (idx: int32) ->
let cache =
match cache with
| null ->
let v = ConcurrentDictionary<int32, _>(Environment.ProcessorCount, 11)
cache <- v
v
| v -> v

match cache.TryGetValue idx with
| true, res -> res
| _ ->
let res = f idx
cache[idx] <- res
res

"""
|> asLibrary
|> typeCheckWithStrictNullness
|> shouldSucceed

[<Fact>]
let ``Boolean literal to string is not nullable`` () =
FSharp """module MyLibrary
Expand Down
Loading