Add Ts of grain classes that extend Grain<T> to consider for serializer generation

[sergeybykov]

This is to help #1060.

Today we generate serializers for grain state classes that extend GrainState. With #1060 any POCO class can represent grain state. This change adds Ts of all classes that extend Grain to consider for serializer generation.

[ReubenBond]

We could probably remove the .IsNested requirement and shift the IsGenericParameter requirement into SerializerGeneratorManager.RecordTypeToGenerate if it isn't there already (it should be in there)

[ReubenBond]

LGTM after addressing the comment about deduplication/simplification. Thanks, Sergey :)

[sergeybykov]

@ReubenBond Good point. I think I addressed it. In the process I believe I also fixed a bug in SerializerGeneratorManager.RecordTypeToGenerate - the missing return false. Without it the change didn't work.

[gabikliot]

22:26:36 Test method Tester.CodeGenTests.RuntimeCodeGenerationTests.RuntimeCodeGenNestedGenericTest threw exception:
22:26:36 System.AggregateException: One or more errors occurred. ---> Orleans.Runtime.OrleansException: Unexpected error serializing message to gateway gwy.tcp://127.0.0.1:40000/0. System.ArgumentException: No serializer found for object of type NestedGeneric<System.Int32>.Nested. Perhaps you need to mark it [Serializable] or define a custom serializer for it?

[sergeybykov]

Hmm. That's why I was afraid to touch this logic. I'll try to step back a bit as the original version worked.

[sergeybykov]

I reverted my "bug fix" and added back an explicit check for type.IsNested in RecordType. The test passes now.

[gabikliot]

But do we actually have a test that it generates the serializer? All state POCO classes in #1060 are marked with [Serializable].

[ReubenBond]

I can't see the broken diff, but did you remove the IsNested check or just move it?
Why does this check exist?

[sergeybykov]

@gabikliot We do have a couple of tests in VSO. They pointed to the issue. Once this is in, I think we should remove [Serializable] from all grain state classes.

@ReubenBond What broken diff?

I removed the code duplication that you pointed to, by moving the shared code to RoslynCodeGenerator.RecordType. I left the check for IsNested there but removed the redundant check for IsGenericParameter which is already done inside SerializerGenerationManager.RecordTypeToGenerate.

When I tried to remove the check for IsNested from RoslynCodeGenerator.RecordType, I ran into codegen issues. It should be possible to remove it. I just didn't have time to dig deeper and didn't want to hold #1060 for that.

[ReubenBond]

Perhaps I'll look into refactoring the logic at another time, when the VSO tests are all ported and easy to run.

[jdom]

missing an else here?

[sergeybykov]

I don't think so. The grain class may be marked as [Serializable], yet it may also have a T (grainStateType) that needs to be recorded. An exotic case but not impossible.