Avoid calling GetDiagnostics on compilation during EnC analysis #19167
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
tmat
commented
May 2, 2017
| @@ -4416,7 +4416,7 @@ void F() | |||
| } | |||
|
|
|||
| // Add corresponding test to VB | |||
| [WpfFact(Skip = "TODO")] | |||
| [Fact(Skip = "TODO")] | |||
There was a problem hiding this comment.
Needs change #18940 to go in first.
There was a problem hiding this comment.
Could you please change the skip string to "https://github.com/dotnet/roslyn/pull/18940"
tmat
commented
May 2, 2017
| @@ -4313,6 +4323,76 @@ public void Insert_ExternConstruct() | |||
| Diagnostic(RudeEditKind.InsertExtern, "public extern C()", FeaturesResources.constructor)); | |||
| } | |||
|
|
|||
| [Fact(Skip = "TODO")] | |||
|
|
ivanbasov
approved these changes
May 2, 2017
gafter
approved these changes
May 3, 2017
| @@ -4416,7 +4416,7 @@ void F() | |||
| } | |||
|
|
|||
| // Add corresponding test to VB | |||
| [WpfFact(Skip = "TODO")] | |||
| [Fact(Skip = "TODO")] | |||
There was a problem hiding this comment.
Could you please change the skip string to "https://github.com/dotnet/roslyn/pull/18940"
| @@ -4313,6 +4323,76 @@ public void Insert_ExternConstruct() | |||
| Diagnostic(RudeEditKind.InsertExtern, "public extern C()", FeaturesResources.constructor)); | |||
| } | |||
|
|
|||
| [Fact(Skip = "TODO")] | |||
|
@MattGertz For ask mode approval. |
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.
Customer scenario
Applying an EnC change in a project containing large method bodies might take a long time (minutes) during which the compiler calculates all semantic diagnostics and VS UI thread is blocked.
Bugs this fixes:
#10683
Workarounds, if any
None.
Risk
Medium. Although I went over the EnC analyzer code to find any code paths that might not be hardened against erroneous code it might happen I missed something. As a result an exception may occur, a non-fatal Watson reported and the debugging session terminated.
Performance impact
Improves performance.
Is this a regression from a previous update?
Roslyn regressed when compared to pre-Roslyn EnC.
Root cause analysis:
We used safer approach of not analyzing broken code when the analyzer was implemented.
How was the bug found?
Quite a few customer complains.