Skip to content

Commit

Permalink
[RFC FS-1060] Nullness checking (applied to codebase) (#15310)
Browse files Browse the repository at this point in the history
- apply nullness to codebase
- build F.C.S also againts net9 to get best annotations (multi targetting)
- bootstrap FSharp.Build in proto build
  • Loading branch information
dsyme authored Aug 14, 2024
1 parent 14f4369 commit ccd0de1
Show file tree
Hide file tree
Showing 100 changed files with 910 additions and 650 deletions.
14 changes: 13 additions & 1 deletion .fantomasignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ vsintegration/*
!vsintegration/tests/FSharp.Editor.Tests
artifacts/

# For some reason, it tries to format files from remotes (Processing .\.git\refs\remotes\<remote>\FSComp.fsi)
# For some reason, it tries to format files from remotes (Processing ./.git/refs/remotes/<remote>/FSComp.fsi)
.git/

# Explicitly unformatted implementation
Expand Down Expand Up @@ -101,10 +101,22 @@ src/FSharp.Core/option.fsi
src/FSharp.Core/option.fs
src/fsi/console.fs
src/FSharp.Build/FSharpCommandLineBuilder.fs

src/Compiler/Utilities/Activity.fs
src/Compiler/Utilities/sformat.fs
src/Compiler/Utilities/illib.fsi
src/Compiler/Utilities/illib.fs


src/Compiler/Utilities/NullnessShims.fs
src/Compiler/Utilities/LruCache.fsi
src/Compiler/Utilities/LruCache.fs
src/Compiler/Utilities/HashMultiMap.fsi
src/Compiler/Utilities/HashMultiMap.fs
src/Compiler/Facilities/AsyncMemoize.fsi
src/Compiler/Facilities/AsyncMemoize.fs
src/Compiler/AbstractIL/il.fs

# Fantomas limitations on implementation files (to investigate)

src/Compiler/AbstractIL/ilwrite.fs
Expand Down
8 changes: 8 additions & 0 deletions FSharpBuild.Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@
</ItemGroup>
</Target>

<!-- SDK targets override -->
<PropertyGroup Condition="'$(Configuration)' != 'Proto' AND '$(DisableCompilerRedirection)'!='true' AND Exists('$(ProtoOutputPath)')">
<FSharpBuildAssemblyFileOverride>$(ProtoOutputPath)\fsc\FSharp.Build.dll</FSharpBuildAssemblyFileOverride>
</PropertyGroup>
<UsingTask TaskName="FSharpEmbedResourceText" AssemblyFile="$(FSharpBuildAssemblyFileOverride)" Override="true" Condition="'$(Configuration)' != 'Proto' AND '$(DisableCompilerRedirection)'!='true' AND Exists('$(ProtoOutputPath)')" />
<UsingTask TaskName="FSharpEmbedResXSource" AssemblyFile="$(FSharpBuildAssemblyFileOverride)" Override="true" Condition="'$(Configuration)' != 'Proto' AND '$(DisableCompilerRedirection)'!='true' AND Exists('$(ProtoOutputPath)')" />


<Target Name="BeforeResGen"
Inputs="@(EmbeddedResource->'$(IntermediateOutputPath)%(Filename)%(Extension)')"
Outputs="@(EmbeddedResource->'$(IntermediateOutputPath)resources\%(Filename)%(Extension)')"
Expand Down
3 changes: 3 additions & 0 deletions azure-pipelines-PR.yml
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,8 @@ stages:
- checkout: self
clean: true
- script: ./eng/cibuild.sh --configuration $(_BuildConfig) --testcoreclr
env:
SKIP_NETCURRENT_FSC_BUILD: true
displayName: Build / Test
- task: PublishTestResults@2
displayName: Publish Test Results
Expand Down Expand Up @@ -595,6 +597,7 @@ stages:
- script: ./eng/cibuild.sh --configuration $(_BuildConfig) --testcoreclr
env:
COMPlus_DefaultStackSize: 1000000
SKIP_NETCURRENT_FSC_BUILD: true
displayName: Build / Test
- task: PublishTestResults@2
displayName: Publish Test Results
Expand Down
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 @@ -25,6 +25,7 @@
* Treat `{ new Foo() }` as `SynExpr.ObjExpr` ([PR #17388](https://github.com/dotnet/fsharp/pull/17388))
* Optimize metadata reading for type members and custom attributes. ([PR #17364](https://github.com/dotnet/fsharp/pull/17364))
* Enforce `AttributeTargets` on unions. ([PR #17389](https://github.com/dotnet/fsharp/pull/17389))
* Applied nullable reference types to FSharp.Compiler.Service itself ([PR #15310](https://github.com/dotnet/fsharp/pull/15310))
* Ensure that isinteractive multi-emit backing fields are not public. ([Issue #17439](https://github.com/dotnet/fsharp/issues/17438)), ([PR #17439](https://github.com/dotnet/fsharp/pull/17439))
* Enable FSharp 9.0 Language Version ([Issue #17497](https://github.com/dotnet/fsharp/issues/17438)), [PR](https://github.com/dotnet/fsharp/pull/17500)))
* Better error reporting for unions with duplicated fields. ([PR #17521](https://github.com/dotnet/fsharp/pull/17521))
Expand Down
26 changes: 16 additions & 10 deletions src/Compiler/AbstractIL/il.fs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,11 @@ let splitTypeNameRight nm =
// --------------------------------------------------------------------

/// This is used to store event, property and field maps.
type LazyOrderedMultiMap<'Key, 'Data when 'Key: equality>(keyf: 'Data -> 'Key, lazyItems: InterruptibleLazy<'Data list>) =
type LazyOrderedMultiMap<'Key, 'Data when 'Key: equality
#if !NO_CHECKNULLS
and 'Key:not null
#endif
>(keyf: 'Data -> 'Key, lazyItems: InterruptibleLazy<'Data list>) =

let quickMap =
lazyItems
Expand Down Expand Up @@ -515,7 +519,8 @@ type ILAssemblyRef(data) =

let retargetable = aname.Flags = AssemblyNameFlags.Retargetable

ILAssemblyRef.Create(aname.Name, None, publicKey, retargetable, version, locale)
let name = match aname.Name with | null -> aname.FullName | name -> name
ILAssemblyRef.Create(name, None, publicKey, retargetable, version, locale)

member aref.QualifiedName =
let b = StringBuilder(100)
Expand Down Expand Up @@ -823,7 +828,7 @@ type ILTypeRef =
member x.DebugText = x.ToString()

/// For debugging
override x.ToString() = x.FullName
override x.ToString() : string = x.FullName

and [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>] ILTypeSpec =
{
Expand Down Expand Up @@ -875,7 +880,7 @@ and [<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugT
&& (x.GenericArgs = y.GenericArgs)

override x.ToString() =
x.TypeRef.ToString() + if isNil x.GenericArgs then "" else "<...>"
x.TypeRef.FullName + if isNil x.GenericArgs then "" else "<...>"

and [<RequireQualifiedAccess; StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>] ILType =
| Void
Expand Down Expand Up @@ -1017,8 +1022,9 @@ type ILMethodRef =
[<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
member x.DebugText = x.ToString()

override x.ToString() =
x.DeclaringTypeRef.ToString() + "::" + x.Name + "(...)"
member x.FullName = x.DeclaringTypeRef.FullName + "::" + x.Name + "(...)"

override x.ToString() = x.FullName

[<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>]
type ILFieldRef =
Expand All @@ -1033,7 +1039,7 @@ type ILFieldRef =
member x.DebugText = x.ToString()

override x.ToString() =
x.DeclaringTypeRef.ToString() + "::" + x.Name
x.DeclaringTypeRef.FullName + "::" + x.Name

[<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>]
type ILMethodSpec =
Expand Down Expand Up @@ -1072,7 +1078,7 @@ type ILMethodSpec =
[<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
member x.DebugText = x.ToString()

override x.ToString() = x.MethodRef.ToString() + "(...)"
override x.ToString() = x.MethodRef.FullName + "(...)"

[<StructuralEquality; StructuralComparison; StructuredFormatDisplay("{DebugText}")>]
type ILFieldSpec =
Expand Down Expand Up @@ -1213,7 +1219,7 @@ type ILAttribute =
[<DebuggerBrowsable(DebuggerBrowsableState.Never)>]
member x.DebugText = x.ToString()

override x.ToString() = x.Method.ToString() + "(...)"
override x.ToString() = x.Method.MethodRef.FullName

[<NoEquality; NoComparison; Struct>]
type ILAttributes(array: ILAttribute[]) =
Expand Down Expand Up @@ -1571,7 +1577,7 @@ type ILFieldInit =
| ILFieldInit.UInt64 u64 -> box u64
| ILFieldInit.Single ieee32 -> box ieee32
| ILFieldInit.Double ieee64 -> box ieee64
| ILFieldInit.Null -> (null :> Object)
| ILFieldInit.Null -> (null :> objnull)

// --------------------------------------------------------------------
// Native Types, for marshalling to the native C interface.
Expand Down
4 changes: 3 additions & 1 deletion src/Compiler/AbstractIL/il.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ type ILTypeRef =

member internal EqualsWithPrimaryScopeRef: ILScopeRef * obj -> bool

override ToString: unit -> string

interface System.IComparable

/// Type specs and types.
Expand Down Expand Up @@ -664,7 +666,7 @@ type ILFieldInit =
| Double of double
| Null

member AsObject: unit -> obj
member AsObject: unit -> objnull

[<RequireQualifiedAccess; StructuralEquality; StructuralComparison>]
type internal ILNativeVariant =
Expand Down
9 changes: 7 additions & 2 deletions src/Compiler/AbstractIL/ilnativeres.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ type Directory(name, id) =
member val ID = id
member val NumberOfNamedEntries = Unchecked.defaultof<uint16> with get, set
member val NumberOfIdEntries = Unchecked.defaultof<uint16> with get, set
member val Entries = List<obj>()
member val Entries = List<objnull>()

type NativeResourceWriter() =
static member private CompareResources (left: Win32Resource) (right: Win32Resource) =
Expand Down Expand Up @@ -1149,7 +1149,12 @@ type NativeResourceWriter() =
dataWriter.WriteByte 0uy

false
| e -> failwithf "Unknown entry %s" (if isNull e then "<NULL>" else e.GetType().FullName)
| e ->
failwithf
"Unknown entry %s"
(match e with
| null -> "<NULL>"
| e -> e.GetType().FullName)

if id >= 0 then
writer.WriteInt32 id
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/AbstractIL/ilpars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
%{

#nowarn "1182" // the generated code often has unused variable "parseState"
#nowarn "3261" // the generated code would need to properly annotate nulls, e.g. changing System.Object to `obj|null`

open Internal.Utilities.Library

Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/AbstractIL/ilread.fs
Original file line number Diff line number Diff line change
Expand Up @@ -941,11 +941,11 @@ let mkCacheGeneric lowMem _inbase _nm (sz: int) =
fun f (idx: 'T) ->
let cache =
match cache with
| Null ->
| null ->
let v = ConcurrentDictionary<_, _>(Environment.ProcessorCount, sz)
cache <- v
v
| NonNull v -> v
| v -> v

match cache.TryGetValue idx with
| true, v ->
Expand Down
Loading

0 comments on commit ccd0de1

Please sign in to comment.