EnC: Fix symbol mapping of delegates with indexed name

Author: tmat

src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs

@@ -7,13 +7,12 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using Microsoft.CodeAnalysis.CodeGen;
+using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
 using Microsoft.CodeAnalysis.Emit;
-using Microsoft.CodeAnalysis.Emit.EditAndContinue;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Symbols;
 using Roslyn.Utilities;
@@ -27,25 +26,31 @@ internal sealed class CSharpSymbolMatcher : SymbolMatcher
         public CSharpSymbolMatcher(
             SourceAssemblySymbol sourceAssembly,
             SourceAssemblySymbol otherAssembly,
-            SynthesizedTypeMaps synthesizedTypes,
-            IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherSynthesizedMembers,
-            IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherDeletedMembers)
+            SynthesizedTypeMaps otherSynthesizedTypes,
+            IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> otherSynthesizedMembers,
+            IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> otherDeletedMembers)
         {
-            _visitor = new Visitor(sourceAssembly, otherAssembly, synthesizedTypes, otherSynthesizedMembers, otherDeletedMembers, new DeepTranslator(otherAssembly.GetSpecialType(SpecialType.System_Object)));
+            _visitor = new Visitor(
+                sourceAssembly,
+                otherAssembly,
+                otherSynthesizedTypes,
+                otherSynthesizedMembers,
+                otherDeletedMembers,
+                new DeepTranslator(otherAssembly.GetSpecialType(SpecialType.System_Object)));
         }
 
         public CSharpSymbolMatcher(
-            SynthesizedTypeMaps synthesizedTypes,
             SourceAssemblySymbol sourceAssembly,
-            PEAssemblySymbol otherAssembly)
+            PEAssemblySymbol otherAssembly,
+            SynthesizedTypeMaps otherSynthesizedTypes)
         {
             _visitor = new Visitor(
                 sourceAssembly,
                 otherAssembly,
-                synthesizedTypes,
-                otherSynthesizedMembers: null,
-                deepTranslator: null,
-                otherDeletedMembers: null);
+                otherSynthesizedTypes,
+                otherSynthesizedMembers: SpecializedCollections.EmptyReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>(),
+                otherDeletedMembers: SpecializedCollections.EmptyReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>(),
+                deepTranslator: null);
         }
 
         public override Cci.IDefinition? MapDefinition(Cci.IDefinition definition)
@@ -93,9 +98,12 @@ static bool isPrivateImplementationDetail(Cci.IDefinition definition)
         internal bool TryGetAnonymousTypeValue(AnonymousTypeManager.AnonymousTypeOrDelegateTemplateSymbol template, out AnonymousTypeValue typeValue)
             => _visitor.TryGetAnonymousTypeValue(template, out typeValue);
 
+        protected override bool TryGetMatchingDelegateWithIndexedName(INamedTypeSymbolInternal delegateTemplate, ImmutableArray<AnonymousTypeValue> values, out AnonymousTypeValue match)
+            => _visitor.TryGetMatchingDelegateWithIndexedName((AnonymousTypeManager.AnonymousDelegateTemplateSymbol)delegateTemplate, values, out match);
+
         private sealed class Visitor : CSharpSymbolVisitor<Symbol?>
         {
-            private readonly SynthesizedTypeMaps _synthesizedTypes;
+            private readonly SynthesizedTypeMaps _otherSynthesizedTypes;
             private readonly SourceAssemblySymbol _sourceAssembly;
 
             // metadata or source assembly:
@@ -105,9 +113,9 @@ private sealed class Visitor : CSharpSymbolVisitor<Symbol?>
             /// Members that are not listed directly on their containing type or namespace symbol as they were synthesized in a lowering phase,
             /// after the symbol has been created.
             /// </summary>
-            private readonly IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? _otherSynthesizedMembers;
+            private readonly IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> _otherSynthesizedMembers;
 
-            private readonly IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? _otherDeletedMembers;
+            private readonly IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> _otherDeletedMembers;
 
             private readonly SymbolComparer _comparer;
             private readonly ConcurrentDictionary<Symbol, Symbol?> _matches = new(ReferenceEqualityComparer.Instance);
@@ -123,12 +131,12 @@ private sealed class Visitor : CSharpSymbolVisitor<Symbol?>
             public Visitor(
                 SourceAssemblySymbol sourceAssembly,
                 AssemblySymbol otherAssembly,
-                SynthesizedTypeMaps synthesizedTypes,
-                IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherSynthesizedMembers,
-                IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>>? otherDeletedMembers,
+                SynthesizedTypeMaps otherSynthesizedTypes,
+                IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> otherSynthesizedMembers,
+                IReadOnlyDictionary<ISymbolInternal, ImmutableArray<ISymbolInternal>> otherDeletedMembers,
                 DeepTranslator? deepTranslator)
             {
-                _synthesizedTypes = synthesizedTypes;
+                _otherSynthesizedTypes = otherSynthesizedTypes;
                 _sourceAssembly = sourceAssembly;
                 _otherAssembly = otherAssembly;
                 _otherSynthesizedMembers = otherSynthesizedMembers;
@@ -470,45 +478,44 @@ private CustomModifier VisitCustomModifier(CustomModifier modifier)
                     CSharpCustomModifier.CreateRequired(type);
             }
 
-            internal bool TryGetAnonymousDelegateValue(AnonymousTypeManager.AnonymousDelegateTemplateSymbol delegateSymbol, out SynthesizedDelegateValue otherDelegateSymbol)
+            private bool TryGetAnonymousDelegateValue(AnonymousTypeManager.AnonymousDelegateTemplateSymbol delegateSymbol, out SynthesizedDelegateValue otherDelegateSymbol)
             {
-                Debug.Assert((object)delegateSymbol.ContainingSymbol == (object)_sourceAssembly.GlobalNamespace);
-
                 var key = new SynthesizedDelegateKey(delegateSymbol.MetadataName);
-                return _synthesizedTypes.AnonymousDelegates.TryGetValue(key, out otherDelegateSymbol);
+                return _otherSynthesizedTypes.AnonymousDelegates.TryGetValue(key, out otherDelegateSymbol);
             }
 
             internal bool TryGetAnonymousTypeValue(AnonymousTypeManager.AnonymousTypeOrDelegateTemplateSymbol template, out AnonymousTypeValue otherType)
             {
-                Debug.Assert((object)template.ContainingSymbol == (object)_sourceAssembly.GlobalNamespace);
-
                 if (template is AnonymousTypeManager.AnonymousTypeTemplateSymbol typeTemplate)
                 {
-                    return _synthesizedTypes.AnonymousTypes.TryGetValue(typeTemplate.GetAnonymousTypeKey(), out otherType);
+                    return _otherSynthesizedTypes.AnonymousTypes.TryGetValue(typeTemplate.GetAnonymousTypeKey(), out otherType);
                 }
 
                 var delegateTemplate = (AnonymousTypeManager.AnonymousDelegateTemplateSymbol)template;
                 Debug.Assert(delegateTemplate.DelegateInvokeMethod != null);
 
+                otherType = default;
+
                 var key = new AnonymousDelegateWithIndexedNamePartialKey(delegateTemplate.Arity, delegateTemplate.DelegateInvokeMethod.ParameterCount);
-                if (_synthesizedTypes.AnonymousDelegatesWithIndexedNames.TryGetValue(key, out var otherTypeCandidates))
+                return _otherSynthesizedTypes.AnonymousDelegatesWithIndexedNames.TryGetValue(key, out var otherTypeCandidates) &&
+                       TryGetMatchingDelegateWithIndexedName(delegateTemplate, otherTypeCandidates, out otherType);
+            }
+
+            internal bool TryGetMatchingDelegateWithIndexedName(AnonymousTypeManager.AnonymousDelegateTemplateSymbol delegateTemplate, ImmutableArray<AnonymousTypeValue> values, out AnonymousTypeValue match)
+            {
+                foreach (var otherTypeCandidate in values)
                 {
-                    // The key is partial (not unique). Find a matching Invoke method signature.
+                    var otherDelegateType = (NamedTypeSymbol?)otherTypeCandidate.Type.GetInternalSymbol();
+                    Debug.Assert(otherDelegateType is not null);
 
-                    foreach (var otherTypeCandidate in otherTypeCandidates)
+                    if (isCorrespondingAnonymousDelegate(delegateTemplate, otherDelegateType))
                     {
-                        var otherDelegateType = (NamedTypeSymbol?)otherTypeCandidate.Type.GetInternalSymbol();
-                        Debug.Assert(otherDelegateType is not null);
-
-                        if (isCorrespondingAnonymousDelegate(delegateTemplate, otherDelegateType))
-                        {
-                            otherType = otherTypeCandidate;
-                            return true;
-                        }
+                        match = otherTypeCandidate;
+                        return true;
                     }
                 }
 
-                otherType = default;
+                match = default;
                 return false;
 
                 bool isCorrespondingAnonymousDelegate(NamedTypeSymbol type, NamedTypeSymbol otherType)
@@ -526,12 +533,12 @@ otherType.DelegateInvokeMethod is { } otherInvokeMethod &&
                                 x.IsParamsArray == y.IsParamsArray &&
                                 x.IsParamsCollection == y.IsParamsCollection) &&
                         isCorrespondingType(invokeMethod.ReturnTypeWithAnnotations, otherInvokeMethod.ReturnTypeWithAnnotations);
-                }
 
-                bool isCorrespondingType(TypeWithAnnotations type, TypeWithAnnotations expectedType)
-                {
-                    var otherType = type.WithTypeAndModifiers((TypeSymbol?)this.Visit(type.Type), this.VisitCustomModifiers(type.CustomModifiers));
-                    return otherType.Equals(expectedType, TypeCompareKind.CLRSignatureCompareOptions);
+                    bool isCorrespondingType(TypeWithAnnotations type, TypeWithAnnotations expectedType)
+                    {
+                        var otherType = type.WithTypeAndModifiers((TypeSymbol?)this.Visit(type.Type), this.VisitCustomModifiers(type.CustomModifiers));
+                        return otherType.Equals(expectedType, TypeCompareKind.CLRSignatureCompareOptions);
+                    }
                 }
             }
 
@@ -799,12 +806,12 @@ private IReadOnlyDictionary<string, ImmutableArray<ISymbolInternal>> GetAllEmitt
                     members.AddRange(((NamespaceSymbol)symbol).GetMembers());
                 }
 
-                if (_otherSynthesizedMembers != null && _otherSynthesizedMembers.TryGetValue(symbol, out var synthesizedMembers))
+                if (_otherSynthesizedMembers.TryGetValue(symbol, out var synthesizedMembers))
                 {
                     members.AddRange(synthesizedMembers);
                 }
 
-                if (_otherDeletedMembers?.TryGetValue(symbol, out var deletedMembers) == true)
+                if (_otherDeletedMembers.TryGetValue(symbol, out var deletedMembers))
                 {
                     members.AddRange(deletedMembers);
                 }

src/Compilers/CSharp/Portable/Emitter/EditAndContinue/EmitHelpers.cs

@@ -67,14 +67,14 @@ internal static EmitDifferenceResult EmitDifference(
                 var metadataAssembly = (PEAssemblySymbol)metadataDecoder.ModuleSymbol.ContainingAssembly;
 
                 var sourceToMetadata = new CSharpSymbolMatcher(
-                    metadataSymbols.SynthesizedTypes,
-                    sourceAssembly,
-                    metadataAssembly);
+                    sourceAssembly: sourceAssembly,
+                    otherAssembly: metadataAssembly,
+                    otherSynthesizedTypes: metadataSymbols.SynthesizedTypes);
 
                 var previousSourceToMetadata = new CSharpSymbolMatcher(
-                    metadataSymbols.SynthesizedTypes,
-                    previousSourceAssembly,
-                    metadataAssembly);
+                    sourceAssembly: previousSourceAssembly,
+                    otherAssembly: metadataAssembly,
+                    otherSynthesizedTypes: metadataSymbols.SynthesizedTypes);
 
                 CSharpSymbolMatcher? currentSourceToPreviousSource = null;
                 if (baseline.Ordinal > 0)
@@ -84,7 +84,7 @@ internal static EmitDifferenceResult EmitDifference(
                     currentSourceToPreviousSource = new CSharpSymbolMatcher(
                         sourceAssembly: sourceAssembly,
                         otherAssembly: previousSourceAssembly,
-                        baseline.SynthesizedTypes,
+                        otherSynthesizedTypes: baseline.SynthesizedTypes,
                         otherSynthesizedMembers: baseline.SynthesizedMembers,
                         otherDeletedMembers: baseline.DeletedMembers);
                 }
@@ -205,37 +205,40 @@ internal static EmitBaseline MapToCompilation(
             RoslynDebug.AssertNotNull(previousGeneration.PEModuleBuilder);
             RoslynDebug.AssertNotNull(moduleBeingBuilt.EncSymbolChanges);
 
-            var synthesizedTypes = moduleBeingBuilt.GetSynthesizedTypes();
+            var currentSynthesizedTypes = moduleBeingBuilt.GetAllSynthesizedTypes();
             var currentSynthesizedMembers = moduleBeingBuilt.GetAllSynthesizedMembers();
             var currentDeletedMembers = moduleBeingBuilt.EncSymbolChanges.DeletedMembers;
 
             // Mapping from previous compilation to the current.
             var previousSourceAssembly = ((CSharpCompilation)previousGeneration.Compilation).SourceAssembly;
 
             var matcher = new CSharpSymbolMatcher(
-                previousSourceAssembly,
-                compilation.SourceAssembly,
-                synthesizedTypes,
-                currentSynthesizedMembers,
-                currentDeletedMembers);
+                sourceAssembly: previousSourceAssembly,
+                otherAssembly: compilation.SourceAssembly,
+                otherSynthesizedTypes: currentSynthesizedTypes,
+                otherSynthesizedMembers: currentSynthesizedMembers,
+                otherDeletedMembers: currentDeletedMembers);
+
+            var mappedSynthesizedTypes = matcher.MapSynthesizedTypes(previousGeneration.SynthesizedTypes, currentSynthesizedTypes);
 
             var mappedSynthesizedMembers = matcher.MapSynthesizedOrDeletedMembers(previousGeneration.SynthesizedMembers, currentSynthesizedMembers, isDeletedMemberMapping: false);
 
             // Deleted members are mapped the same way as synthesized members, so we can just call the same method.
             var mappedDeletedMembers = matcher.MapSynthesizedOrDeletedMembers(previousGeneration.DeletedMembers, currentDeletedMembers, isDeletedMemberMapping: true);
 
             // TODO: can we reuse some data from the previous matcher?
-            var matcherWithAllSynthesizedMembers = new CSharpSymbolMatcher(
-                previousSourceAssembly,
-                compilation.SourceAssembly,
-                synthesizedTypes,
-                mappedSynthesizedMembers,
-                mappedDeletedMembers);
-
-            return matcherWithAllSynthesizedMembers.MapBaselineToCompilation(
+            var matcherWithAllSynthesizedTypesAndMembers = new CSharpSymbolMatcher(
+                sourceAssembly: previousSourceAssembly,
+                otherAssembly: compilation.SourceAssembly,
+                otherSynthesizedTypes: mappedSynthesizedTypes,
+                otherSynthesizedMembers: mappedSynthesizedMembers,
+                otherDeletedMembers: mappedDeletedMembers);
+
+            return matcherWithAllSynthesizedTypesAndMembers.MapBaselineToCompilation(
                 previousGeneration,
                 compilation,
                 moduleBeingBuilt,
+                mappedSynthesizedTypes,
                 mappedSynthesizedMembers,
                 mappedDeletedMembers);
         }

src/Compilers/CSharp/Portable/Emitter/EditAndContinue/PEDeltaAssemblyBuilder.cs

@@ -224,19 +224,6 @@ private static bool TryGetAnonymousTypeKey(
         internal CSharpDefinitionMap PreviousDefinitions
             => (CSharpDefinitionMap)_changes.DefinitionMap;
 
-        public SynthesizedTypeMaps GetSynthesizedTypes()
-        {
-            var result = new SynthesizedTypeMaps(
-                Compilation.AnonymousTypeManager.GetAnonymousTypeMap(),
-                Compilation.AnonymousTypeManager.GetAnonymousDelegates(),
-                Compilation.AnonymousTypeManager.GetAnonymousDelegatesWithIndexedNames());
-
-            // Should contain all entries in previous generation.
-            Debug.Assert(PreviousGeneration.SynthesizedTypes.IsSubsetOf(result));
-
-            return result;
-        }
-
         public override IEnumerable<Cci.INamespaceTypeDefinition> GetTopLevelTypeDefinitions(EmitContext context)
             => GetTopLevelTypeDefinitionsExcludingNoPiaAndRootModule(context, includePrivateImplementationDetails: true);
 
@@ -258,16 +245,6 @@ internal override MethodInstrumentation GetMethodBodyInstrumentations(MethodSymb
             return _changes.DefinitionMap.GetMethodBodyInstrumentations(method);
         }
 
-        internal override ImmutableArray<AnonymousTypeKey> GetPreviousAnonymousTypes()
-        {
-            return ImmutableArray.CreateRange(PreviousGeneration.SynthesizedTypes.AnonymousTypes.Keys);
-        }
-
-        internal override ImmutableArray<SynthesizedDelegateKey> GetPreviousAnonymousDelegates()
-        {
-            return ImmutableArray.CreateRange(PreviousGeneration.SynthesizedTypes.AnonymousDelegates.Keys);
-        }
-
         internal override int GetNextAnonymousTypeIndex()
             => PreviousGeneration.GetNextAnonymousTypeIndex();
 

src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs

@@ -505,16 +505,6 @@ internal virtual VariableSlotAllocator TryCreateVariableSlotAllocator(MethodSymb
         internal virtual MethodInstrumentation GetMethodBodyInstrumentations(MethodSymbol method)
             => new MethodInstrumentation { Kinds = EmitOptions.InstrumentationKinds };
 
-        internal virtual ImmutableArray<AnonymousTypeKey> GetPreviousAnonymousTypes()
-        {
-            return ImmutableArray<AnonymousTypeKey>.Empty;
-        }
-
-        internal virtual ImmutableArray<SynthesizedDelegateKey> GetPreviousAnonymousDelegates()
-        {
-            return ImmutableArray<SynthesizedDelegateKey>.Empty;
-        }
-
         internal virtual int GetNextAnonymousTypeIndex()
         {
             return 0;