0

I've taken a stab at writing an incremental source generator; it is generating the correct source code, but it's not doing so incrementally. I feel like it has to be something wrong with my Initialize method or my custom return type (ClassInfo) not being cache friendly. I've never written an IEquatable either, so I really thing it has something to do with that.

ClassInfo

public readonly struct ClassInfo : IEquatable<ClassInfo>
{
   public readonly string? Namespace { get; }
   public readonly string Name { get; }
   public readonly ImmutableArray<IPropertySymbol> PropertyNames { get; }

   public ClassInfo(ITypeSymbol type)
   {
      Namespace = type.ContainingNamespace.IsGlobalNamespace ? null : type.ContainingNamespace.ToDisplayString();
      Name = type.Name;
      PropertyNames = GetPropertyNames(type);
   }

   private static ImmutableArray<IPropertySymbol> GetPropertyNames(ITypeSymbol type)
   {
      return type.GetMembers()
         .Select(m =>
            {
               // Only properties
               if (m is not IPropertySymbol prop || m.DeclaredAccessibility != Accessibility.Public)
                  return null;

               // Without ignore attribute
               if (GenHelper.IsPropsToStringIgnore(m))
                  return null;

               return (IPropertySymbol)m;
               //return SymbolEqualityComparer.Default.Equals(prop.Type, type) ? prop.Name : null;
            })
         .Where(m => m != null)!
         .ToImmutableArray<IPropertySymbol>();
   }

   public override bool Equals(object? obj) => obj is ClassInfo other && Equals(other);

   public bool Equals(ClassInfo other)
   {
      if (ReferenceEquals(null, other))
         return false;
      //if (ReferenceEquals(this, other))
      //   return true;

      return Namespace == other.Namespace
         && Name == other.Name
         && PropertyNames.SequenceEqual(other.PropertyNames); //  <-- Problem Line
   }

   public override int GetHashCode()
   {
      var hashCode = (Namespace != null ? Namespace.GetHashCode() : 0);
      hashCode = (hashCode * 397) ^ Name.GetHashCode();
      hashCode = (hashCode * 397) ^ PropertyNames.GetHashCode(); //  <-- Problem Line

      return hashCode;
   }
}

IncrementalGenerator.Initialize

public void Initialize(IncrementalGeneratorInitializationContext context)
{
  context.RegisterPostInitializationOutput(ctx =>
  {
     ctx.AddSource("PropsToStringAttribute.g.cs", SourceText.From(AttributeTexts.PropsToStringAttribute, Encoding.UTF8));
     ctx.AddSource("PropToStringAttribute.g.cs", SourceText.From(AttributeTexts.PropToStringAttribute, Encoding.UTF8));
     ctx.AddSource("PropsToStringIgnoreAttribute.g.cs", SourceText.From(AttributeTexts.PropsToStringIgnoreAttribute, Encoding.UTF8));
  });

  
  var classProvider = context.SyntaxProvider
      .CreateSyntaxProvider(
          static (node, _) => node is ClassDeclarationSyntax { AttributeLists.Count: > 0 },
          static (ctx, ct) => GetClassInfoOrNull(ctx, ct)
          )
      .Where(type => type is not null)
      .Collect()
      .SelectMany((classes, _) => classes.Distinct());

  context.RegisterSourceOutput(classProvider, Generate);

}

GetClassInfoOrNull

internal static ClassInfo? GetClassInfoOrNull(GeneratorSyntaxContext context, CancellationToken cancellationToken)
{
  // We know the node is a ClassDeclarationSyntax thanks to IsSyntaxTargetForGeneration
  var classDeclarationSyntax = (ClassDeclarationSyntax)context.Node;

  var type = ModelExtensions.GetDeclaredSymbol(context.SemanticModel, classDeclarationSyntax, cancellationToken) as ITypeSymbol;

  return IsPropsToString(type) ? new ClassInfo(type!) : null;
}

IsPropsToString

public static bool IsPropsToString(ISymbol? type)
{
   return type is not null && 
          type.GetAttributes()
              .Any(a => a.AttributeClass is
              {
                 Name: ClassAttributeName,
                 ContainingNamespace:
                 {
                    Name: PTSNamespace,
                    ContainingNamespace.IsGlobalNamespace: true
                 }
              });
}

IsPropsToStringIgnore

public static bool IsPropsToStringIgnore(ISymbol type)
{
   return type is not null && 
          type.GetAttributes()
              .Any(a => a.AttributeClass is
              {
                 Name: PropertyIgnoreAttributeName,
                 ContainingNamespace:
                 {
                    Name: PTSNamespace,
                    ContainingNamespace.IsGlobalNamespace: true
                 }
              });
}

As a side note, I mostly followed this https://www.thinktecture.com/en/net/roslyn-source-generators-performance/

Edit 9/2/22

I have narrowed down the problem to two lines of code noted above in ClassInfo.Equals and ClassInfo.GetHashCode; the two lines that deal with equating the array of names. I commented out those two lines and started to get incremental code generation. However, I wasn't getting new code generation when properties changes (as espected), I instead had to change the name of the class(es) to get new code generated (again, as expected).

Edit 9/7/22 Added project to GitHub

Edit 9/8/22 I tried not using SequenceEquals to compare my PropertyNames array, but it didnt work.

public bool Equals(ClassInfo other)
{
   if (PropertyNames.Count() != other.PropertyNames.Count())
      return false;
      
   int i = 0;
   bool propIsEqual = true;

   while (propIsEqual && i < PropertyNames.Count())
   {
      propIsEqual &= SymbolEqualityComparer.Default.Equals(PropertyNames[i], other.PropertyNames[i]);
      i++;
   }

   return Namespace == other.Namespace
      && Name == other.Name
      && propIsEqual;
      //PropertyNames.SequenceEqual(other.PropertyNames); //  <-- Problem Line
}
master_ruko
  • 629
  • 1
  • 4
  • 22
  • Just an FYI. It might take me a day to respond to and questions/comments/concerns because of my sh** work schedule. Thank you in advance for any help you may provide! – master_ruko Aug 30 '22 at 15:42
  • Please [edit] your question to include your source code as a working [mcve], which can be compiled and tested by others. – Progman Sep 05 '22 at 15:48
  • If you want to debug a source generator, just add `if (!Debugger.IsAttached) Debugger.Launch();` to the beginning of your generator code and debug the generator process from Visual Studio 2022. – Simon Mourier Sep 06 '22 at 17:52
  • Please add your code to the question itself, not on an external site. – Progman Sep 07 '22 at 15:09
  • I don't see this project on github (I see only Button_IsEnabled_Issue). Is the repo public? – Olivier Jacot-Descombes Sep 08 '22 at 15:07
  • @OlivierJacot-Descombes It is now. Sorry, that was my bad. – master_ruko Sep 08 '22 at 15:09

1 Answers1

3

You declared ClassInfo as struct, so it makes no sense to use ReferenceEquals in your Equals implementation. It will box the struct and always return other references.

public bool Equals(ClassInfo other)
{
    return Namespace == other.Namespace
        && Name == other.Name
        && PropertyNames.SequenceEqual(other.PropertyNames);
}

PropertyNames.GetHashCode() only gets the hash code of the array object but does not include the array items; however it should, as Equals does it by calling SequenceEqual. The explicit IStructuralEquatable implementation of GetHashCode in ImmutableArray<T> does what we need.

public override int GetHashCode()
{
    unchecked {
        int hashCode = Namespace != null ? Namespace.GetHashCode() : 0;
        hashCode = (hashCode * 397) ^ Name.GetHashCode();
        hashCode = (hashCode * 397) ^
            ((IStructuralEquatable)PropertyNames)
               .GetHashCode(EqualityComparer<IPropertySymbol>.Default);
        return hashCode;
    }
}

The IPropertySymbol implementation(s) must override the Equals and the GetHashCode methods as well. This is also important for SequenceEqual to work as expected.

Edit 22/09/08

We can remove uncertainties about mysterious equality comparers by comparing the properties ourselves:

public bool Equals(ClassInfo other)
{
    // Length is faster than the extension method Count()
    if (PropertyNames.Length != other.PropertyNames.Length) {
        return false;
    }

    for (int i = 0; i < PropertyNames.Length; i++) {
        if (PropertyNames[i].Name != other.PropertyNames[i].Name) {
            return false; // We don't need to do the other tests.
        }
    }

    return Namespace == other.Namespace && Name == other.Name;
}

Note: In your test project the "Counter = x" now gets incremented when we change a property name, but not if we add e.g. a comment.

Consequently, we do the same for GetHashCode (see also What is the best algorithm for overriding GetHashCode? and especially Jon Skeet's answer):

public override int GetHashCode()
{
    unchecked {
        int hash = 17;
        hash = hash * 23 + (Namespace ?? "").GetHashCode();
        hash = hash * 23 + Name.GetHashCode();
        foreach (IPropertySymbol property in PropertyNames) {
            hash = hash * 23 + property.Name.GetHashCode();
        }

        return hash;
    }
}
Olivier Jacot-Descombes
  • 104,806
  • 13
  • 138
  • 188
  • Thank you for your help. I made the changes you suggested and understand them, but don't know what to do with the last statement. The changes to ```Equals``` and ```GetHashCode``` alone did not fix the issue. Also, in case you want/need to see more code, I posted my project to github (link in question). – master_ruko Sep 07 '22 at 15:24
  • `PropertyNames` is of type `ImmutableArray`. So, the items in this array must override `Equals` and `GetHashCode` to make `PropertyNames.SequenceEqual(other.PropertyNames)` and my `GetHashCode` work. – Olivier Jacot-Descombes Sep 07 '22 at 15:37
  • But the items in ```PropertyNames``` are members of a ```TypeSymbol``` objects being passed to the ```ClassInfo``` constructor by a ```SyntaxProvider```. How do I override ```Equals``` and ```GetHashCode``` for that? Do I make an extension class for ```IProbertySymbol``` and override them in there, or make a custom class that inherits from ```IPropertySymbol``` and override in there, or is there something else? – master_ruko Sep 07 '22 at 16:48
  • Some research tell me that the [IPropertySymbol Interface](https://learn.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.ipropertysymbol?view=roslyn-dotnet-4.3.0) implements `IEquatable`. So, these symbols should be okay. – Olivier Jacot-Descombes Sep 08 '22 at 10:16
  • I looked at the definition of ```ISymbol``` in VS and say that it implements ```IEquatable``` so I assumed that I wouldn't need to do it myself. Thats why I was confused by the last statement in your answer. That being said, your answer doesn't fix my problem. Is there something else I should be using besides ```SequenceEqual```? Or maybe I should just loop through ```PropertyNames``` and do the equality check myself? – master_ruko Sep 08 '22 at 13:50
  • I tried manually comparing each element of ```PropertyNames```, but that didn't work either. I added that code as an edit to the question. – master_ruko Sep 08 '22 at 14:57
  • Now, I am comparing the property names explicitly and it seems to work as expected. – Olivier Jacot-Descombes Sep 08 '22 at 16:37
  • Yeah, I tried that to and got a successful result, but the the name isn't the only thing I want to generate on. What I have in the ```Generate``` method is just something simple I ht – master_ruko Sep 08 '22 at 16:49
  • What I have in the ```Generate``` method is just something simple that I put together to test if the generation was working; not the final version of my generation code. I figured having an array of ```IPropertySymbol```s was better than pulling all the individual pieces I need in the transform stage. I am trying to keep ```ClassInfo``` construction as simple as possible inorder to keep the ```SyntaxProvider``` creation as fast as possible. – master_ruko Sep 08 '22 at 16:55
  • It seems that the [SymbolEqualityComparer](https://github.com/dotnet/roslyn/blob/3e39dd3535962bf9e30bd650e4ff34b610b8349a/src/Compilers/CSharp/Portable/Symbols/SymbolEqualityComparer.cs#L48) just uses reference equality (not sure, so). So, different compilation passes might return different references for the same symbols. – Olivier Jacot-Descombes Sep 09 '22 at 11:59
  • You can also compare `prop.Type.Name` and `prop.DeclaredAccessibility` for instance. – Olivier Jacot-Descombes Sep 09 '22 at 15:00
  • Ok, so, then I can't just do a general comparison of the properties. I'll have to figure something else out, because I wanted to have a property attribute that has parameters to alter what code is generated for each property. – master_ruko Sep 09 '22 at 15:48
  • Thank you for taking the time to work through this with me. – master_ruko Sep 09 '22 at 15:54