Skip to content

Write compilation references and compilation options to PDBs #14981

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.

Sign up for GitHub

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

Jump to bottom
Draft
baronfel wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Write compilation references and compilation options to PDBs #14981

baronfel wants to merge 6 commits into from

Conversation

@baronfel
Copy link
Member

@baronfel baronfel commented Mar 27, 2023

Fixes #12002

This writes the Compilation Metadata References and Compilation Options tables to the CustomDebugInformation area of the PDB. There are a number of TODOs, and I'm not 100% sure I'm getting the actual information we will want to get a real end-to-end setup.

With these changes, we look like other .NET assemblies!

image

What this should do immediately is allow VS-based tooling to synthesize MetadataReferences from this cached data, so that go-to-def for sourcelinked files has a matching set of assembly references to do typechecking against (and therefore light up IDE tooling).

There are a number of TODOs that I'd like to get some feedback on. I'm fairly sure I'm reading required data correctly, but I want to make sure the sources I'm pulling it from are correct.
Also I'm certain I'm missing a number of compiler options that would be necessary to have on the read-side of this to ensure 1:1 compilation.
Finally, I haven't done the step that would map from the compilation options + compilation references to a proper FSharpProjectOptions - that would probably need to be done to show a true end-to-end in an editor.

I would love any and all review/pointers for the next steps here.

@vzarytovskii
Copy link
Member

vzarytovskii commented Mar 27, 2023 •
edited
Loading

Test framework will probably also need to be extended, to check those:

fsharp/tests/FSharp.Test.Utilities/Compiler.fs

Lines 1210 to 1215 in 3d4eb5c

let verifyPdb (options: PdbVerificationOption list) (result: CompilationResult) : CompilationResult =
match result with
| CompilationResult.Success r -> verifyPortablePdb r options
| _ -> failwith "Result should be \"Success\" in order to verify PDB."
result

Example for sequence points:

fsharp/tests/FSharp.Test.Utilities/Compiler.fs

Lines 1151 to 1161 in 3d4eb5c

let private verifySequencePoints (reader: MetadataReader) expectedSequencePoints =
let sequencePoints =
[ for sp in reader.MethodDebugInformation do
let mdi = reader.GetMethodDebugInformation sp
yield! mdi.GetSequencePoints() ]
|> List.sortBy (fun sp -> sp.StartLine)
|> List.map (fun sp -> (Line sp.StartLine, Col sp.StartColumn, Line sp.EndLine, Col sp.EndColumn) )
if sequencePoints <> expectedSequencePoints then
failwith $"Expected sequence points are different from PDB.\nExpected: %A{expectedSequencePoints}\nActual: %A{sequencePoints}"

And import tables:

fsharp/tests/FSharp.Test.Utilities/Compiler.fs

Lines 1117 to 1149 in 3d4eb5c

let private verifyPdbImportTables (reader: MetadataReader) (scopes: ImportScope list list) =
// There always should be 2 import scopes - 1 empty "root" one, and one flattened table of imports for current scope.
if reader.ImportScopes.Count < 2 then
failwith $"Expected to have at least 2 import scopes, but found {reader.ImportScopes.Count}."
// Sanity check: explicitly test that first import scope is indeed an apty one (i.e. there are no imports).
let rootScope = reader.ImportScopes.ToImmutableArray().Item(0) |> reader.GetImportScope
let rootScopeImportsLength = rootScope.GetImports().ToImmutableArray().Length
if rootScopeImportsLength <> 0 then
failwith $"Expected root scope to have 0 imports, but got {rootScopeImportsLength}."
let pdbScopes = [ for import in reader.ImportScopes -> reader.GetImportScope import ] |> List.skip 1 |> List.rev
if pdbScopes.Length <> scopes.Length then
failwith $"Expected import scopes amount is {scopes.Length}, but got {pdbScopes.Length}."
for (pdbScope, expectedScope) in List.zip pdbScopes scopes do
let imports = [ for import in pdbScope.GetImports() ->
match import.Kind with
| ImportDefinitionKind.ImportNamespace ->
let targetNamespaceBlob = import.TargetNamespace
let targetNamespaceBytes = reader.GetBlobBytes(targetNamespaceBlob)
let name = Encoding.UTF8.GetString(targetNamespaceBytes, 0, targetNamespaceBytes.Length)
Some { Kind = import.Kind; Name = name }
| _ -> None ] |> List.filter Option.isSome |> List.map Option.get
if expectedScope.Length <> imports.Length then
failwith $"Expected imports amount is {expectedScope.Length}, but got {imports.Length}\nExpected:\n%A{expectedScope}\nActual:%A{imports}"
if expectedScope <> imports then
failwith $"Expected imports are different from PDB.\nExpected:\n%A{expectedScope}\nActual:%A{imports}"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

F# Compiler and Tooling
Status: New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider emitting CompilationOptions into Portable PDB

2 participants

@baronfel @vzarytovskii