Multi-assembly+debug emit for FSI

docs/debug-emit.md

@@ -504,7 +504,3 @@ Some design-time services are un-implemented by F#:
 * Unimplemented: [Proximity expressions](https://github.com/dotnet/fsharp/issues/4271) (for Autos window)
 
 These are major holes in the F# experience and should be implemented.
-
-### Missing debug emit for F# Interactive
-
-For F# Interactive [we do not currently emit debug information for script code](https://github.com/dotnet/fsharp/issues/5457). This is because of a missing piece of functionality in the Reflection.Emit APIs, and means we have to change our approach to emitting code fragments in F# Interactive to no longer use dynamic assemblies.

docs/fsi-emit.md

@@ -0,0 +1,84 @@
+---
+title: F# Interactive Emit
+category: Compiler Internals
+categoryindex: 200
+index: 375
+---
+# F# Interactive Code Generation
+
+F# Interactive (`dotnet fsi`) accepts incremental code fragments. This capability is also used by [hosted execution capability of the FSharp.Compiler.Service API](fcs/interactive.fsx) which is used to build the F# kernel for .NET Interactive notebooks.
+
+Historically F# Interactive code was  emitted into a single dynamic assembly using Reflection.Emit and ilreflect.fs (meaning one assembly that is continually growing). However, .NET Core Reflection.Emit does not support the emit of debug symbols for dynamic assemblies, so in Feb 2022 we switched to emitting multiple non-dynamic assemblies (meaning assemblies dynamically created in-memory using ilwrite.fs, and loaded, but not growing).
+
+The assemblies are named:
+
+`FSI-ASSEMBLY1`
+`FSI-ASSEMBLY2`
+
+etc.
+
+## Compat switch
+
+There is a switch `fsi --multiemit` that turns on the use of multi-assembly generation (when it is off, we use Reflection Emit for  single-dynamic-assembly generation).  This is on by default for .NET Core, and off by default for .NET Framework for compat reasons.
+
+## Are multiple assemblies too costly?
+
+There is general assumption in this that on modern dev machines (where users execute multiple interactions) then generating 50, 100 or 1,000 or 10,000 dynamic assemblies by repeated manual execution of code is not a problem: the extra overheads of multiple assemblies compared to one dynamic assembly is of no real significance in developer REPL scenarios given the vast amount of memory available on modern 64-bit machines.
+
+Quick check: adding 10,000 `let x = 1;;` interactions to .NET Core `dotnet fsi` adds about 300MB to the FSI.EXE process, meaning 30K/interaction. A budget of 1GB for interactive fragments (reasonable on a 64-bit machine), and an expected maximum of 10000 fragments before restart (that's a lot!), then each fragment can take up to 100K. This is well below the cost of a new assembly.
+
+Additionally, these costs are not substantially reduced if `--multiemit` is disabled, so they've always been the approximate costs of F# Interactive fragment generation.
+
+## Internals and accessibility across fragments
+
+Generating into multiple assemblies raises issues for some things that are assembly bound such as "internals" accessibility. In a first iteration of this we had a failing case here:
+
+```fsharp
+> artifacts\bin\fsi\Debug\net50\fsi.exe --optimize-
+...
+// Fragment 1
+> let internal f() = 1;;
+val internal f: unit -> int
+
+// Fragment 2 - according to existing rules it is allowed to access internal things of the first
+f();; 
+System.MethodAccessException: Attempt by method '<StartupCode$FSI_0003>.$FSI_0003.main@()' to access method 'FSI_0002.f()' failed.
+   at <StartupCode$FSI_0003>.$FSI_0003.main@()
+```
+
+This is because we are now generating into multiple assemblies. Another bug was this:
+
+```fsharp
+> artifacts\bin\fsi\Debug\net50\fsi.exe --optimize+
+...
+// Fragment 1 - not `x` becomes an internal field of the class
+> type C() =
+>    let mutable x = 1
+>    member _.M() = x
+> ;;
+...
+// Fragment 2 - inlining 'M()' gave an access to the internal field `x`
+> C().M();;
+...<bang>...
+```
+
+According to the current F# scripting programming model (the one checked in the editor), the "internal" thing should be accessible in subsequent fragments. Should this be changed? No:
+
+* It's very hard to adjust the implementation of the editor scripting model to consider fragments delimited by `;;` to be different assemblies, whether in the editor or in F# Interactive. 
+* And would we even want to?  It's common enough for people to debug code scattered with "internal" declarations.  
+* In scripts, the `;;` aren't actually accurate markers for what will or won't be sent to F# Interactive, which get added implicitly.
+
+  For example, consider the script
+
+  ```fsharp
+  let internal f() = 1;;
+  f();; 
+  ```
+
+  In the editor should this be given an error or not?  That is, should the `;;` be seen as accurate indicators of separate script fragments? (Answer: yes if we know the script will be piped-to-input, no if the script is used as a single file entry - when the `;;` are ignored)
+
+* Further, this would be a breaking change, e.g. it could arise in an automated compat situation if people are piping into standard input and the input contains `;;` markers.
+
+Because of this we emit IVTs for the next 30 `FSI-ASSEMBLYnnn` assemblies on each assembly fragment, giving a warning when an internal thing is accessed across assembly boundaries within that 30 (reporting it as a deprecated feature), and give an error if internal access happens after that.
+
+From a compat perspective this seems reasonable, and the compat flag is available to return the whole system to generate-one-assembly behavior.

src/fsharp/AttributeChecking.fs

@@ -125,7 +125,7 @@ type AttribInfo =
 /// Check custom attributes. This is particularly messy because custom attributes come in in three different
 /// formats.
 let AttribInfosOfIL g amap scoref m (attribs: ILAttributes) = 
-    attribs.AsList  |> List.map (fun a -> ILAttribInfo (g, amap, scoref, a, m))
+    attribs.AsList()  |> List.map (fun a -> ILAttribInfo (g, amap, scoref, a, m))
 
 let AttribInfosOfFS g attribs = 
     attribs |> List.map (fun a -> FSAttribInfo (g, a))
@@ -443,7 +443,7 @@ let MethInfoIsUnseen g (m: range) (ty: TType) minfo =
         // We are only interested in filtering out the method on System.Object, so it is sufficient
         // just to look at the attributes on IL methods.
         if tcref.IsILTycon then 
-                tcref.ILTyconRawMetadata.CustomAttrs.AsArray 
+                tcref.ILTyconRawMetadata.CustomAttrs.AsArray()
                 |> Array.exists (fun attr -> attr.Method.DeclaringType.TypeSpec.Name = typeof<TypeProviderEditorHideMethodsAttribute>.FullName)
         else 
             false
@@ -452,9 +452,7 @@ let MethInfoIsUnseen g (m: range) (ty: TType) minfo =
         false
 #endif
 
-    //let isUnseenByBeingTupleMethod () = isAnyTupleTy g ty
-
-    isUnseenByObsoleteAttrib () || isUnseenByHidingAttribute () //|| isUnseenByBeingTupleMethod ()
+    isUnseenByObsoleteAttrib () || isUnseenByHidingAttribute ()
 
 /// Indicate if a property has 'Obsolete' or 'CompilerMessageAttribute'.
 /// Used to suppress the item in intellisense.

src/fsharp/CompilerConfig.fs

@@ -447,27 +447,27 @@ type TcConfigBuilder =
       mutable showTimes: bool
       mutable showLoadedAssemblies: bool
       mutable continueAfterParseFailure: bool
+
 #if !NO_EXTENSIONTYPING
       /// show messages about extension type resolution?
       mutable showExtensionTypeMessages: bool
 #endif
 
-      /// pause between passes?
+      /// Pause between passes?
       mutable pause: bool
-      /// whenever possible, emit callvirt instead of call
+
+      /// Whenever possible, emit callvirt instead of call
       mutable alwaysCallVirt: bool
 
-      /// if true, strip away data that would not be of use to end users, but is useful to us for debugging
-      // REVIEW: "stripDebugData"?
+      /// If true, strip away data that would not be of use to end users, but is useful to us for debugging
       mutable noDebugAttributes: bool
 
-      /// if true, indicates all type checking and code generation is in the context of fsi.exe
+      /// If true, indicates all type checking and code generation is in the context of fsi.exe
       isInteractive: bool
-      isInvalidationSupported: bool
 
-      /// used to log sqm data
+      isInvalidationSupported: bool
 
-      /// if true - every expression in quotations will be augmented with full debug info (filename, location in file)
+      /// If true - every expression in quotations will be augmented with full debug info (filename, location in file)
       mutable emitDebugInfoInQuotations: bool
 
       mutable exename: string option
@@ -477,9 +477,14 @@ type TcConfigBuilder =
 
       /// When false FSI will lock referenced assemblies requiring process restart, false = disable Shadow Copy false (*default*)
       mutable shadowCopyReferences: bool
+
       mutable useSdkRefs: bool
+
       mutable fxResolver: FxResolver option
 
+      // Is F# Interactive using multi-assembly emit?
+      mutable fsiMultiAssemblyEmit: bool
+
       /// specify the error range for FxResolver
       rangeForErrors: range
 
@@ -660,6 +665,7 @@ type TcConfigBuilder =
           shadowCopyReferences = false
           useSdkRefs = true
           fxResolver = None
+          fsiMultiAssemblyEmit = true
           internalTestSpanStackReferring = false
           noConditionalErasure = false
           pathMap = PathMap.empty
@@ -921,6 +927,7 @@ type TcConfig private (data: TcConfigBuilder, validate: bool) =
 #endif
                 None, data.legacyReferenceResolver.Impl.HighestInstalledNetFrameworkVersion()
 
+    member _.fsiMultiAssemblyEmit = data.fsiMultiAssemblyEmit
     member x.FxResolver = data.FxResolver
     member x.primaryAssembly = data.primaryAssembly
     member x.noFeedback = data.noFeedback

src/fsharp/CompilerConfig.fsi

@@ -272,6 +272,7 @@ type TcConfigBuilder =
       mutable shadowCopyReferences: bool
       mutable useSdkRefs: bool
       mutable fxResolver: FxResolver option
+      mutable fsiMultiAssemblyEmit: bool
       rangeForErrors: range
       sdkDirOverride: string option
 
@@ -458,6 +459,9 @@ type TcConfig =
     member isInteractive: bool
     member isInvalidationSupported: bool
 
+    /// Indicates if F# Interactive is using single-assembly emit via Reflection.Emit, where internals are available.
+    member fsiMultiAssemblyEmit: bool
+
     member xmlDocInfoLoader: IXmlDocumentationInfoLoader option
 
     member FxResolver: FxResolver

src/fsharp/CompilerImports.fs

@@ -650,7 +650,7 @@ let MakeScopeRefForILModule (ilModule: ILModuleDef) =
     | None -> ILScopeRef.Module (mkRefToILModule ilModule)
 
 let GetCustomAttributesOfILModule (ilModule: ILModuleDef) =
-    (match ilModule.Manifest with Some m -> m.CustomAttrs | None -> ilModule.CustomAttrs).AsList
+    (match ilModule.Manifest with Some m -> m.CustomAttrs | None -> ilModule.CustomAttrs).AsList()
 
 let GetAutoOpenAttributes ilModule =
     ilModule |> GetCustomAttributesOfILModule |> List.choose TryFindAutoOpenAttr
@@ -669,7 +669,7 @@ type RawFSharpAssemblyDataBackedByFileOnDisk (ilModule: ILModuleDef, ilAssemblyR
          member _.TryGetILModuleDef() = Some ilModule
 
          member _.GetRawFSharpSignatureData(m, ilShortAssemName, filename) =
-            let resources = ilModule.Resources.AsList
+            let resources = ilModule.Resources.AsList()
             let sigDataReaders =
                 [ for iresource in resources do
                     if IsSignatureDataResource iresource then
@@ -688,7 +688,7 @@ type RawFSharpAssemblyDataBackedByFileOnDisk (ilModule: ILModuleDef, ilAssemblyR
 
          member _.GetRawFSharpOptimizationData(m, ilShortAssemName, filename) =
             let optDataReaders =
-                ilModule.Resources.AsList
+                ilModule.Resources.AsList()
                 |> List.choose (fun r -> if IsOptimizationDataResource r then Some(GetOptimizationDataResourceName r, (fun () -> r.GetBytes())) else None)
 
             // Look for optimization data in a file
@@ -733,14 +733,14 @@ type RawFSharpAssemblyData (ilModule: ILModuleDef, ilAssemblyRefs) =
          member _.TryGetILModuleDef() = Some ilModule
 
          member _.GetRawFSharpSignatureData(_, _, _) =
-            let resources = ilModule.Resources.AsList
+            let resources = ilModule.Resources.AsList()
             [ for iresource in resources do
                 if IsSignatureDataResource iresource then
                     let ccuName = GetSignatureDataResourceName iresource
                     yield (ccuName, fun () -> iresource.GetBytes()) ]
 
          member _.GetRawFSharpOptimizationData(_, _, _) =
-            ilModule.Resources.AsList
+            ilModule.Resources.AsList()
             |> List.choose (fun r -> if IsOptimizationDataResource r then Some(GetOptimizationDataResourceName r, (fun () -> r.GetBytes())) else None)
 
          member _.GetRawTypeForwarders() =
@@ -1499,7 +1499,7 @@ and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse
 
         let phase2 () =
 #if !NO_EXTENSIONTYPING
-            ccuinfo.TypeProviders <- tcImports.ImportTypeProviderExtensions (ctok, tcConfig, filename, ilScopeRef, ilModule.ManifestOfAssembly.CustomAttrs.AsList, ccu.Contents, invalidateCcu, m)
+            ccuinfo.TypeProviders <- tcImports.ImportTypeProviderExtensions (ctok, tcConfig, filename, ilScopeRef, ilModule.ManifestOfAssembly.CustomAttrs.AsList(), ccu.Contents, invalidateCcu, m)
 #endif
             [ResolvedImportedAssembly ccuinfo]
         phase2
@@ -1593,7 +1593,7 @@ and [<Sealed>] TcImports(tcConfigP: TcConfigProvider, initialResolutions: TcAsse
                      match ilModule.TryGetILModuleDef() with
                      | None -> () // no type providers can be used without a real IL Module present
                      | Some ilModule ->
-                         let tps = tcImports.ImportTypeProviderExtensions (ctok, tcConfig, filename, ilScopeRef, ilModule.ManifestOfAssembly.CustomAttrs.AsList, ccu.Contents, invalidateCcu, m)
+                         let tps = tcImports.ImportTypeProviderExtensions (ctok, tcConfig, filename, ilScopeRef, ilModule.ManifestOfAssembly.CustomAttrs.AsList(), ccu.Contents, invalidateCcu, m)
                          ccuinfo.TypeProviders <- tps
 #else
                      ()

src/fsharp/IlxGen.fs

@@ -1565,11 +1565,11 @@ type TypeDefBuilder(tdef: ILTypeDef, tdefDiscards) =
     let gnested = TypeDefsBuilder()
 
     member b.Close() =
-        tdef.With(methods = mkILMethods (tdef.Methods.AsList @ ResizeArray.toList gmethods),
-                  fields = mkILFields (tdef.Fields.AsList @ ResizeArray.toList gfields),
-                  properties = mkILProperties (tdef.Properties.AsList @ HashRangeSorted gproperties ),
-                  events = mkILEvents (tdef.Events.AsList @ ResizeArray.toList gevents),
-                  nestedTypes = mkILTypeDefs (tdef.NestedTypes.AsList @ gnested.Close()))
+        tdef.With(methods = mkILMethods (tdef.Methods.AsList() @ ResizeArray.toList gmethods),
+                  fields = mkILFields (tdef.Fields.AsList() @ ResizeArray.toList gfields),
+                  properties = mkILProperties (tdef.Properties.AsList() @ HashRangeSorted gproperties ),
+                  events = mkILEvents (tdef.Events.AsList() @ ResizeArray.toList gevents),
+                  nestedTypes = mkILTypeDefs (tdef.NestedTypes.AsList() @ gnested.Close()))
 
     member b.AddEventDef edef = gevents.Add edef
 
@@ -1618,11 +1618,11 @@ and TypeDefsBuilder() =
               let tdef = b.Close()
               // Skip the <PrivateImplementationDetails$> type if it is empty
               if not eliminateIfEmpty
-                 || not tdef.NestedTypes.AsList.IsEmpty
-                 || not tdef.Fields.AsList.IsEmpty
-                 || not tdef.Events.AsList.IsEmpty
-                 || not tdef.Properties.AsList.IsEmpty
-                 || not (Array.isEmpty tdef.Methods.AsArray) then
+                 || not (tdef.NestedTypes.AsList()).IsEmpty
+                 || not (tdef.Fields.AsList()).IsEmpty
+                 || not (tdef.Events.AsList()).IsEmpty
+                 || not (tdef.Properties.AsList()).IsEmpty
+                 || not (Array.isEmpty (tdef.Methods.AsArray())) then
                   yield tdef ]
 
     member b.FindTypeDefBuilder nm =
@@ -1704,7 +1704,9 @@ type AssemblyBuilder(cenv: cenv, anonTypeTable: AnonTypeGenerationTable) as mgbu
             let ilFieldDefs =
                 mkILFields
                     [ for _, fldName, fldTy in flds ->
-                        let fdef = mkILInstanceField (fldName, fldTy, None, ILMemberAccess.Private)
+                        // The F# Interactive backend may split to multiple assemblies.
+                        let access = (if cenv.opts.isInteractive then ILMemberAccess.Public else  ILMemberAccess.Private)
+                        let fdef = mkILInstanceField (fldName, fldTy, None, access)
                         fdef.With(customAttrs = mkILCustomAttrs [ g.DebuggerBrowsableNeverAttribute ]) ]
 
             // Generate property definitions for the fields compiled as properties
@@ -4536,7 +4538,7 @@ and GenFormalReturnType m cenv eenvFormal returnTy : ILReturn =
     | None -> ilRet
     | Some ty ->
     match GenReadOnlyAttributeIfNecessary cenv.g ty with
-    | Some attr -> ilRet.WithCustomAttrs (mkILCustomAttrs (ilRet.CustomAttrs.AsList @ [attr]))
+    | Some attr -> ilRet.WithCustomAttrs (mkILCustomAttrs (ilRet.CustomAttrs.AsList() @ [attr]))
     | None -> ilRet
 
 and instSlotParam inst (TSlotParam(nm, ty, inFlag, fl2, fl3, attrs)) =
@@ -5042,7 +5044,7 @@ and GenStaticDelegateClosureTypeDefs cenv (tref: ILTypeRef, ilGenParams, attrs,
     // Apply the abstract attribute, turning the sealed class into abstract sealed (i.e. static class).
     // Remove the redundant constructor.
     tdefs |> List.map (fun td -> td.WithAbstract(true)
-                                   .With(methods= mkILMethodsFromArray (td.Methods.AsArray |> Array.filter (fun m -> not m.IsConstructor))))
+                                   .With(methods= mkILMethodsFromArray (td.Methods.AsArray() |> Array.filter (fun m -> not m.IsConstructor))))
 
 and GenGenericParams cenv eenv tps =
     tps |> DropErasedTypars |> List.map (GenGenericParam cenv eenv)
@@ -8591,9 +8593,7 @@ open System
 
 /// The lookup* functions are the conversions available from ilreflect.
 type ExecutionContext =
-    { LookupFieldRef: ILFieldRef -> FieldInfo
-      LookupMethodRef: ILMethodRef -> MethodInfo
-      LookupTypeRef: ILTypeRef -> Type
+    { LookupTypeRef: ILTypeRef -> Type
       LookupType: ILType -> Type }
 
 // A helper to generate a default value for any System.Type. I couldn't find a System.Reflection
@@ -8615,7 +8615,7 @@ let LookupGeneratedValue (amap: ImportMap) (ctxt: ExecutionContext) eenv (v: Val
   try
     // Convert the v.Type into a System.Type according to ilxgen and ilreflect.
     let objTyp() =
-        let ilTy = GenType amap v.Range TypeReprEnv.Empty v.Type (* TypeReprEnv.Empty ok, not expecting typars *)
+        let ilTy = GenType amap v.Range TypeReprEnv.Empty v.Type
         ctxt.LookupType ilTy
     // Lookup the compiled v value (as an object).
     match StorageForVal amap.g v.Range v eenv with

src/fsharp/IlxGen.fsi

@@ -79,8 +79,6 @@ type public IlxGenResults =
 /// Used to support the compilation-inversion operations "ClearGeneratedValue" and "LookupGeneratedValue"
 type ExecutionContext =
     {
-      LookupFieldRef: ILFieldRef -> FieldInfo
-      LookupMethodRef: ILMethodRef -> MethodInfo
       LookupTypeRef: ILTypeRef -> Type
       LookupType: ILType -> Type
     }