Partial properties: duplicate membersByName before merging accessors (#…

…74969)
dotnet · Sep 3, 2024 · 4619ed7 · 4619ed7
1 parent c6a0795
commit 4619ed7
Showing 1 changed file with 18 additions and 31 deletions.
diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs
@@ -3668,12 +3668,7 @@ void mergePartialMethods(ref Dictionary<ReadOnlyMemory<char>, ImmutableArray<Sym
                 }
                 else
                 {
-                    if ((object)membersByName == _lazyEarlyAttributeDecodingMembersDictionary)
-                    {
-                        // Avoid mutating the cached dictionary and especially avoid doing this possibly on multiple threads in parallel.
-                        membersByName = new Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>>(membersByName, ReadOnlyMemoryOfCharComparer.Instance);
-                    }
-
+                    DuplicateMembersByNameIfCached(ref membersByName);
                     membersByName[name] = FixPartialMember(membersByName[name], prevMethod, currentMethod);
                 }
             }
@@ -3692,40 +3687,23 @@ void mergePartialProperties(ref Dictionary<ReadOnlyMemory<char>, ImmutableArray<
                 }
                 else
                 {
-                    var (currentGet, prevGet) = ((SourcePropertyAccessorSymbol?)currentProperty.GetMethod, (SourcePropertyAccessorSymbol?)prevProperty.GetMethod);
-                    if (currentGet != null || prevGet != null)
-                    {
-                        var accessorName = (currentGet ?? prevGet)!.Name.AsMemory();
-                        mergeAccessors(ref membersByName, accessorName, currentGet, prevGet);
-                    }
-
-                    var (currentSet, prevSet) = ((SourcePropertyAccessorSymbol?)currentProperty.SetMethod, (SourcePropertyAccessorSymbol?)prevProperty.SetMethod);
-                    if (currentSet != null || prevSet != null)
-                    {
-                        var accessorName = (currentSet ?? prevSet)!.Name.AsMemory();
-                        mergeAccessors(ref membersByName, accessorName, currentSet, prevSet);
-                    }
-
-                    if ((object)membersByName == _lazyEarlyAttributeDecodingMembersDictionary)
-                    {
-                        // Avoid mutating the cached dictionary and especially avoid doing this possibly on multiple threads in parallel.
-                        membersByName = new Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>>(membersByName, ReadOnlyMemoryOfCharComparer.Instance);
-                    }
-
+                    DuplicateMembersByNameIfCached(ref membersByName);
+                    mergeAccessors(ref membersByName, (SourcePropertyAccessorSymbol?)currentProperty.GetMethod, (SourcePropertyAccessorSymbol?)prevProperty.GetMethod);
+                    mergeAccessors(ref membersByName, (SourcePropertyAccessorSymbol?)currentProperty.SetMethod, (SourcePropertyAccessorSymbol?)prevProperty.SetMethod);
                     membersByName[name] = FixPartialMember(membersByName[name], prevProperty, currentProperty);
                 }
 
-                void mergeAccessors(ref Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>> membersByName, ReadOnlyMemory<char> name, SourcePropertyAccessorSymbol? currentAccessor, SourcePropertyAccessorSymbol? prevAccessor)
+                void mergeAccessors(ref Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>> membersByName, SourcePropertyAccessorSymbol? currentAccessor, SourcePropertyAccessorSymbol? prevAccessor)
                 {
-                    Debug.Assert(currentAccessor != null || prevAccessor != null);
-                    if (currentAccessor != null && prevAccessor != null)
+                    if (currentAccessor is { } && prevAccessor is { })
                     {
+                        var name = currentAccessor.Name.AsMemory();
                         var implementationAccessor = currentProperty.IsPartialDefinition ? prevAccessor : currentAccessor;
                         membersByName[name] = Remove(membersByName[name], implementationAccessor);
                     }
-                    else
+                    else if (currentAccessor is { } || prevAccessor is { })
                     {
-                        var (foundAccessor, containingProperty, otherProperty) = prevAccessor != null ? (prevAccessor, prevProperty, currentProperty) : (currentAccessor!, currentProperty, prevProperty);
+                        var (foundAccessor, containingProperty, otherProperty) = prevAccessor is { } ? (prevAccessor, prevProperty, currentProperty) : (currentAccessor!, currentProperty, prevProperty);
                         // When an accessor is present on definition but not on implementation, the accessor is said to be missing on the implementation.
                         // When an accessor is present on implementation but not on definition, the accessor is said to be unexpected on the implementation.
                         var (errorCode, propertyToBlame) = foundAccessor.IsPartialDefinition
@@ -3737,6 +3715,15 @@ void mergeAccessors(ref Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>>
             }
         }
 
+        private void DuplicateMembersByNameIfCached(ref Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>> membersByName)
+        {
+            if ((object)membersByName == _lazyEarlyAttributeDecodingMembersDictionary)
+            {
+                // Avoid mutating the cached dictionary and especially avoid doing this possibly on multiple threads in parallel.
+                membersByName = new Dictionary<ReadOnlyMemory<char>, ImmutableArray<Symbol>>(membersByName, ReadOnlyMemoryOfCharComparer.Instance);
+            }
+        }
+
         /// <summary>Links together the definition and implementation parts of a partial method. Returns a member list which has the implementation part removed.</summary>
         private static ImmutableArray<Symbol> FixPartialMember(ImmutableArray<Symbol> symbols, SourceOrdinaryMethodSymbol part1, SourceOrdinaryMethodSymbol part2)
         {