Assembly reader shim (#4915)

* Add assembly reader shim

* Add test

* Extract IModuleReader interface

* Update naming per PR review

Author: auduchinok

fcs/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <FSharpSourcesRoot>$(MSBuildProjectDirectory)\..\..\src</FSharpSourcesRoot>
   </PropertyGroup>
@@ -26,6 +26,9 @@
     <Compile Include="$(FSharpSourcesRoot)\..\tests\service\Common.fs">
       <Link>Common.fs</Link>
     </Compile>
+    <Compile Include="$(FSharpSourcesRoot)\..\tests\service\AssemblyReaderShim.fs">
+      <Link>AssemblyReaderShim.fs</Link>
+    </Compile>
     <Compile Include="$(FSharpSourcesRoot)\..\tests\service\EditorTests.fs">
       <Link>EditorTests.fs</Link>
     </Compile>

src/absil/ilread.fs

@@ -3945,11 +3945,20 @@ type ILReaderOptions =
       metadataOnly: MetadataOnlyFlag
       tryGetMetadataSnapshot: ILReaderTryGetMetadataSnapshot }
 
+
+type ILModuleReader =
+    abstract ILModuleDef : ILModuleDef
+    abstract ILAssemblyRefs : ILAssemblyRef list
+    
+    /// ILModuleReader objects only need to be explicitly disposed if memory mapping is used, i.e. reduceMemoryUsage = false
+    inherit  System.IDisposable
+
+
 [<Sealed>]
-type ILModuleReader(ilModule: ILModuleDef, ilAssemblyRefs: Lazy<ILAssemblyRef list>, dispose: unit -> unit) =
-    member x.ILModuleDef = ilModule
-    member x.ILAssemblyRefs = ilAssemblyRefs.Force()
-    interface IDisposable with
+type ILModuleReaderImpl(ilModule: ILModuleDef, ilAssemblyRefs: Lazy<ILAssemblyRef list>, dispose: unit -> unit) =
+    interface ILModuleReader with
+        member x.ILModuleDef = ilModule
+        member x.ILAssemblyRefs = ilAssemblyRefs.Force()
         member x.Dispose() = dispose()
 
 // ++GLOBAL MUTABLE STATE (concurrency safe via locking)
@@ -3990,7 +3999,7 @@ let tryMemoryMapWholeFile opts fileName =
 let OpenILModuleReaderFromBytes fileName bytes opts = 
     let pefile = ByteFile(fileName, bytes) :> BinaryFile
     let ilModule, ilAssemblyRefs, pdb = openPE (fileName, pefile, opts.pdbDirPath, (opts.reduceMemoryUsage = ReduceMemoryFlag.Yes), opts.ilGlobals, true)
-    new ILModuleReader(ilModule, ilAssemblyRefs, (fun () -> ClosePdbReader pdb))
+    new ILModuleReaderImpl(ilModule, ilAssemblyRefs, (fun () -> ClosePdbReader pdb)) :> ILModuleReader
 
 let OpenILModuleReader fileName opts = 
     // Pseudo-normalize the paths.
@@ -4046,18 +4055,18 @@ let OpenILModuleReader fileName opts =
                         createByteFileChunk opts fullPath (Some (metadataPhysLoc, metadataSize))
 
                 let ilModule, ilAssemblyRefs = openPEMetadataOnly (fullPath, peinfo, pectxtEager, pevEager, mdfile, reduceMemoryUsage, opts.ilGlobals) 
-                new ILModuleReader(ilModule, ilAssemblyRefs, ignore)
+                new ILModuleReaderImpl(ilModule, ilAssemblyRefs, ignore)
             else
                 // If we are not doing metadata-only, then just go ahead and read all the bytes and hold them either strongly or weakly
                 // depending on the heuristic
                 let pefile = createByteFileChunk opts fullPath None
                 let ilModule, ilAssemblyRefs, _pdb = openPE (fullPath, pefile, None, reduceMemoryUsage, opts.ilGlobals, false) 
-                new ILModuleReader(ilModule, ilAssemblyRefs, ignore)
+                new ILModuleReaderImpl(ilModule, ilAssemblyRefs, ignore)
 
         if keyOk then 
             ilModuleReaderCacheLock.AcquireLock (fun ltok -> ilModuleReaderCache.Put(ltok, key, ilModuleReader))
 
-        ilModuleReader
+        ilModuleReader :> ILModuleReader
 
     else
         // This case is primarily used in fsc.exe. 
@@ -4079,13 +4088,25 @@ let OpenILModuleReader fileName opts =
                 disposer, pefile
 
         let ilModule, ilAssemblyRefs, pdb = openPE (fullPath, pefile, opts.pdbDirPath, reduceMemoryUsage, opts.ilGlobals, false)
-        let ilModuleReader = new ILModuleReader(ilModule, ilAssemblyRefs, (fun () -> ClosePdbReader pdb))
+        let ilModuleReader = new ILModuleReaderImpl(ilModule, ilAssemblyRefs, (fun () -> ClosePdbReader pdb))
 
         // Readers with PDB reader disposal logic don't go in the cache.  Note the PDB reader is only used in static linking.
         if keyOk && opts.pdbDirPath.IsNone then 
             ilModuleReaderCacheLock.AcquireLock (fun ltok -> ilModuleReaderCache.Put(ltok, key, ilModuleReader))
 
-        ilModuleReader
+        ilModuleReader :> ILModuleReader
+
+[<AutoOpen>]
+module Shim =
+    open Microsoft.FSharp.Compiler.Lib
 
+    type IAssemblyReader =
+        abstract GetILModuleReader: filename: string * readerOptions: ILReaderOptions -> ILModuleReader
 
+    [<Sealed>]
+    type DefaultAssemblyReader() =
+        interface IAssemblyReader with
+            member __.GetILModuleReader(filename, readerOptions) =
+                OpenILModuleReader filename readerOptions
 
+    let mutable AssemblyReader = DefaultAssemblyReader() :> IAssemblyReader

src/absil/ilread.fsi

@@ -38,12 +38,12 @@ type ILReaderMetadataSnapshot = (obj * nativeint * int)
 type ILReaderTryGetMetadataSnapshot = (* path: *) string * (* snapshotTimeStamp: *) System.DateTime -> ILReaderMetadataSnapshot option
 
 [<RequireQualifiedAccess>]
-type internal MetadataOnlyFlag = Yes | No
+type MetadataOnlyFlag = Yes | No
 
 [<RequireQualifiedAccess>]
-type internal ReduceMemoryFlag = Yes | No
+type ReduceMemoryFlag = Yes | No
 
-type internal ILReaderOptions =
+type ILReaderOptions =
    { pdbDirPath: string option
 
      ilGlobals: ILGlobals
@@ -64,16 +64,17 @@ type internal ILReaderOptions =
      /// and from which we can read the metadata. Only used when metadataOnly=true.
      tryGetMetadataSnapshot: ILReaderTryGetMetadataSnapshot }
 
+
 /// Represents a reader of the metadata of a .NET binary.  May also give some values (e.g. IL code) from the PE file
 /// if it was provided.
-[<Sealed>]
-type internal ILModuleReader =
-    member ILModuleDef : ILModuleDef
-    member ILAssemblyRefs : ILAssemblyRef list
+type ILModuleReader =
+    abstract ILModuleDef: ILModuleDef
+    abstract ILAssemblyRefs: ILAssemblyRef list
 
     /// ILModuleReader objects only need to be explicitly disposed if memory mapping is used, i.e. reduceMemoryUsage = false
-    interface System.IDisposable
-    
+    inherit System.IDisposable
+
+
 /// Open a binary reader, except first copy the entire contents of the binary into 
 /// memory, close the file and ensure any subsequent reads happen from the in-memory store. 
 /// PDB files may not be read with this option. 
@@ -89,4 +90,16 @@ type Statistics =
       mutable weakByteFileCount : int
       mutable byteFileCount : int }
 
-val GetStatistics : unit -> Statistics
+val GetStatistics : unit -> Statistics
+
+[<AutoOpen>]
+module Shim =
+
+    type IAssemblyReader =
+        abstract GetILModuleReader: filename: string * readerOptions: ILReaderOptions -> ILModuleReader
+
+    [<Sealed>]
+    type DefaultAssemblyReader =
+        interface IAssemblyReader
+
+    val mutable AssemblyReader: IAssemblyReader

src/fsharp/CompileOps.fs

@@ -2624,14 +2624,7 @@ type TcConfigBuilder =
             ri, fileNameOfPath ri, ILResourceAccess.Public 
 
 
-let OpenILBinary(filename, reduceMemoryUsage, ilGlobalsOpt, pdbDirPath, shadowCopyReferences, tryGetMetadataSnapshot) = 
-      let ilGlobals   = 
-          // ILScopeRef.Local can be used only for primary assembly (mscorlib or System.Runtime) itself
-          // Remaining assemblies should be opened using existing ilGlobals (so they can properly locate fundamental types)
-          match ilGlobalsOpt with 
-          | None -> mkILGlobals ILScopeRef.Local
-          | Some g -> g
-
+let OpenILBinary(filename, reduceMemoryUsage, ilGlobals, pdbDirPath, shadowCopyReferences, tryGetMetadataSnapshot) =
       let opts : ILReaderOptions = 
           { ilGlobals = ilGlobals
             metadataOnly = MetadataOnlyFlag.Yes
@@ -2652,7 +2645,7 @@ let OpenILBinary(filename, reduceMemoryUsage, ilGlobalsOpt, pdbDirPath, shadowCo
             ignore shadowCopyReferences
 #endif
             filename
-      OpenILModuleReader location opts
+      AssemblyReader.GetILModuleReader(location, opts)
 
 #if DEBUG
 [<System.Diagnostics.DebuggerDisplayAttribute("AssemblyResolution({resolvedPath})")>]
@@ -2782,12 +2775,13 @@ type TcConfig private (data : TcConfigBuilder, validate:bool) =
     do if ((primaryAssemblyExplicitFilenameOpt.IsSome || fslibExplicitFilenameOpt.IsSome) && data.framework) then
             error(Error(FSComp.SR.buildExplicitCoreLibRequiresNoFramework("--noframework"), rangeStartup))
 
+    let ilGlobals = mkILGlobals ILScopeRef.Local
     let clrRootValue, targetFrameworkVersionValue  = 
         match primaryAssemblyExplicitFilenameOpt with
         | Some(primaryAssemblyFilename) ->
             let filename = ComputeMakePathAbsolute data.implicitIncludeDir primaryAssemblyFilename
             try 
-                use ilReader = OpenILBinary(filename, data.reduceMemoryUsage, None, None, data.shadowCopyReferences, data.tryGetMetadataSnapshot)
+                use ilReader = OpenILBinary(filename, data.reduceMemoryUsage, ilGlobals, None, data.shadowCopyReferences, data.tryGetMetadataSnapshot)
                 let ilModule = ilReader.ILModuleDef
                 match ilModule.ManifestOfAssembly.Version with 
                 | Some(v1, v2, _, _) -> 
@@ -2820,7 +2814,7 @@ type TcConfig private (data : TcConfigBuilder, validate:bool) =
             let filename = ComputeMakePathAbsolute data.implicitIncludeDir fslibFilename
             if fslibReference.ProjectReference.IsNone then 
                 try 
-                    use ilReader = OpenILBinary(filename, data.reduceMemoryUsage, None, None, data.shadowCopyReferences, data.tryGetMetadataSnapshot)
+                    use ilReader = OpenILBinary(filename, data.reduceMemoryUsage, ilGlobals, None, data.shadowCopyReferences, data.tryGetMetadataSnapshot)
                     ()
                 with e -> 
                     error(Error(FSComp.SR.buildErrorOpeningBinaryFile(filename, e.Message), rangeStartup))
@@ -4144,7 +4138,17 @@ type TcImports(tcConfigP:TcConfigProvider, initialResolutions:TcAssemblyResoluti
                     None
             else
                 None
-        let ilILBinaryReader = OpenILBinary(filename, tcConfig.reduceMemoryUsage, ilGlobalsOpt, pdbDirPath, tcConfig.shadowCopyReferences, tcConfig.tryGetMetadataSnapshot)
+
+        let ilGlobals =
+            // ILScopeRef.Local can be used only for primary assembly (mscorlib or System.Runtime) itself
+            // Remaining assemblies should be opened using existing ilGlobals (so they can properly locate fundamental types)
+            match ilGlobalsOpt with
+            | None -> mkILGlobals ILScopeRef.Local
+            | Some g -> g
+
+        let ilILBinaryReader =
+            OpenILBinary (filename, tcConfig.reduceMemoryUsage, ilGlobals, pdbDirPath, tcConfig.shadowCopyReferences, tcConfig.tryGetMetadataSnapshot)
+
         tcImports.AttachDisposeAction(fun _ -> (ilILBinaryReader :> IDisposable).Dispose())
         ilILBinaryReader.ILModuleDef, ilILBinaryReader.ILAssemblyRefs
       with e ->

tests/service/AssemblyReaderShim.fs

@@ -0,0 +1,33 @@
+#if INTERACTIVE
+#r "../../debug/fcs/net45/FSharp.Compiler.Service.dll" // note, run 'build fcs debug' to generate this, this DLL has a public API so can be used from F# Interactive
+#r "../../packages/NUnit.3.5.0/lib/net45/nunit.framework.dll"
+#load "FsUnit.fs"
+#load "Common.fs"
+#else
+module FSharp.Compiler.Service.Tests.AssemblyReaderShim
+#endif
+
+open FSharp.Compiler.Service.Tests.Common
+open FsUnit
+open Microsoft.FSharp.Compiler.AbstractIL.ILBinaryReader
+open NUnit.Framework
+
+[<Test>]
+let ``Assembly reader shim gets requests`` () =
+    let defaultReader = Shim.AssemblyReader
+    let mutable gotRequest = false
+    let reader =
+        { new IAssemblyReader with
+            member x.GetILModuleReader(path, opts) =
+                gotRequest <- true
+                defaultReader.GetILModuleReader(path, opts)
+        }
+    Shim.AssemblyReader <- reader
+    let source = """
+module M
+let x = 123
+"""
+
+    let fileName, options = Common.mkTestFileAndOptions source [| |]
+    Common.checker.ParseAndCheckFileInProject(fileName, 0, source, options) |> Async.RunSynchronously |> ignore
+    gotRequest |> should be True