-
Notifications
You must be signed in to change notification settings - Fork 4k
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.
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
Include source files w/o method bodies in the PDB documents #39136
Changes from 3 commits
d26e2ef
68bb295
a4cef18
7b98102
a200fc8
25ee210
c08e2a2
eaab9f3
28c8b82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -759,7 +759,7 @@ private DocumentHandle GetOrAddDocument(DebugSourceDocument document, Dictionary | |
/// This is done after serializing method debug info to ensure that we embed all requested | ||
/// text even if there are no corresponding sequence points. | ||
/// </remarks> | ||
public void AddRemainingEmbeddedDocuments(IEnumerable<DebugSourceDocument> documents) | ||
public void AddRemainingEmbeddedDocuments(ImmutableArray<DebugSourceDocument> documents) | ||
{ | ||
foreach (var document in documents) | ||
{ | ||
|
@@ -768,6 +768,21 @@ public void AddRemainingEmbeddedDocuments(IEnumerable<DebugSourceDocument> docum | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Add document entries for all debug documents that does not yet have an entry. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
typo: |
||
/// </summary> | ||
/// <remarks> | ||
/// This is done after serializing method debug info to ensure that we embed all requested | ||
/// text even if there are no corresponding sequence points. | ||
/// </remarks> | ||
public void AddRemainingDebugDocuments(ImmutableArray<DebugSourceDocument> documents) | ||
{ | ||
foreach (var document in documents) | ||
{ | ||
GetOrAddDocument(document, _documentIndex); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need two methods? They seem to be doing the same thing. #Resolved |
||
|
||
#endregion | ||
|
||
#region Edit and Continue | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,7 @@ internal static bool WritePeToStream( | |
} | ||
|
||
nativePdbWriterOpt.WriteRemainingEmbeddedDocuments(mdWriter.Module.DebugDocumentsBuilder.EmbeddedDocuments); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary anymore - embedded sources are added to the set of all debug documents on the document builder in Also, once we remove this call we can remove |
||
nativePdbWriterOpt.WriteRemainingDebugDocuments(mdWriter.Module.DebugDocumentsBuilder.GetAllDebugDocuments()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this boxes the |
||
} | ||
|
||
Stream peStream = getPeStream(); | ||
|
@@ -152,6 +153,7 @@ internal static bool WritePeToStream( | |
if (mdWriter.EmitPortableDebugMetadata) | ||
{ | ||
mdWriter.AddRemainingEmbeddedDocuments(mdWriter.Module.DebugDocumentsBuilder.EmbeddedDocuments); | ||
mdWriter.AddRemainingDebugDocuments(mdWriter.Module.DebugDocumentsBuilder.GetAllDebugDocuments()); | ||
|
||
// The algorithm must be specified for deterministic builds (checked earlier). | ||
Debug.Assert(!isDeterministic || context.Module.PdbChecksumAlgorithm.Name != null); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will result in non-deterministic PDB. We need to order the documents somehow. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also optimize this to avoid allocations and extra work. Return IReadOnlyDictionary from this method. Enumerate the dictionary in AddRemainingDebugDocuments to filter out documents that are already indexed. Sort the remaining and add them.
In reply to: 333275492 [](ancestors = 333275492)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I just think that filtering is not necessary because we add only item not yet added.
In reply to: 333278515 [](ancestors = 333278515,333275492)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filtering is an optimization, so that we don't need to sort all the documents upfront just to realize we only need to add a few. #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me try
In reply to: 333303087 [](ancestors = 333303087)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @tmat! Updated.
In reply to: 333303897 [](ancestors = 333303897,333303087)