Elimination of reference duplicates in the ReferenceManager should not depend on order

Roslyn.sln

@@ -2748,6 +2748,7 @@ Global
 		{DEB3D675-5A3C-46DA-8945-F2EFAB135EA0} = {62F787B2-1E8B-4A3C-BCC0-0EBAE50B42B7}
 		{E3CD2895-76A8-4D11-A316-EA67CB5EA42C} = {32A48625-F0AD-419D-828B-A50BDABA38EA}
 		{8CE3A581-2969-4864-A803-013E9D977C3A} = {C65C6143-BED3-46E6-869E-9F0BE6E84C37}
+		{6FF42825-5464-4151-AC55-ED828168C192} = {A41D1B99-F489-4C43-BBDF-96D61B19A6B9}
 		{CCBD3438-3E84-40A9-83AD-533F23BCFCA5} = {A41D1B99-F489-4C43-BBDF-96D61B19A6B9}
 		{6FD1CC3E-6A99-4736-9B8D-757992DDE75D} = {38940C5F-97FD-4B2A-B2CD-C4E4EF601B05}
 		{286B01F3-811A-40A7-8C1F-10C9BB0597F7} = {38940C5F-97FD-4B2A-B2CD-C4E4EF601B05}

src/Compilers/Core/CodeAnalysisTest/MetadataReferences/AssemblyIdentityComparerTests.cs

@@ -569,6 +569,28 @@ public void IgnoreOrFwUnifyVersion()
             }
         }
 
+        [Fact]
+        public void AsymmetricUnification()
+        {
+            // Note:
+            // System.Numerics.Vectors, Version=4.0 is an FX assembly
+            // System.Numerics.Vectors, Version=4.1+ is not an FX assembly
+            //
+            // It seems like a bug in fusion: it only determines whether the definition is an FX assembly 
+            // and calculates the result based upon that, regardless of whether the reference is an FX assembly or not.
+            // We do replicate that behavior.
+            TestMatch(
+                "System.Numerics.Vectors, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a",
+                "System.Numerics.Vectors, Version=4.1.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a",
+                AssemblyIdentityComparer.ComparisonResult.NotEquivalent);
+
+            TestMatch(
+                "System.Numerics.Vectors, Version=4.1.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a",
+                "System.Numerics.Vectors, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a",
+                AssemblyIdentityComparer.ComparisonResult.Equivalent,
+                unificationApplied: true);
+        }
+
         [Fact]
         public void Portability()
         {

src/Compilers/Core/Portable/Desktop/DesktopAssemblyIdentityComparer.cs

@@ -81,13 +81,13 @@ internal AssemblyPortabilityPolicy PortabilityPolicy
             ref AssemblyIdentity reference,
             ref AssemblyIdentity definition,
             AssemblyIdentityParts referenceParts,
-            out bool isFxAssembly)
+            out bool isDefinitionFxAssembly)
         {
             if (reference.ContentType == AssemblyContentType.Default &&
                 SimpleNameComparer.Equals(reference.Name, definition.Name) &&
                 SimpleNameComparer.Equals(reference.Name, "mscorlib"))
             {
-                isFxAssembly = true;
+                isDefinitionFxAssembly = true;
                 reference = definition;
                 return true;
             }
@@ -96,7 +96,7 @@ internal AssemblyPortabilityPolicy PortabilityPolicy
             {
                 // Reference is not retargetable, but definition is retargetable.
                 // Non-equivalent.
-                isFxAssembly = false;
+                isDefinitionFxAssembly = false;
                 return false;
             }
 
@@ -113,7 +113,7 @@ internal AssemblyPortabilityPolicy PortabilityPolicy
             {
                 if (!AssemblyIdentity.IsFullName(referenceParts))
                 {
-                    isFxAssembly = false;
+                    isDefinitionFxAssembly = false;
                     return false;
                 }
 
@@ -139,11 +139,11 @@ internal AssemblyPortabilityPolicy PortabilityPolicy
 
             if (reference.IsRetargetable && definition.IsRetargetable)
             {
-                isFxAssembly = IsRetargetableAssembly(definition);
+                isDefinitionFxAssembly = IsRetargetableAssembly(definition);
             }
             else
             {
-                isFxAssembly = IsFrameworkAssembly(definition);
+                isDefinitionFxAssembly = IsFrameworkAssembly(definition);
             }
 
             return true;

src/Compilers/Core/Portable/MetadataReference/AssemblyIdentityComparer.cs

@@ -113,8 +113,8 @@ internal ComparisonResult Compare(AssemblyIdentity reference, string referenceDi
 
             Debug.Assert(reference.ContentType == definition.ContentType);
 
-            bool isFxAssembly;
-            if (!ApplyUnificationPolicies(ref reference, ref definition, parts, out isFxAssembly))
+            bool isDefinitionFxAssembly;
+            if (!ApplyUnificationPolicies(ref reference, ref definition, parts, out isDefinitionFxAssembly))
             {
                 return ComparisonResult.NotEquivalent;
             }
@@ -151,7 +151,7 @@ internal ComparisonResult Compare(AssemblyIdentity reference, string referenceDi
                     return ComparisonResult.Equivalent;
                 }
 
-                isFxAssembly = false;
+                isDefinitionFxAssembly = false;
             }
 
             if (!SimpleNameComparer.Equals(reference.Name, definition.Name))
@@ -177,7 +177,16 @@ internal ComparisonResult Compare(AssemblyIdentity reference, string referenceDi
                 hasSomeVersionParts &&
                 (hasPartialVersion || reference.Version != definition.Version))
             {
-                if (isFxAssembly)
+                // Note:
+                // System.Numerics.Vectors, Version=4.0 is an FX assembly
+                // System.Numerics.Vectors, Version=4.1+ is not an FX assembly
+                //
+                // It seems like a bug in Fusion: it only determines whether the definition is an FX assembly 
+                // and calculates the result based upon that, regardless of whether the reference is an FX assembly or not.
+                // We do replicate the behavior.
+                //
+                // As a result unification is asymmetric when comparing the above identities.
+                if (isDefinitionFxAssembly)
                 {
                     unificationApplied = true;
                     return ComparisonResult.Equivalent;
@@ -211,9 +220,9 @@ internal ComparisonResult Compare(AssemblyIdentity reference, string referenceDi
             return AssemblyIdentity.MemberwiseEqual(x, y);
         }
 
-        internal virtual bool ApplyUnificationPolicies(ref AssemblyIdentity reference, ref AssemblyIdentity definition, AssemblyIdentityParts referenceParts, out bool isFxAssembly)
+        internal virtual bool ApplyUnificationPolicies(ref AssemblyIdentity reference, ref AssemblyIdentity definition, AssemblyIdentityParts referenceParts, out bool isDefinitionFxAssembly)
         {
-            isFxAssembly = false;
+            isDefinitionFxAssembly = false;
             return true;
         }
     }

src/Compilers/Core/Portable/ReferenceManager/CommonReferenceManager.Resolution.cs

@@ -584,8 +584,13 @@ private static void AddModule(PEModule module, int referenceIndex, ResolvedRefer
             {
                 foreach (var other in sameSimpleNameIdentities)
                 {
-                    // only compare strong with strong (weak is never equivalent to strong and vice versa)
-                    if (other.Identity.IsStrongName && IdentityComparer.ReferenceMatchesDefinition(identity, other.Identity))
+                    // Only compare strong with strong (weak is never equivalent to strong and vice versa).
+                    // In order to eliminate duplicate references we need to try to match their identities in both directions since 
+                    // ReferenceMatchesDefinition is not neccessarily symmetric.
+                    // (e.g. System.Numerics.Vectors, Version=4.1+ matches System.Numerics.Vectors, Version=4.0, but not the other way around.)
+                    if (other.Identity.IsStrongName && 
+                        IdentityComparer.ReferenceMatchesDefinition(identity, other.Identity) &&
+                        IdentityComparer.ReferenceMatchesDefinition(other.Identity, identity))
                     {
                         equivalent = other;
                         break;

src/Compilers/Test/Resources/Core/CompilerTestResources.csproj

@@ -411,6 +411,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="ResourceLoader.cs" />
+    <Compile Include="TestKeys.cs" />
     <Compile Include="TestResources.cs" />
   </ItemGroup>
   <ItemGroup>
@@ -420,4 +421,4 @@
     <Import Project="..\..\..\..\..\build\Targets\VSL.Imports.targets" />
     <Import Project="$(SolutionDir)\.nuget\NuGet.targets" Condition="Exists('$(SolutionDir)\.nuget\NuGet.targets')" />
   </ImportGroup>
-</Project>
+</Project>
\ No newline at end of file

src/Compilers/Test/Resources/Core/TestKeys.cs

+using System.Collections.Immutable;
+
+namespace TestResources
+{
+    public static class TestKeys
+    {
+        public static readonly ImmutableArray<byte> PublicKey_ce65828c82a341f2 = ImmutableArray.Create(new byte[] 
+        {
+            0x00, 0x24, 0x00, 0x00, 0x04, 0x80, 0x00, 0x00, 0x94, 0x00, 0x00, 0x00, 0x06, 0x02, 0x00, 0x00,
+            0x00, 0x24, 0x00, 0x00, 0x52, 0x53, 0x41, 0x31, 0x00, 0x04, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00,
+            0x2B, 0x98, 0x6F, 0x6B, 0x5E, 0xA5, 0x71, 0x7D, 0x35, 0xC7, 0x2D, 0x38, 0x56, 0x1F, 0x41, 0x3E,
+            0x26, 0x70, 0x29, 0xEF, 0xA9, 0xB5, 0xF1, 0x07, 0xB9, 0x33, 0x1D, 0x83, 0xDF, 0x65, 0x73, 0x81,
+            0x32, 0x5B, 0x3A, 0x67, 0xB7, 0x58, 0x12, 0xF6, 0x3A, 0x94, 0x36, 0xCE, 0xCC, 0xB4, 0x94, 0x94,
+            0xDE, 0x8F, 0x57, 0x4F, 0x8E, 0x63, 0x9D, 0x4D, 0x26, 0xC0, 0xFC, 0xF8, 0xB0, 0xE9, 0xA1, 0xA1,
+            0x96, 0xB8, 0x0B, 0x6F, 0x6E, 0xD0, 0x53, 0x62, 0x8D, 0x10, 0xD0, 0x27, 0xE0, 0x32, 0xDF, 0x2E,
+            0xD1, 0xD6, 0x08, 0x35, 0xE5, 0xF4, 0x7D, 0x32, 0xC9, 0xEF, 0x6D, 0xA1, 0x0D, 0x03, 0x66, 0xA3,
+            0x19, 0x57, 0x33, 0x62, 0xC8, 0x21, 0xB5, 0xF8, 0xFA, 0x5A, 0xBC, 0x5B, 0xB2, 0x22, 0x41, 0xDE,
+            0x6F, 0x66, 0x6A, 0x85, 0xD8, 0x2D, 0x6B, 0xA8, 0xC3, 0x09, 0x0D, 0x01, 0x63, 0x6B, 0xD2, 0xBB,
+        });
+
+        public static readonly ImmutableArray<byte> PublicKey_b03f5f7f11d50a3a = ImmutableArray.Create(new byte[]
+        {
+            0x00, 0x24, 0x00, 0x00, 0x04, 0x80, 0x00, 0x00, 0x94, 0x00, 0x00, 0x00, 0x06, 0x02, 0x00, 0x00,
+            0x00, 0x24, 0x00, 0x00, 0x52, 0x53, 0x41, 0x31, 0x00, 0x04, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00,
+            0x07, 0xd1, 0xfa, 0x57, 0xc4, 0xae, 0xd9, 0xf0, 0xa3, 0x2e, 0x84, 0xaa, 0x0f, 0xae, 0xfd, 0x0d,
+            0xe9, 0xe8, 0xfd, 0x6a, 0xec, 0x8f, 0x87, 0xfb, 0x03, 0x76, 0x6c, 0x83, 0x4c, 0x99, 0x92, 0x1e,
+            0xb2, 0x3b, 0xe7, 0x9a, 0xd9, 0xd5, 0xdc, 0xc1, 0xdd, 0x9a, 0xd2, 0x36, 0x13, 0x21, 0x02, 0x90,
+            0x0b, 0x72, 0x3c, 0xf9, 0x80, 0x95, 0x7f, 0xc4, 0xe1, 0x77, 0x10, 0x8f, 0xc6, 0x07, 0x77, 0x4f,
+            0x29, 0xe8, 0x32, 0x0e, 0x92, 0xea, 0x05, 0xec, 0xe4, 0xe8, 0x21, 0xc0, 0xa5, 0xef, 0xe8, 0xf1,
+            0x64, 0x5c, 0x4c, 0x0c, 0x93, 0xc1, 0xab, 0x99, 0x28, 0x5d, 0x62, 0x2c, 0xaa, 0x65, 0x2c, 0x1d,
+            0xfa, 0xd6, 0x3d, 0x74, 0x5d, 0x6f, 0x2d, 0xe5, 0xf1, 0x7e, 0x5e, 0xaf, 0x0f, 0xc4, 0x96, 0x3d,
+            0x26, 0x1c, 0x8a, 0x12, 0x43, 0x65, 0x18, 0x20, 0x6d, 0xc0, 0x93, 0x34, 0x4d, 0x5a, 0xd2, 0x93,
+        });
+    }
+}
\ No newline at end of file

src/Compilers/VisualBasic/Test/Semantic/Compilation/ReferenceManagerTests.vb

@@ -11,10 +11,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UnitTests
         Inherits BasicTestBase
 
         Private Shared ReadOnly s_signedDll As VisualBasicCompilationOptions =
-            New VisualBasicCompilationOptions(OutputKind.DynamicallyLinkedLibrary,
-                                              optimizationLevel:=OptimizationLevel.Release,
-                                              cryptoKeyFile:=SigningTestHelpers.KeyPairFile,
-                                              strongNameProvider:=New SigningTestHelpers.VirtualizedStrongNameProvider(ImmutableArray.Create(Of String)()))
+            TestOptions.ReleaseDll.WithCryptoPublicKey(TestResources.TestKeys.PublicKey_ce65828c82a341f2)
 
         <WorkItem(5483, "DevDiv_Projects/Roslyn")>
         <WorkItem(527917, "DevDiv")>