Emit (+flatten) import scope tables

eng/Versions.props

@@ -89,6 +89,7 @@
     <SystemDesignVersion>4.0.0</SystemDesignVersion>
     <SystemDiagnosticsProcessVersion>4.3.0</SystemDiagnosticsProcessVersion>
     <SystemDiagnosticsTraceSourceVersion>4.3.0</SystemDiagnosticsTraceSourceVersion>
+    <MicrosoftDiaSymReaderPortablePdbVersion>1.6.0</MicrosoftDiaSymReaderPortablePdbVersion>
     <SystemIoCompressionVersion>4.3.0</SystemIoCompressionVersion>
     <SystemLinqExpressionsVersion>4.3.0</SystemLinqExpressionsVersion>
     <SystemLinqQueryableVersion>4.3.0</SystemLinqQueryableVersion>

src/fsharp/IlxGen.fs

@@ -1418,35 +1418,45 @@ let AddBindingsForTycon allocVal (cloc: CompileLocation) (tycon: Tycon) eenv =
 let rec AddBindingsForModuleDefs allocVal (cloc: CompileLocation) eenv mdefs =
     List.fold (AddBindingsForModuleDef allocVal cloc) eenv mdefs
 
-and AddDebugImportsToEnv _cenv eenv (openDecls: OpenDeclaration list) =
+and AddDebugImportsToEnv (cenv: cenv) eenv (openDecls: OpenDeclaration list) =
     let ilImports =
         [| 
           for openDecl in openDecls do
             for modul in openDecl.Modules do
                 if modul.IsNamespace then
                     ILDebugImport.ImportNamespace (fullDisplayTextOfModRef modul)
-                // Emit of 'open type' and 'open <module>' is causing problems
-                // See https://github.com/dotnet/fsharp/pull/12010#issuecomment-903339109
-                //
-                // It may be nested types/modules in particular
-                //else
-                //    ILDebugImport.ImportType (mkILNonGenericBoxedTy modul.CompiledRepresentationForNamedType)
-            //for t in openDecl.Types do
-            //    let m = defaultArg openDecl.Range Range.range0
-            //    ILDebugImport.ImportType (GenType cenv.amap m TypeReprEnv.Empty t)
+                else
+                    ILDebugImport.ImportType (mkILNonGenericBoxedTy modul.CompiledRepresentationForNamedType)
+            for t in openDecl.Types do
+                let m = defaultArg openDecl.Range Range.range0
+                ILDebugImport.ImportType (GenType cenv.amap m TypeReprEnv.Empty t)
         |]
-        |> Array.distinctBy (function
-            | ILDebugImport.ImportNamespace nsp -> nsp
-            | ILDebugImport.ImportType t -> t.QualifiedName)
 
     if ilImports.Length = 0 then
         eenv
     else
+        // We flatten _all_ the import scopes, creating repetition, because C# debug engine doesn't seem to handle
+        // nesting of import scopes at all. This means every new "open" in, say, a nested module in F# causes 
+        // duplication of all the implicit/enclosing "open" in within the debug information. 
+        // However overall there are not very many "open" declarations and debug information can be large
+        // so this is not considered a problem.
         let imports =
-            { Parent = eenv.imports
-              Imports = ilImports }
+            [| match eenv.imports with 
+               | None -> ()
+               | Some parent -> yield! parent.Imports
+               yield! ilImports |]
+             |> Array.filter (function
+                | ILDebugImport.ImportNamespace _ -> true
+                | ILDebugImport.ImportType t ->
+                    t.IsNominal &&
+                    // We filter out FSI_NNNN types (dynamic modules), since we don't really need them in the import tables.
+                    not (t.QualifiedName.StartsWithOrdinal FsiDynamicModulePrefix
+                         && t.TypeRef.Scope = ILScopeRef.Local ))
+             |> Array.distinctBy (function
+                | ILDebugImport.ImportNamespace nsp -> nsp
+                | ILDebugImport.ImportType t -> t.QualifiedName)
 
-        { eenv with imports = Some imports }
+        { eenv with imports = Some { Parent = None; Imports = imports } }
 
 and AddBindingsForModuleDef allocVal cloc eenv x =
     match x with

src/fsharp/absil/ilwrite.fs

@@ -770,13 +770,17 @@ and GetResolutionScopeAsElem cenv (scoref, enc) =
         (rs_TypeRef, GetTypeDescAsTypeRefIdx cenv (scoref, enc2, n2))
 
 
-let emitTypeInfoAsTypeDefOrRefEncoded cenv (bb: ByteBuffer) (scoref, enc, nm) =
+let getTypeInfoAsTypeDefOrRefEncoded cenv (scoref, enc, nm) =
     if isScopeRefLocal scoref then
         let idx = GetIdxForTypeDef cenv (TdKey(enc, nm))
-        bb.EmitZ32 (idx <<< 2) // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeDef
+        idx <<< 2 // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeDef
     else
         let idx = GetTypeDescAsTypeRefIdx cenv (scoref, enc, nm)
-        bb.EmitZ32 ((idx <<< 2) ||| 0x01) // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeRef
+        ((idx <<< 2) ||| 0x01) // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeRef
+
+let emitTypeInfoAsTypeDefOrRefEncoded cenv (bb: ByteBuffer) (scoref, enc, nm) =
+    let tok = getTypeInfoAsTypeDefOrRefEncoded cenv (scoref, enc, nm)
+    bb.EmitZ32 tok
 
 let getTypeDefOrRefAsUncodedToken (tag, idx) =
     let tab =
@@ -2219,21 +2223,25 @@ let GetFieldDefTypeAsBlobIdx cenv env ty =
                                               EmitType cenv env bb ty)
     GetBytesAsBlobIdx cenv bytes
 
-let GenPdbImport (cenv: cenv) env (input: ILDebugImport) =
+let GenPdbImport (cenv: cenv) (input: ILDebugImport) =
     match input with 
-    | ILDebugImport.ImportType ty -> PdbImport.ImportType (getTypeDefOrRefAsUncodedToken (GetTypeAsTypeDefOrRef cenv env ty))
+    | ILDebugImport.ImportType ty ->
+        let tspec = ty.TypeSpec
+        let tok = getTypeInfoAsTypeDefOrRefEncoded cenv (tspec.Scope, tspec.Enclosing, tspec.Name)
+        PdbImport.ImportType tok
+
     | ILDebugImport.ImportNamespace nsp -> PdbImport.ImportNamespace nsp
 
-let rec GenPdbImports (cenv: cenv) env (input: ILDebugImports option) =
+let rec GenPdbImports (cenv: cenv) (input: ILDebugImports option) =
     match input with 
     | None -> None
     | Some ilImports -> 
         match cenv.pdbImports.TryGetValue(ilImports) with
         | true, v -> Some v
         | _ ->
             let v : PdbImports = 
-                { Imports = ilImports.Imports |> Array.map (GenPdbImport cenv env)
-                  Parent = GenPdbImports cenv env ilImports.Parent }
+                { Imports = ilImports.Imports |> Array.map (GenPdbImport cenv)
+                  Parent = GenPdbImports cenv ilImports.Parent }
             cenv.pdbImports.[ilImports] <- v
             Some v
 
@@ -2248,7 +2256,7 @@ let GenILMethodBody mname cenv env (il: ILMethodBody) =
       else
         [| |]
 
-    let imports = GenPdbImports cenv env il.DebugImports
+    let imports = GenPdbImports cenv il.DebugImports
     let requiredStringFixups, seh, code, seqpoints, scopes = Codebuf.EmitMethodCode cenv imports localSigs env mname il.Code
     let codeSize = code.Length
     use methbuf = ByteBuffer.Create (codeSize * 3)

src/fsharp/absil/ilwritepdb.fs

@@ -490,10 +490,10 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s
         //        writer.WriteByte((byte)ImportDefinitionKind.ImportAssemblyReferenceAlias);
         //        writer.WriteCompressedInteger(MetadataTokens.GetHeapOffset(_debugMetadataOpt.GetOrAddBlobUTF8(import.AliasOpt)));
 
-    let serializeImportsBlob (scope: PdbImports) =
+    let serializeImportsBlob (imports: PdbImport[]) =
         let writer = new BlobBuilder()
 
-        for import in scope.Imports do
+        for import in imports do
             serializeImport writer import
 
         metadata.GetOrAddBlob(writer)
@@ -515,7 +515,7 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s
             | None -> moduleImportScopeHandle
             | Some parent -> getImportScopeIndex parent
 
-        let blob = serializeImportsBlob imports
+        let blob = serializeImportsBlob imports.Imports
         let result = metadata.AddImportScope(parentScopeHandle, blob)
 
         importScopesTable.Add(imports, result)
@@ -542,19 +542,15 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s
     let writeMethodScopes methToken rootScope =
 
         let flattenedScopes = flattenScopes rootScope
-
-        // Get or create the import scope for this method
-        let importScopeHandle =
-#if EMIT_IMPORT_SCOPES
-            match s.Imports with 
-            | None -> Unchecked.defaultof<_>
-            | Some imports -> getImportScopeIndex imports
-#else
-            getImportScopeIndex |> ignore // make sure this code counts as used
-            Unchecked.defaultof<_>
-#endif
 
         for scope in flattenedScopes do
+            // Get or create the import scope for this method
+            let importScopeHandle =
+
+                match scope.Imports with 
+                | None -> Unchecked.defaultof<_>
+                | Some imports -> getImportScopeIndex imports
+
             let lastRowNumber = MetadataTokens.GetRowNumber(LocalVariableHandle.op_Implicit lastLocalVariableHandle)
             let nextHandle = MetadataTokens.LocalVariableHandle(lastRowNumber + 1)
 
@@ -684,14 +680,9 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s
         sortMethods showTimes info
         metadata.SetCapacity(TableIndex.MethodDebugInformation, info.Methods.Length)
 
-// Currently disabled, see 
-#if EMIT_IMPORT_SCOPES
         defineModuleImportScope()
-#else
-        defineModuleImportScope |> ignore // make sure this function counts as used
-#endif
 
-        for minfo in info.Methods do 
+        for minfo in info.Methods do
             emitMethod minfo
 
         let entryPoint =
@@ -994,6 +985,7 @@ let logDebugInfo (outfile: string) (info: PdbData) =
         fprintfn sw "      %s- [%d-%d]" offs scope.StartOffset scope.EndOffset
         if scope.Locals.Length > 0 then
           fprintfn sw "      %s  Locals: %A" offs [ for p in scope.Locals -> sprintf "%d: %s" p.Index p.Name ]
+
         for child in scope.Children do writeScope (offs + "  ") child
 
       match meth.RootScope with

tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs

@@ -0,0 +1,66 @@
+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace FSharp.Compiler.ComponentTests.Debugger
+
+open Xunit
+open FSharp.Test.Compiler
+open System.Reflection.Metadata
+
+module PortablePdbs =
+
+    [<Fact>]
+    let ``Valid Portable PDBs are produced by compiler`` () =
+        FSharp """
+namespace UserNamespace
+
+open System
+
+module Foo =
+    let getcwd () = Environment.CurrentDirectory
+
+
+namespace UserNamespace2
+
+open System.IO
+
+module Bar =
+    let baz _ = ()
+
+open System.Collections.Generic
+
+module Baz = 
+    let aiou _ = ()
+        """
+        |> asLibrary
+        |> withPortablePdb
+        |> compile
+        |> shouldSucceed
+        |> verifyPdb [
+            VerifyImportScopes [
+                [
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "System" }
+                ]
+                [
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.IO" }
+                ]
+                [
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.IO" }
+                    { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.Collections.Generic" }
+                ]
+            ]
+        ]

tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj

@@ -100,6 +100,7 @@
     <Compile Include="Conformance\UnitsOfMeasure\Diagnostics.fs" />
     <Compile Include="Conformance\UnitsOfMeasure\Parsing.fs" />
     <Compile Include="Conformance\UnitsOfMeasure\TypeChecker.fs" />
+    <Compile Include="Debugger\PortablePdbs.fs" />
     <Compile Include="Diagnostics\async.fs" />
     <Compile Include="Diagnostics\General.fs" />
     <Compile Include="Globalization\GlobalizationTestCases.fs" />