Avoid virtual implementations for AddSynthesizedAttributes
#81289
Conversation
| foreach (var tp in oldTypeParameters) | ||
| { | ||
| var newTp = synthesized ? | ||
| TypeParameterSymbol newTp = synthesized ? |
There was a problem hiding this comment.
I see, the types no longer inherit from each other, a target type is necessary.
| namespace Microsoft.CodeAnalysis.CSharp.Symbols | ||
| { | ||
| internal class SubstitutedTypeParameterSymbol : WrappedTypeParameterSymbol | ||
| internal sealed class SubstitutedTypeParameterSymbol : SubstitutedTypeParameterSymbolBase |
| /// A type parameter for a synthesized class or method. | ||
| /// </summary> | ||
| internal sealed class SynthesizedSubstitutedTypeParameterSymbol : SubstitutedTypeParameterSymbol | ||
| internal sealed class SynthesizedSubstitutedTypeParameterSymbol : SubstitutedTypeParameterSymbolBase |
There was a problem hiding this comment.
I think we should change name of this type. I think it always represents a definition of a synthesized type parameter and "Substituted" in the name adds some confusion. I suggest removing it and use "SynthesizedTypeParameterSymbol" as the name for the class.
There was a problem hiding this comment.
Then adding synthesized attributes would totally make sense for this symbol.
I suggest asserting the following condition in this constructor: Refers to: src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedSubstitutedTypeParameterSymbol.cs:27 in 4b84820. [](commit_id = 4b84820, deletion_comment = False) |
| { | ||
| internal SubstitutedTypeParameterSymbol(Symbol newContainer, TypeMap map, TypeParameterSymbol substitutedFrom, int ordinal) | ||
| : base(newContainer, map, substitutedFrom, ordinal) | ||
| { |
Since we are splitting types, I suggest to keep going and splitting implementation of this method as well by doing the following:
Refers to: src/Compilers/CSharp/Portable/Symbols/SubstitutedTypeParameterSymbol.cs:62 in 4b84820. [](commit_id = 4b84820, deletion_comment = False) |
| } | ||
| } | ||
|
|
||
| internal sealed override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<CSharpAttributeData> attributes) |
| } | ||
|
|
||
| internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<CSharpAttributeData> attributes) | ||
| { |
| } | ||
|
|
||
| protected class RewrittenMethodParameterSymbol : RewrittenParameterSymbol | ||
| protected class RewrittenMethodParameterSymbol : RewrittenMethodParameterSymbolBase |
| } | ||
|
|
||
| internal sealed override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<CSharpAttributeData> attributes) | ||
| { |
| } | ||
|
|
||
| protected class RewrittenMethodParameterSymbol : RewrittenParameterSymbol | ||
| protected class RewrittenMethodParameterSymbol : RewrittenMethodParameterSymbolBase |
|
Done with review pass (commit 4) |
2 participants
Closes #78655