Improve Portable PDB debugging in FSharp.Private.Compiler.dll #6383
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dsyme
approved these changes
Mar 28, 2019
| // printf "reduce %d\n" prod | ||
| let redResult = reduction parseState | ||
| valueStack.Push(ValueInfo(redResult, lhsPos.[0], lhsPos.[1])) | ||
| try |
There was a problem hiding this comment.
The diff could be made minimal, though it's no big deal, the other changes are an improvement
There was a problem hiding this comment.
I know, but my OCD doesn't work that way :-(
src/absil/ilwritepdb.fs
Outdated
|
|
||
| if i < 1 || offsetDelta <> 0 then // ILOffset must not be 0 | ||
| builder.WriteCompressedInteger(offsetDelta) | ||
|
|
||
| if sps.[i].Line = 0xfeefee && sps.[i].EndLine = 0xfeefee then // Hidden-sequence-point-record | ||
| let capLine v = if v < 0 || v >= 0x20000000 then 0xfeefee else v |
There was a problem hiding this comment.
The use of 0xfeefee here is very odd. FEEFEE has a special meaning for a hidden sequence point https://blogs.msdn.microsoft.com/jmstall/2005/06/19/line-hidden-and-0xfeefee-sequence-points/. I think this should be something else.
There was a problem hiding this comment.
Since the data is invalid, I can throw the sequence point away instead of writing a hidden sequence point.
Lets see what that looks like.
brettfo
reviewed
Mar 28, 2019
This was referenced Mar 30, 2019
KevinRansom
pushed a commit
that referenced
this pull request
Mar 30, 2019
#6383 seems to have fixed it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Whilst Portable PDB's seem to work well for most projects. The FSharp.Private.Compiler project, and I expect the FSharp.Compiler.Service dll have an abysmal portable pdb debugging experience.
Fsi on the coreclr can AV when generating code under certain circumstances when generating exception stacks for generated code, due to errors in the portable pdb file.
The F# build generates Desktop PDBs for FSharp.Private.Compiler.dll rather than creating portable pdbs and converting to desktop pdbs because the pdb converter
The underlying cause of all of these issues is that during the reduction phase of parsing the compiler merges ranges to compute the source code range for a reduced expression.
It turns out that parser presents a stack of expressions for reductions that may exist in different source files. The merge took no account that the range could exist over multiple files.
Part of the fix here is to pin the end of the range and to discard source ranges in other files here:
https://github.com/Microsoft/visualfsharp/compare/master...KevinRansom:symbols?expand=1#diff-b328b19055c7a49c80ee3f8ae4c8c536R376.
A separate but related issue is that SynExpr.Paren can have a value for leftParenRange, and rightParenRange in different files. The fix here is to change the Range calculation such that if they are in different files, it uses the leftParenRange as Range.
Additionally this PR improves the value validation when creating the Portable PDB file, to match limits in the portable PDB specs.
And re-enable portable pdb generation.
@dsyme, Please take a look at this, and let me know if there is a better approach.