Flesh out handling of symbols synthesized during lowering of extension methods #77287
Conversation
AlekseyTs
added
Area-Compilers
Feature - Extension Everything
dotnet-issue-labeler
bot
added
the
untriaged
| @@ -1039,6 +1039,8 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r | |||
| // The async state machine type is not synthesized until the async method body is rewritten. If we are | |||
| // only emitting metadata the method body will not have been rewritten, and the async state machine | |||
| // type will not have been created. In this case, omit the attribute. | |||
|
|
|||
| // PROTOTYPE: These attributes probably should be synthesized only on the extension implementation method | |||
| if (moduleBuilder.CompilationState.TryGetStateMachineType(MethodCompiler.TryGetCorrespondingExtensionImplementationMethod(this) ?? (MethodSymbol)this, out NamedTypeSymbol stateMachineType)) | |||
There was a problem hiding this comment.
Looks like TryGetCorrespondingExtensionImplementationMethod is no longer used inside MethodCompiler, so perhaps it shouldn't live there? #Resolved
There was a problem hiding this comment.
perhaps it shouldn't live there?
I like its current location and at this point we don't even know whether the method will survive at the end.
| @@ -2090,5 +2090,29 @@ internal bool TryGetTranslatedImports(ImportChain chain, out ImmutableArray<Cci. | |||
| { | |||
| return _translatedImportsMap.GetOrAdd(chain, imports); | |||
| } | |||
|
|
|||
| public override void AddSynthesizedDefinition(NamedTypeSymbol container, Cci.INestedTypeDefinition nestedType) | |||
There was a problem hiding this comment.
Consider marking as sealed override here and below. #Resolved
There was a problem hiding this comment.
I see no reason to seal these overrides.
| internal DelegateCacheContainer(TypeSymbol containingType, int generationOrdinal) | ||
| : base(GeneratedNames.DelegateCacheContainerType(generationOrdinal), containingMethod: null) | ||
| internal DelegateCacheContainer(NamedTypeSymbol containingType, int generationOrdinal) | ||
| : base(GeneratedNames.DelegateCacheContainerType(generationOrdinal), []) | ||
| { | ||
| Debug.Assert(containingType.IsDefinition); | ||
|
|
||
| _containingSymbol = containingType; | ||
| } | ||
|
|
||
| /// <summary>Creates a method-scope generic delegate cache container.</summary> |
| @@ -22,7 +22,7 @@ internal sealed class DelegateCacheRewriter | |||
| private readonly SyntheticBoundNodeFactory _factory; | |||
| private readonly int _topLevelMethodOrdinal; | |||
|
|
|||
| private Dictionary<MethodSymbol, DelegateCacheContainer>? _genericCacheContainers; | |||
| private Dictionary<Symbol, DelegateCacheContainer>? _genericCacheContainers; | |||
There was a problem hiding this comment.
Consider updating the doc comment of DelegateCacheRewriter since it doesn't target just methods anymore. #Resolved
There was a problem hiding this comment.
Consider updating the doc comment of
DelegateCacheRewritersince it doesn't target just methods anymore.
Could you, please, elaborate? I think the purpose of the rewriter and what it targets isn't changed in this PR.
There was a problem hiding this comment.
You are right, I have misinterpreted the doc comment.
|
|
||
| _containingSymbol = ownerMethod.ContainingType; | ||
| _containingSymbol = containingType; | ||
| _constructedContainer = Construct(ConstructedFromTypeParameters); | ||
| } | ||
|
|
There was a problem hiding this comment.
Is the PROTOTYPE comment below still necessary? #Resolved
|
@jcouv, @dotnet/roslyn-compiler For the second review |
1 similar comment
|
@jcouv, @dotnet/roslyn-compiler For the second review |
Relates to test plan #76130