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

Fix missing incremental generator caching #38

Closed

Conversation

3schwartz
Copy link

Hi,

First of all thank you for a awesome post (and i general great) post regarding incremental generators.

I was reading this blog and the main documentation and both describes one shouldn't combine with the whole compilation, since this will trigger the flow of every key stroke in the IDE.

As an example, create a new library ExampleGenerator and have ExampleGenerator.csproj look like

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
	  <TargetFramework>netstandard2.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
	  <LangVersion>Latest</LangVersion>
  </PropertyGroup>

	<ItemGroup>
		<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.3" PrivateAssets="all" />
		<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.2.0" PrivateAssets="all" />
	</ItemGroup>
	
</Project>

Create file ExampleGenerator.cs with the following incremental generator

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
using System.Collections.Immutable;
using System.Text;

namespace ExampleGenerator
{
    [Generator]
    public class ExampleGenerator : IIncrementalGenerator
    {
        private static int counter;

        public void Initialize(IncrementalGeneratorInitializationContext context)
        {
            context.RegisterPostInitializationOutput(context => context.AddSource(
                "ExampleExtensionAttribute.g.cs", SourceText.From(@"
namespace Example
{
    [System.AttributeUsage(System.AttributeTargets.Enum)]
    public class ExampleExtensionAttribute : System.Attribute
    {}
}", Encoding.UTF8)));

            var classProvider = context.SyntaxProvider
                .CreateSyntaxProvider(
                    static (node, _) => node is EnumDeclarationSyntax m && m.AttributeLists.Count > 0,
                    static (c, _) => GetEnumDeclarationSyntax(c))
                .Where(static e => e is not null)
                .Collect()
                .Combine(context.CompilationProvider);

            context.RegisterSourceOutput(classProvider,
                static (spc, source) => Execute(source.Left, source.Right, spc));
        }

        private static void Execute(ImmutableArray<EnumDeclarationSyntax?> classes, Compilation compilation, SourceProductionContext context)
        {
            foreach (var c in classes)
            {
                context.AddSource($"Example.{c.Identifier.Text}.cs", $@"
// Counter: {Interlocked.Increment(ref counter)}

namespace Example
{{
   partial class {c.Identifier.Text}
   {{
   }}
}}
");
            }
        }

        private static EnumDeclarationSyntax? GetEnumDeclarationSyntax(GeneratorSyntaxContext context)
        {
            var enumDeclarationSyntax = (EnumDeclarationSyntax)context.Node;

            foreach (AttributeListSyntax attributeListSyntax in enumDeclarationSyntax.AttributeLists)
            {
                foreach (AttributeSyntax attributeSyntax in attributeListSyntax.Attributes)
                {
                    if (context.SemanticModel.GetSymbolInfo(attributeSyntax).Symbol is not IMethodSymbol attributeSymbol)
                    {
                        continue;
                    }

                    INamedTypeSymbol attributeContainingTypeSymbol = attributeSymbol.ContainingType;
                    string fullName = attributeContainingTypeSymbol.ToDisplayString();

                    if (fullName == "Example.ExampleExtensionAttribute")
                    {
                        return enumDeclarationSyntax;
                    }
                }
            }
            return null;
        }
    }
}

Thanks to Pawel Gerr for the counter idea and you for example how find enums with attributes.

Now create a console project ExampleConsole and have ExampleConsole.csproj look like

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\ExampleGenerator\ExampleGenerator.csproj"
					      ReferenceOutputAssembly="false"
                          OutputItemType="Analyzer" />
  </ItemGroup>

</Project>

Create three files with enums, two which have the attribute.

SomeOther.cs

namespace ExampleConsole
{
    internal enum SomeOther
    {
    }
}

Bar.cs

namespace ExampleConsole
{
    [ExampleExtension]
    internal enum Bar
    {
    }
}

Foo.cs

namespace ExampleConsole
{
    [ExampleExtension]
    internal enum Foo
    {
    }
}

Now if you do ANY changes to any file, you can see in Dependencies->Analyzers->ExampleGenerator that both Example.Bar.cs and Example.Foo.cs have their counter increased.

The best solution would be to avoid the .Collect() and just provide IncrementalValuesProvider instead of a IncrementalValueProvider. Since you are already creating a new file for each syntax, the change is just to avoid the loop.

Hope you have the time to look at my PR - please let me now any comments and thoughts.

@andrewlock
Copy link
Owner

Thanks for flagging this, I really need to understand it properly - have you flagged this to the .NET team in https://github.com/dotnet/runtime? Because if this is the case, I think every single Source Generator currently shipping (and due to ship in .NET 7) has this problem 😬

@3schwartz
Copy link
Author

Thanks for flagging this, I really need to understand it properly - have you flagged this to the .NET team in https://github.com/dotnet/runtime? Because if this is the case, I think every single Source Generator currently shipping (and due to ship in .NET 7) has this problem 😬

Yes, I have made the following issue dotnet/runtime#74557, and PR dotnet/runtime#74558.

From comments in the PR it sounds like it is a well known issue - and from the comments it seems like the comparer shouldn't be used but instead the logic you have in the execute step should be moved to the predicate.

@3schwartz
Copy link
Author

Hi @andrewlock

So looking at this comment dotnet/runtime#64579 (comment) and dotnet/runtime#68353 (comment), one clearly shouldn't use combine with compilation and use custom comparer.

Looking at how implemented in this PR dotnet/runtime#64579 code in predicate extract the exact information needed and gives this to the output.
See code at https://github.com/teo-tsirpanis/dotnet-runtime/tree/ee127b9e3fd2a2a356b6bbea1c60181a658d1648/src/libraries/System.Private.CoreLib/gen.

I have refactored the PR such that it follows the same principles - hence moved code from Execute(..) up in predicate and made sure caching is used.

You can validate the cache is indeed used correctly by doing the following changes in EnumGenerator

[Generator]
public partial class EnumGenerator : IIncrementalGenerator
{
    private static int counter;

    ...

    static void Execute(EnumToGenerate? enumToGenerate, SourceProductionContext context)
    {
        if (enumToGenerate is EnumToGenerate eg && enumToGenerate != null)
        {
            StringBuilder sb = new();
            var result = SourceGenerationHelper.GenerateExtensionClass(sb, eg, Interlocked.Increment(ref counter));
            context.AddSource(eg.Name + "_EnumExtensions.g.cs", SourceText.From(result, Encoding.UTF8));
        }
    }
    
    ...
}

and following in SourceGenerationHelper

public static class SourceGenerationHelper
{
    ...
    public static string GenerateExtensionClass(StringBuilder sb, EnumToGenerate enumToGenerate, int counter)
    {
        sb
            .Append(Header)
            .Append(@"
#if NETCOREAPP && !NETCOREAPP2_0 && !NETCOREAPP1_1 && !NETCOREAPP1_0
using System;
#endif
// Counter ").Append(counter).Append(@"
");
        ...
    }
}

now if you create a new console project, create some enums with the attribute and reference the generator project you can see it increment correctly, and only when changes to the respective enum has been made (when looking in dependencies->analysers->..).

Hope you have the time for a review :-)

@andrewlock
Copy link
Owner

Hey @3schwartz, sorry for the delay, I've been very busy so the OSS has taken a back seat! I took a look at the new API in .NET 7, and I think there's a better approach to this now: the PR is here if you'd like to take a look 🙂 #41

@andrewlock
Copy link
Owner

Hopefully fixed in #41, but let me know if you think I've missed something!

@andrewlock andrewlock closed this Dec 17, 2022
@3schwartz
Copy link
Author

Hopefully fixed in #41, but let me know if you think I've missed something!

Agree :-)

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

2 participants