Class <>z__ReadOnlyArray<T> generated for literal collections is not tagged with CompilerGeneratedAttribute

[ndepend]

Class <>z__ReadOnlyArray<T> generated for literal collections is not tagged with System.Runtime.CompilerServices.CompilerGeneratedAttribute. I think it should.

[jnm2]

@cston This wouldn't have spec implications, would it?

[cston]

@jnm2, no spec implications come to mind. Was there some potential implication in particular that you were thinking of?

[jnm2]

@cston No, just asking in order to learn! Thanks. Could this be up-for-grabs then?

[jaredpar]

@jnm2 will let @cston comment if he's okay with someone taking this over or not.

[cston]

I'll mark the issue up for grabs.

For the implementation, it should be sufficient to override AddSynthesizedAttributes() in SynthesizedReadOnlyListTypeSymbol and add the attribute there. For existing examples in the codebase, search for TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor).

I don't think we'll need to add CompilerGeneratedAttribute to any members within those generated types since the entire types will be marked as compiler generated. (For comparison, for anonymous types there is an attribute on the type, but not the members - see sharplab.io.)

[waf]

It should possibly also be tagged with SerializableAttribute similar to List and Array.

[DoctorKrolic]

It should possibly also be tagged with SerializableAttribute similar to List and Array.

[Serializable] is a legacy from the past. Compiler should not bloat IL with this attribute, as it is effectively useless

[FairPlayer4]

Found a small edge case where [Serializable] would be useful.
In ASP.NET (not Core) I used the collection expression with an IReadOnlyList{string} in a class that is stored in the session and got a serialization exception.

Easy fix was making the IReadOnlyList static.

[DoctorKrolic]

In ASP.NET (not Core) I used the collection expression

Collection expressions are a C#12 feature. C#12 is officially supported on .NET 8+. Given that you use ASP.NET (legacy), you are on .NET Framework, therefore your case falls into unsupported territory. I understand, that migrating to modern .NET might be too expensive or in some other way impossible for your project, but that doesn't mean that we should poison new language features with all that many-years-old legacy attributes and so on. Starting with .NET 9 adding [Serializable] will have no impact other than a little bit of additional disk space needed to store the assembly

[jnm2]

@FairPlayer4 In cases where you can't make it static, but you don't want the minimal implementation that the compiler generates, you can also pick your own collection type with an explicit cast, such as IReadOnlyList<string> list = (string[])["..."].

[FairPlayer4]

Please don't change anything in the compiler, I actually wrote my comment so that the unlucky few who encounter this know why it's happening. More as documentation.

In my my opinion the behavior is correct and it should not be expected that an implementation of a Collection Interface has some attribute just because the most common Collection implementations have that attribute. Especially if the attribute is considered obsolete.

@jnm2 That's a good way to handle it, thanks