Support Roslyn 3.8 and Roslyn 4.0 source generator scenarios #1216
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
Contributor
sharwell
commented
Aug 7, 2021
•
sharwell
commented
- Split InterfaceStubGenerator.Core into one project for Roslyn 3.8 and a second project for Roslyn 4.0
- Update packaging to include both source generators in the refit package
- Add build logic to selectively include the Roslyn 4.0 source generator for design-time scenarios in Visual Studio 2022
- Design time builds in Visual Studio 2019 use the Roslyn 3.8 implementation.
- Design time builds in Visual Studio 2022 use the Roslyn 4.0 implementation.
- All command line builds use the Roslyn 3.8 implementation. Prior to Add generator driver cache to VBCSCompiler dotnet/roslyn#55023, there is no significant performance penalty for using the Roslyn 3.8 implementation of the source generator. We can decide how to handle command line scenarios later.
- Add tests verify the V1 and V2 implementations produce the same output
- Convert the V2 implementation to use incremental generation instead of SyntaxReceiver
- As a separate change, show how the generator can improve incrementality by collecting an intermediate value to its own value source
sharwell
force-pushed
the
multi-target
branch
3 times, most recently
from
847fc26
to
2422617
Compare
|
Is this ready to merge? |
|
@clairernovotny as best I can tell, yes. Whether or not you merge now, the next step of testing is getting some sort of beta package into a public feed so it can be tested in a few real-world cases (both inside the IDE and from command lines). |
eerhardt
reviewed
Aug 12, 2021
eerhardt
reviewed
Aug 16, 2021
| @@ -4,4 +4,17 @@ | |||
| <CompilerVisibleProperty Include="RefitInternalNamespace" /> | |||
| </ItemGroup> | |||
|
|
|||
| <Choose> | |||
| <When Condition="'$(VisualStudioVersion)' >= '17.0' AND '$(DesignTimeBuild)' == 'True'"> | |||
There was a problem hiding this comment.
Check out dotnet/msbuild#4911. Sorry - In my previous reply I didn't have time to look it up.
Suggested change
| <When Condition="'$(VisualStudioVersion)' >= '17.0' AND '$(DesignTimeBuild)' == 'True'"> | |
| <When Condition="$([MSBuild]::VersionGreaterThanOrEquals($(VisualStudioVersion), '17.0')) AND '$(DesignTimeBuild)' == 'True'"> |
|
COOL... is this available on a preview feed somewhere? I'd like to test it ASAP 😁 |
|
Refit's CI feed is here: https://pkgs.dev.azure.com/dotnet/ReactiveUI/_packaging/Refit/nuget/v3/index.json |
|
Awesome, thank you for that, @clairernovotny. Unfortunately I appear to be running into an issue here. Is there a configuration setting I should be applying, perhaps? Any link/resources you can send would be greatly appreciated. I do not see anything obvious here in this PR. Here is what I see when I try to build with the preview build: Interesting to note that the reported directories If I am not mistaken, it appears it's attempting to use both generators? I've tried clearing all |
|
@Mike-E-angelo thanks for checking this out. It looks like NuGet is including everything under |
|
Doing some spelunking here @sharwell. 😁 I updated And deleted the Did a clean and rebuild. I still get the following: So it appears it is only v2 now but still some duplication going on. |
|
@Mike-E-angelo Try also removing this line: It looks like NuGet is including the items in roslyn40, and then this line is including them again. |
|
That worked @sharwell! Thank you. I see what you mean now. That's weird it's including them implicitly. It would seem that it would be an explicit thing such as how you have it with the In any case, there is indeed a new champ in the CPU Heat Sweepstakes. 😆 Check it out: This is V1: So according to dotTrace it went from ~36% CPU time down to 6%.. a 6x differential. DANGGGG. Nice work there. :) |
|
@Mike-E-angelo if you change the folder from |
|
WHEW... this is tricky to figure out @sharwell. Despite my best attempts, it seems as if anything in the |
|
Hey all... this does not appear to be working with Shouldn't the Refit source generator be listed alongside the |
|
I found the problem. Let me create a new issue to track. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.