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

Collect incremental state without passing compilation to output stage #3452

Merged
merged 3 commits into from Apr 20, 2022

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Feb 8, 2022

No description provided.

@@ -30,22 +30,70 @@ private enum NodeKind

public void Initialize(IncrementalGeneratorInitializationContext context)
{
var compilation = context.CompilationProvider;
var referencedAssemblies = context.CompilationProvider.SelectMany(
Copy link
Member Author

Choose a reason for hiding this comment

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

@chsienki this is IncrementalValuesProvider<IAssemblySymbol>, which should be efficient provided the IAssemblySymbol for a metadata reference is carried forward from one compilation to the next. Do you know if this is true for the IDE and/or command line?

Choose a reason for hiding this comment

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

Yes, they should be carried forward. There is also context.MetadataReferences which provides the same thing. You don't get the IAssemblySymbol directly, but if you new up a compilation with the reference you can retrieve it, and the input will then never change unless the user adjusts references.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll switch over to context.MetadataReferences after 4.2.0 is released (this is the version that corrects the signature for this property).

{
}

private record ExistingTypeData(string TypeName, ImmutableArray<string> MemberNames)
Copy link
Member Author

Choose a reason for hiding this comment

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

@chsienki is there a straightforward way to have a properly equatable list of strings in a record, without having to implement custom equality?

@@ -1393,5 +1441,20 @@ public string GetAccessorResultType(SyntaxData syntaxData)
return null;
}
}

private record CompilationData(ImmutableDictionary<string, ExistingTypeData> ExistingTypes)
Copy link
Member Author

Choose a reason for hiding this comment

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

@chsienki is there a straightforward way to have a properly equatable dictionary in a record, without having to implement custom equality?

@codecov
Copy link

codecov bot commented Feb 8, 2022

Codecov Report

Merging #3452 (253d408) into master (6cd35b5) will decrease coverage by 0.27%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3452      +/-   ##
==========================================
- Coverage   93.23%   92.95%   -0.28%     
==========================================
  Files        1063     1063              
  Lines      113033   136647   +23614     
  Branches     3980     7071    +3091     
==========================================
+ Hits       105383   127020   +21637     
- Misses       6630     8421    +1791     
- Partials     1020     1206     +186     

#nullable enable

// NOTE: This code is derived from an implementation originally in dotnet/runtime:
// https://github.com/dotnet/runtime/blob/v5.0.3/src/libraries/System.Private.CoreLib/src/System/HashCode.cs

Choose a reason for hiding this comment

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

Could you reference Microsoft.Bcl.HashCode instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to avoid dependencies in the source generator, since it makes build integration much more complex.

@sharwell sharwell marked this pull request as ready for review April 20, 2022 02:20
@sharwell sharwell merged commit 9c5d064 into DotNetAnalyzers:master Apr 20, 2022
@sharwell sharwell deleted the incremental-state branch April 20, 2022 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants