Skip to content
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

EnC: Implements support for #line mappings #53735

Merged
merged 12 commits into from
Jun 4, 2021
Merged

Conversation

tmat
Copy link
Member

@tmat tmat commented May 27, 2021

Implements support for #line and #ExternalSource directives in EnC.

The change includes compiler and IDE changes.

Compiler
Adds new compiler APIs that enumerate all #line-mapped regions in the file.
Adds a few internal extension methods.
Improves implementation of OneOrMany struct.
Fixes a minor issue in VisualBasicDataFlowAnalysis.

Public API tracking issue: #53752

IDE

  1. All code paths in the EnC implementation need to distinguish between unmapped locations (sources that contain #line directives) and mapped locations (files that are targets of #line directives).
  • PDB contains mapped locations and therefore any active statement/instruction locations that we receive from the debugger
    are mapped.

  • The user edits mapped files (e.g. Razor files) hence active tracking spans are created in mapped files.

  • Razor source generator produces unmapped files containing #line directives mapping to .razor files.

    Translating unmapped locations to mapped locations is straightforward - the compiler provides APIs that implement this mapping based on the #line directives in the unmapped file. The reverse is more complicated since we do not know what the source file is that contains #line directive that maps to a given location in an arbitrary file whose path we receive from the PDB. Searching all files in the solution for all #line directives that target the specific file would be slow.

    The implementation is based on an observation that in the cases where we need to reverse map the location we only need to do so for files that were changed wrt the current EnC baseline. For example, when we need to determine the new locations of active statements we assume that only changed files could have changed these locations. We start with setting the new locations to the old ones and then we enumerate all changed unmapped files and their #line mappings to see if any of them affects any of the active statements in question.

  1. Adds Solution.GetTextDocumentAsync and Project.GetTextDocumentAsync APIs that retrieve any regular/additional/configfile/source-generated document.

  2. Implements a new algorithm for calculating line deltas (second commit) The debugger and SymReader provide a feature that allows us to avoid recompiling methods if only the line numbers of sequence points changed. This addressed a very common case - if an edit adds a new line in the middle of a file all spans of sequence points on the lines following this new line must be updated by +1 line delta. Each line delta passed to the debugger is effectively an open-ended range starting at a given line and adding specified delta to all lines following this line. Our implementation now needs to account for #line mappings when calculating these deltas since the sequence points use the mapped locations.

  3. Implements a workaround for Razor design-time/compiler-time views. When EnC diagnostic analyzer is invoked it is invoked a design-time document. We need to translate that document to a corresponding source-generated document in the compile-time solution. The resulting diagnostics then need to be mapped back to the design-time document. This mapping is not perfect since the #lines in design-time and compiler-time documents are not exactly the same and can't currently map columns. This will need to be handled in Razor tooling (@NTaylorMullen).

  4. Finally, changes implemented to support the mappings broke workarounds we have had in the EnC analyzer for some VB AsNew clause, which is a unique initializer syntax that can be shared across multiple field symbols. The change revisits how we map edits to symbols in the analysis and how we represent VB AsNew clause, C# expression-bodied properties and indexers.

Fixes #49631
Fixes #53263
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1342465

Recommended to review IDE changes by looking at each commit separately.

@tmat
Copy link
Member Author

tmat commented May 27, 2021

@davidwengier PTAL

@davidwengier
Copy link
Contributor

            bool IsCurrentSegmentBreakpointSpanMappable()

Local functions above other code? Cyrus would never approve 😛


Refers to: src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs:2029 in 02026b7. [](commit_id = 02026b759c02b9ecbc60987f43db6108f3a2e632, deletion_comment = False)

Copy link
Contributor

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I went cross eyed, but I looked at all of the IDE bits at least.

This was referenced May 28, 2021
@RikkiGibson RikkiGibson self-assigned this Jun 1, 2021
Copy link
Contributor

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had some comments, didn't quite make it through the compilers changes today, but will resume tomorrow.

@tmat
Copy link
Member Author

tmat commented Jun 2, 2021

@RikkiGibson Filed #53838

Copy link
Contributor

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, modulo changes that were made from feedback but not yet pushed

@RikkiGibson RikkiGibson requested a review from a team June 3, 2021 00:39
@RikkiGibson
Copy link
Contributor

@dotnet/roslyn-compiler Please review

@cston
Copy link
Member

cston commented Jun 3, 2021

        var resolver = new TestSourceResolver();

Is this used?


Refers to: src/Compilers/CSharp/Test/Syntax/Diagnostics/LocationsTests.cs:143 in db5b1bd. [](commit_id = db5b1bd, deletion_comment = False)

Copy link
Member

@cston cston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiler changes LGTM.

@tmat tmat merged commit 4458d00 into dotnet:main Jun 4, 2021
@ghost ghost added this to the Next milestone Jun 4, 2021
@tmat tmat deleted the RazorEnc17-main branch June 4, 2021 04:43
@RikkiGibson RikkiGibson modified the milestones: Next, 17.0.P2 Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants