Avoid rebinding references bound to explicitly specified definitions, even if...

Avoid rebinding references bound to explicitly specified definitions, even if a better version is available after resolution

src/Compilers/CSharp/Portable/Symbols/ReferenceManager.cs

@@ -8,11 +8,11 @@
 using Microsoft.CodeAnalysis.CSharp.Emit;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
+using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp
 {
-    using Symbols.Retargeting;
     using MetadataOrDiagnostic = System.Object;
 
     public partial class CSharpCompilation
@@ -307,7 +307,6 @@ private void InitializeAssemblyReuseData(AssemblySymbol assemblySymbol, Immutabl
             private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
             {
                 var resolutionDiagnostics = DiagnosticBag.GetInstance();
-                var implicitlyResolvedReferences = ArrayBuilder<MetadataReference>.GetInstance();
 
                 try
                 {
@@ -327,29 +326,36 @@ private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
                         resolutionDiagnostics);
 
                     var assemblyBeingBuiltData = new AssemblyDataForAssemblyBeingBuilt(new AssemblyIdentity(name: SimpleAssemblyName), referencedAssemblies, modules);
-                    var allAssemblyData = referencedAssemblies.Insert(0, assemblyBeingBuiltData);
+                    var explicitAssemblyData = referencedAssemblies.Insert(0, assemblyBeingBuiltData);
 
                     // Let's bind all the references and resolve missing one (if resolver is available)
                     bool hasCircularReference;
                     int corLibraryIndex;
+                    ImmutableArray<MetadataReference> implicitlyResolvedReferences;
+                    ImmutableArray<ResolvedReference> implicitlyResolvedReferenceMap;
+                    ImmutableArray<AssemblyData> allAssemblyData;
+
                     BoundInputAssembly[] bindingResult = Bind(
-                        ref allAssemblyData,
+                        explicitAssemblyData,
                         compilation.Options.MetadataReferenceResolver,
                         compilation.Options.MetadataImportOptions,
-                        implicitlyResolvedReferences,
+                        out allAssemblyData,
+                        out implicitlyResolvedReferences,
+                        out implicitlyResolvedReferenceMap,
                         resolutionDiagnostics,
                         out hasCircularReference,
                         out corLibraryIndex);
 
                     Debug.Assert(bindingResult.Length == allAssemblyData.Length);
 
+                    references = references.AddRange(implicitlyResolvedReferences);
+                    referenceMap = referenceMap.AddRange(implicitlyResolvedReferenceMap);
+
                     Dictionary<MetadataReference, int> referencedAssembliesMap, referencedModulesMap;
                     ImmutableArray<ImmutableArray<string>> aliasesOfReferencedAssemblies;
                     BuildReferencedAssembliesAndModulesMaps(
                         references,
                         referenceMap,
-                        implicitlyResolvedReferences,
-                        referencedAssemblies.Length,
                         modules.Length,
                         out referencedAssembliesMap,
                         out referencedModulesMap,
@@ -394,12 +400,15 @@ private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
                     // This should be done after we created/found all AssemblySymbols 
                     Dictionary<AssemblyIdentity, MissingAssemblySymbol> missingAssemblies = null;
 
+                    // -1 for assembly being built:
+                    int totalReferencedAssemblyCount = allAssemblyData.Length - 1;
+
                     // Setup bound references for newly created SourceAssemblySymbol
                     ImmutableArray<ModuleReferences<AssemblySymbol>> moduleReferences;
                     SetupReferencesForSourceAssembly(
-                        allAssemblyData,
                         assemblySymbol,
                         modules,
+                        totalReferencedAssemblyCount,
                         bindingResult,
                         ref missingAssemblies,
                         out moduleReferences);
@@ -461,7 +470,6 @@ private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
                 finally
                 {
                     resolutionDiagnostics.Free();
-                    implicitlyResolvedReferences.Free();
                 }
             }
 
@@ -678,9 +686,9 @@ private static void UpdateSymbolCacheNoLock(List<int> newSymbols, ImmutableArray
             }
 
             private static void SetupReferencesForSourceAssembly(
-                ImmutableArray<AssemblyData> allAssemblyData,
                 SourceAssemblySymbol sourceAssembly,
                 ImmutableArray<PEModule> modules,
+                int totalReferencedAssemblyCount,
                 BoundInputAssembly[] bindingResult,
                 ref Dictionary<AssemblyIdentity, MissingAssemblySymbol> missingAssemblies,
                 out ImmutableArray<ModuleReferences<AssemblySymbol>> moduleReferences)
@@ -693,8 +701,7 @@ private static void UpdateSymbolCacheNoLock(List<int> newSymbols, ImmutableArray
                 int refsUsed = 0;
                 for (int moduleIndex = 0; moduleIndex < moduleSymbols.Length; moduleIndex++)
                 {
-                    // -1 for assembly being built
-                    int refsCount = (moduleIndex == 0) ? (allAssemblyData.Length - 1) : modules[moduleIndex - 1].ReferencedAssemblies.Length;
+                    int refsCount = (moduleIndex == 0) ? totalReferencedAssemblyCount : modules[moduleIndex - 1].ReferencedAssemblies.Length;
 
                     var identities = new AssemblyIdentity[refsCount];
                     var symbols = new AssemblySymbol[refsCount];
@@ -731,7 +738,7 @@ private static void UpdateSymbolCacheNoLock(List<int> newSymbols, ImmutableArray
                     refsUsed += refsCount;
                 }
 
-                moduleReferences = moduleReferencesBuilder.AsImmutableOrEmpty();
+                moduleReferences = moduleReferencesBuilder.ToImmutableOrEmptyAndFree();
             }
 
             private static AssemblySymbol GetAssemblyDefinitionSymbol(

src/Compilers/Core/Portable/Collections/ArrayBuilderExtensions.cs

@@ -139,5 +139,10 @@ public static T Peek<T>(this ArrayBuilder<T> builder)
         {
             return builder[builder.Count - 1];
         }
+
+        public static ImmutableArray<T> ToImmutableOrEmptyAndFree<T>(this ArrayBuilder<T> builderOpt)
+        {
+            return builderOpt?.ToImmutableAndFree() ?? ImmutableArray<T>.Empty;
+        }
     }
 }

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

@@ -106,7 +106,7 @@ public int Index
 
             private string GetDebuggerDisplay()
             {
-                return IsSkipped ? "<skipped>" : (_kind == MetadataImageKind.Assembly ? "A[" : "M[") + Index + "]: aliases=" + _aliases.ToString();
+                return IsSkipped ? "<skipped>" : $"{(_kind == MetadataImageKind.Assembly ? "A" : "M")}[{Index}]: aliases='{string.Join("','", _aliases)}'";
             }
         }
 
@@ -344,7 +344,7 @@ public ReferencedAssemblyIdentity(AssemblyIdentity identity, MetadataReference m
                         : ((object)modulesBuilder == null ? 0 : modulesBuilder.Count);
 
                     int reversedIndex = count - 1 - referenceMap[i].Index;
-                    referenceMap[i] = new ResolvedReference(reversedIndex, referenceMap[i].Kind, GetAliases(references[i], aliasMap));
+                    referenceMap[i] = new ResolvedReference(reversedIndex, referenceMap[i].Kind, GetAndFreeAliases(references[i], aliasMap));
                 }
             }
 
@@ -371,10 +371,10 @@ public ReferencedAssemblyIdentity(AssemblyIdentity identity, MetadataReference m
             return ImmutableArray.CreateRange(referenceMap);
         }
 
-        private static ImmutableArray<string> GetAliases(MetadataReference reference, Dictionary<MetadataReference, ArrayBuilder<string>> aliasMap)
+        private static ImmutableArray<string> GetAndFreeAliases(MetadataReference reference, Dictionary<MetadataReference, ArrayBuilder<string>> aliasMapOpt)
         {
             ArrayBuilder<string> aliases;
-            if (aliasMap != null && aliasMap.TryGetValue(reference, out aliases))
+            if (aliasMapOpt != null && aliasMapOpt.TryGetValue(reference, out aliases))
             {
                 return aliases.ToImmutableAndFree();
             }
@@ -576,7 +576,7 @@ private static void AddModule(PEModule module, int referenceIndex, ResolvedRefer
         // Returns null if an assembly of an equivalent identity has not been added previously, otherwise returns the reference that added it.
         // - Both assembly names are strong (have keys) and are either equal or FX unified 
         // - Both assembly names are weak (no keys) and have the same simple name.
-        private MetadataReference TryAddAssembly(AssemblyIdentity identity, MetadataReference boundReference, DiagnosticBag diagnostics, Location location,
+        private MetadataReference TryAddAssembly(AssemblyIdentity identity, MetadataReference boundReference, DiagnosticBag diagnosticsOpt, Location location,
             ref Dictionary<string, List<ReferencedAssemblyIdentity>> referencesBySimpleName)
         {
             if (referencesBySimpleName == null)
@@ -643,7 +643,7 @@ private static void AddModule(PEModule module, int referenceIndex, ResolvedRefer
                     // BREAKING CHANGE in VB: we report an error for both languages
 
                     // Multiple assemblies with equivalent identity have been imported: '{0}' and '{1}'. Remove one of the duplicate references.
-                    MessageProvider.ReportDuplicateMetadataReferenceStrong(diagnostics, location, boundReference, identity, equivalent.MetadataReference, equivalent.Identity);
+                    MessageProvider.ReportDuplicateMetadataReferenceStrong(diagnosticsOpt, location, boundReference, identity, equivalent.MetadataReference, equivalent.Identity);
                 }
                 // If the versions match exactly we ignore duplicates w/o reporting errors while 
                 // Dev12 C# reports:
@@ -664,7 +664,7 @@ private static void AddModule(PEModule module, int referenceIndex, ResolvedRefer
 
                 if (identity != equivalent.Identity)
                 {
-                    MessageProvider.ReportDuplicateMetadataReferenceWeak(diagnostics, location, boundReference, identity, equivalent.MetadataReference, equivalent.Identity);
+                    MessageProvider.ReportDuplicateMetadataReferenceWeak(diagnosticsOpt, location, boundReference, identity, equivalent.MetadataReference, equivalent.Identity);
                 }
             }
 
@@ -840,6 +840,9 @@ private static PortableExecutableReference ResolveReferenceDirective(string refe
                         }
 
                         continue;
+
+                    default:
+                        throw ExceptionUtilities.Unreachable;
                 }
             }
 
@@ -909,28 +912,5 @@ private static PortableExecutableReference ResolveReferenceDirective(string refe
 
             return new AssemblyReferenceBinding(reference);
         }
-
-        private static bool IsBetterVersionMatch(AssemblyIdentity reference, ImmutableArray<AssemblyData> definitions, AssemblyReferenceBinding previousBinding, AssemblyReferenceBinding newBinding)
-        {
-            Debug.Assert(newBinding.IsBound);
-
-            // Previous binding failed or new binding is an exact match
-            if (!previousBinding.IsBound || newBinding.VersionDifference == 0)
-            {
-                return true;
-            }
-
-            if (previousBinding.VersionDifference != newBinding.VersionDifference)
-            {
-                // lesser than reference version is worst than higher than reference version
-                return previousBinding.VersionDifference < 0;
-            }
-
-            var previousVersion = definitions[previousBinding.DefinitionIndex].Identity.Version;
-            var newVersion = definitions[newBinding.DefinitionIndex].Identity.Version;
-
-            // closer version is better:
-            return (newBinding.VersionDifference > 0) ? newVersion < previousVersion : newVersion > previousVersion;
-        }
     }
 }

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

@@ -383,16 +383,14 @@ internal bool IsBound
         protected static void BuildReferencedAssembliesAndModulesMaps(
             ImmutableArray<MetadataReference> references,
             ImmutableArray<ResolvedReference> referenceMap,
-            IReadOnlyList<MetadataReference> implicitlyResolvedReferences,
-            int referencedAssemblyCount,
             int referencedModuleCount,
             out Dictionary<MetadataReference, int> referencedAssembliesMap,
             out Dictionary<MetadataReference, int> referencedModulesMap,
             out ImmutableArray<ImmutableArray<string>> aliasesOfReferencedAssemblies)
         {
-            referencedAssembliesMap = new Dictionary<MetadataReference, int>(referenceMap.Length + implicitlyResolvedReferences.Count);
+            referencedAssembliesMap = new Dictionary<MetadataReference, int>(referenceMap.Length);
             referencedModulesMap = new Dictionary<MetadataReference, int>(referencedModuleCount);
-            var aliasesOfReferencedAssembliesBuilder = ArrayBuilder<ImmutableArray<string>>.GetInstance(referencedAssemblyCount);
+            var aliasesOfReferencedAssembliesBuilder = ArrayBuilder<ImmutableArray<string>>.GetInstance(referenceMap.Length - referencedModuleCount);
 
             for (int i = 0; i < referenceMap.Length; i++)
             {
@@ -418,22 +416,6 @@ internal bool IsBound
                 }
             }
 
-            for (int i = 0; i < implicitlyResolvedReferences.Count; i++)
-            {
-                var assemblyIndex = referencedAssemblyCount + i;
-                Debug.Assert(aliasesOfReferencedAssembliesBuilder.Count == assemblyIndex);
-
-                referencedAssembliesMap.Add(implicitlyResolvedReferences[i], assemblyIndex);
-
-                // Use aliases specified on the reference returned by the missing assembly resolver.
-                // Unlike explicitly given references, we don't apply de-duplication logic on references 
-                // returned by the resolver. This is because we already have an assembly identity 
-                // (rather than an opaque reference string or a preexisting metadata reference) 
-                // and we don't ask the resolver to resolve an identity that matches identities 
-                // we have already resolved. Hence there are no opportunities for reference duplication.
-                aliasesOfReferencedAssembliesBuilder.Add(implicitlyResolvedReferences[i].Properties.Aliases);
-            }
-
             aliasesOfReferencedAssemblies = aliasesOfReferencedAssembliesBuilder.ToImmutableAndFree();
         }
 

src/Compilers/VisualBasic/Portable/Symbols/ReferenceManager.vb

@@ -256,7 +256,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Friend Function CreateAndSetSourceAssemblyFullBind(compilation As VisualBasicCompilation) As Boolean
 
                 Dim resolutionDiagnostics = DiagnosticBag.GetInstance()
-                Dim implicitlyResolvedReferences = ArrayBuilder(Of MetadataReference).GetInstance()
 
                 Try
                     Dim boundReferenceDirectiveMap As IDictionary(Of String, MetadataReference) = Nothing
@@ -275,21 +274,29 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                         resolutionDiagnostics)
 
                     Dim assemblyBeingBuiltData As New AssemblyDataForAssemblyBeingBuilt(New AssemblyIdentity(name:=SimpleAssemblyName), referencedAssemblies, modules)
-                    Dim allAssemblyData = referencedAssemblies.Insert(0, assemblyBeingBuiltData)
+                    Dim explicitAssemblyData = referencedAssemblies.Insert(0, assemblyBeingBuiltData)
 
                     ' Let's bind all the references and resolve missing one (if resolver is available)
                     Dim corLibraryIndex As Integer
                     Dim hasCircularReference As Boolean
-                    Dim bindingResult() As BoundInputAssembly = Bind(allAssemblyData,
+                    Dim implicitlyResolvedReferences As ImmutableArray(Of MetadataReference) = Nothing
+                    Dim implicitlyResolvedReferenceMap As ImmutableArray(Of ResolvedReference) = Nothing
+                    Dim allAssemblyData As ImmutableArray(Of AssemblyData) = Nothing
+
+                    Dim bindingResult() As BoundInputAssembly = Bind(explicitAssemblyData,
                                                                      compilation.Options.MetadataReferenceResolver,
                                                                      compilation.Options.MetadataImportOptions,
+                                                                     allAssemblyData,
                                                                      implicitlyResolvedReferences,
+                                                                     implicitlyResolvedReferenceMap,
                                                                      resolutionDiagnostics,
                                                                      hasCircularReference,
                                                                      corLibraryIndex)
 
                     Debug.Assert(bindingResult.Length = allAssemblyData.Length)
 
+                    referenceMap = referenceMap.AddRange(implicitlyResolvedReferenceMap)
+
                     Dim referencedAssembliesMap As Dictionary(Of MetadataReference, Integer) = Nothing
                     Dim referencedModulesMap As Dictionary(Of MetadataReference, Integer) = Nothing
                     Dim aliasesOfReferencedAssemblies As ImmutableArray(Of ImmutableArray(Of String)) = Nothing
@@ -297,8 +304,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     BuildReferencedAssembliesAndModulesMaps(
                         references,
                         referenceMap,
-                        implicitlyResolvedReferences,
-                        referencedAssemblies.Length,
                         modules.Length,
                         referencedAssembliesMap,
                         referencedModulesMap,