Record used assembly references for more scenarios.

- Merged extern aliases - Embeddable attributes - Usage of canonical definitions for NoPia embedded types - nameof of a method - Deconstruction - Collection initializers Related to dotnet#37768.
AlekseyTs · Dec 13, 2019 · f6ac474 · f6ac474
1 parent 0f03fa5
commit f6ac474
Show file tree

Hide file tree

Showing 22 changed files with 1,933 additions and 170 deletions.
diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Attributes.cs
@@ -435,7 +435,7 @@ private Symbol BindNamedAttributeArgumentName(AttributeArgumentSyntax namedArgum
             HashSet<DiagnosticInfo> useSiteDiagnostics = null;
             this.LookupMembersWithFallback(result, attributeType, name, 0, ref useSiteDiagnostics);
             diagnostics.Add(identifierName, useSiteDiagnostics);
-            Symbol resultSymbol = this.ResultSymbol(result, name, 0, identifierName, diagnostics, false, out wasError, qualifierOpt: null);
+            Symbol resultSymbol = this.ResultSymbol(result, name, 0, identifierName, diagnostics, false, out wasError, qualifierOpt: null, basesBeingResolved: null);
             resultKind = result.Kind;
             result.Free();
             return resultSymbol;

diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
@@ -5811,7 +5811,7 @@ private BoundExpression BindMemberAccessWithBoundLeft(
                             if (lookupResult.IsMultiViable)
                             {
                                 bool wasError;
-                                Symbol sym = ResultSymbol(lookupResult, rightName, rightArity, node, diagnostics, false, out wasError, ns, options);
+                                Symbol sym = ResultSymbol(lookupResult, rightName, rightArity, node, diagnostics, false, out wasError, ns, basesBeingResolved: null, options);
                                 if (wasError)
                                 {
                                     return new BoundBadExpression(node, LookupResultKind.Ambiguous, lookupResult.Symbols.AsImmutable(), ImmutableArray.Create(boundLeft), CreateErrorType(rightName), hasErrors: true);
@@ -6869,7 +6869,7 @@ private Symbol GetSymbolOrMethodOrPropertyGroup(LookupResult result, SyntaxNode
             }
 
             methodOrPropertyGroup.Clear();
-            return ResultSymbol(result, plainName, arity, node, diagnostics, false, out wasError, qualifierOpt);
+            return ResultSymbol(result, plainName, arity, node, diagnostics, false, out wasError, qualifierOpt, basesBeingResolved: null);
         }
 
         private static bool IsMethodOrPropertyGroup(ArrayBuilder<Symbol> members)

diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs
@@ -232,7 +232,8 @@ private NamespaceOrTypeOrAliasSymbolWithAnnotations BindTypeOrAliasOrKeyword(Syn
                         diagnostics: resultDiagnostics,
                         suppressUseSiteDiagnostics: false,
                         wasError: out wasError,
-                        qualifierOpt: null);
+                        qualifierOpt: null,
+                        basesBeingResolved: null);
 
                     // Here, we're mimicking behavior of dev10.  If the identifier fails to bind
                     // as a type, even if the reason is (e.g.) a type/alias conflict, then treat
@@ -354,7 +355,7 @@ internal Symbol BindNamespaceAliasSymbol(IdentifierNameSyntax node, DiagnosticBa
                 this.LookupSymbolsWithFallback(result, plainName, 0, ref useSiteDiagnostics, null, LookupOptions.NamespaceAliasesOnly);
                 diagnostics.Add(node, useSiteDiagnostics);
 
-                Symbol bindingResult = ResultSymbol(result, plainName, 0, node, diagnostics, false, out wasError, qualifierOpt: null, options: LookupOptions.NamespaceAliasesOnly);
+                Symbol bindingResult = ResultSymbol(result, plainName, 0, node, diagnostics, false, out wasError, qualifierOpt: null, basesBeingResolved: null, options: LookupOptions.NamespaceAliasesOnly);
                 result.Free();
 
                 return bindingResult;
@@ -831,7 +832,7 @@ protected NamespaceOrTypeOrAliasSymbolWithAnnotations BindNonGenericSimpleNamesp
             {
                 bool wasError;
 
-                bindingResult = ResultSymbol(result, identifierValueText, 0, node, diagnostics, suppressUseSiteDiagnostics, out wasError, qualifierOpt, options);
+                bindingResult = ResultSymbol(result, identifierValueText, 0, node, diagnostics, suppressUseSiteDiagnostics, out wasError, qualifierOpt, basesBeingResolved, options);
                 if (bindingResult.Kind == SymbolKind.Alias)
                 {
                     var aliasTarget = ((AliasSymbol)bindingResult).GetAliasTarget(basesBeingResolved);
@@ -1097,7 +1098,7 @@ private NamedTypeSymbol LookupGenericTypeName(
             diagnostics.Add(node, useSiteDiagnostics);
 
             bool wasError;
-            Symbol lookupResultSymbol = ResultSymbol(lookupResult, plainName, arity, node, diagnostics, (basesBeingResolved != null), out wasError, qualifierOpt, options);
+            Symbol lookupResultSymbol = ResultSymbol(lookupResult, plainName, arity, node, diagnostics, (basesBeingResolved != null), out wasError, qualifierOpt, basesBeingResolved, options);
 
             // As we said in the method above, there are three cases here:
             //
@@ -1536,6 +1537,7 @@ internal Symbol ResultSymbol(
             bool suppressUseSiteDiagnostics,
             out bool wasError,
             NamespaceOrTypeSymbol qualifierOpt,
+            ConsList<TypeSymbol> basesBeingResolved,
             LookupOptions options = default(LookupOptions))
         {
             Symbol symbol = resultSymbol(result, simpleName, arity, where, diagnostics, suppressUseSiteDiagnostics, out wasError, qualifierOpt, options);
@@ -1552,7 +1554,7 @@ internal Symbol ResultSymbol(
                     break;
 
                 case SymbolKind.Alias:
-                    if (shouldRecordUsedAssemblyReferences() && ((AliasSymbol)symbol).Target is TypeSymbol type)
+                    if (shouldRecordUsedAssemblyReferences() && ((AliasSymbol)symbol).GetAliasTarget(basesBeingResolved) is TypeSymbol type)
                     {
                         AddAssembliesUsedByTypeReference(type);
                     }

diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs
@@ -1652,7 +1652,7 @@ private void AppendSymbolsWithNameAndArity(
                     // binder.ResultSymbol is defined only for type/namespace lookups
                     bool wasError;
                     var diagnostics = DiagnosticBag.GetInstance();  // client code never expects a null diagnostic bag.
-                    Symbol singleSymbol = binder.ResultSymbol(lookupResult, name, arity, this.Root, diagnostics, true, out wasError, container, options);
+                    Symbol singleSymbol = binder.ResultSymbol(lookupResult, name, arity, this.Root, diagnostics, true, out wasError, container, basesBeingResolved: null, options);
                     diagnostics.Free();
 
                     if (!wasError)

diff --git a/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs b/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
@@ -762,13 +762,20 @@ private void CompileSynthesizedMethods(TypeCompilationState compilationState)
                     _diagnostics.AddRange(diagnosticsThisMethod);
                     diagnosticsThisMethod.Free();
 
-                    // error while generating IL
-                    if (emittedBody == null)
+                    if (_emitMethodBodies)
                     {
-                        break;
-                    }
+                        // error while generating IL
+                        if (emittedBody == null)
+                        {
+                            break;
+                        }
 
-                    _moduleBeingBuiltOpt.SetMethodBody(method, emittedBody);
+                        _moduleBeingBuiltOpt.SetMethodBody(method, emittedBody);
+                    }
+                    else
+                    {
+                        Debug.Assert(emittedBody is null);
+                    }
                 }
             }
             finally

diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs
@@ -1534,7 +1534,7 @@ protected virtual SynthesizedAttributeData TrySynthesizeIsByRefLikeAttribute()
             return Compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_IsByRefLikeAttribute__ctor);
         }
 
-        private void EnsureEmbeddableAttributeExists(EmbeddableAttributes attribute)
+        private void EnsureEmbeddableAttributeExists(EmbeddableAttributes attribute, bool recordUsage)
         {
             Debug.Assert(!_needsGeneratedAttributes_IsFrozen);
 
@@ -1544,25 +1544,25 @@ private void EnsureEmbeddableAttributeExists(EmbeddableAttributes attribute)
             }
 
             // Don't report any errors. They should be reported during binding.
-            if (Compilation.CheckIfAttributeShouldBeEmbedded(attribute, recordUsage: false, diagnosticsOpt: null, locationOpt: null))
+            if (Compilation.CheckIfAttributeShouldBeEmbedded(attribute, recordUsage, diagnosticsOpt: null, locationOpt: null))
             {
                 SetNeedsGeneratedAttributes(attribute);
             }
         }
 
-        internal void EnsureIsReadOnlyAttributeExists()
+        internal void EnsureIsReadOnlyAttributeExists(bool recordUsage)
         {
-            EnsureEmbeddableAttributeExists(EmbeddableAttributes.IsReadOnlyAttribute);
+            EnsureEmbeddableAttributeExists(EmbeddableAttributes.IsReadOnlyAttribute, recordUsage);
         }
 
-        internal void EnsureIsUnmanagedAttributeExists()
+        internal void EnsureIsUnmanagedAttributeExists(bool recordUsage)
         {
-            EnsureEmbeddableAttributeExists(EmbeddableAttributes.IsUnmanagedAttribute);
+            EnsureEmbeddableAttributeExists(EmbeddableAttributes.IsUnmanagedAttribute, recordUsage);
         }
 
-        internal void EnsureNullableAttributeExists()
+        internal void EnsureNullableAttributeExists(bool recordUsage)
         {
-            EnsureEmbeddableAttributeExists(EmbeddableAttributes.NullableAttribute);
+            EnsureEmbeddableAttributeExists(EmbeddableAttributes.NullableAttribute, recordUsage);
         }
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs
@@ -258,7 +258,7 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen
             var typeParameters = localFunction.TypeParameters;
             if (typeParameters.Any(typeParameter => typeParameter.HasUnmanagedTypeConstraint))
             {
-                _factory.CompilationState.ModuleBuilderOpt?.EnsureIsUnmanagedAttributeExists();
+                _factory.CompilationState.ModuleBuilderOpt?.EnsureIsUnmanagedAttributeExists(recordUsage: true);
             }
 
             if (_factory.CompilationState.Compilation.ShouldEmitNullableAttributes(localFunction))
@@ -271,7 +271,7 @@ public override BoundNode VisitLocalFunctionStatement(BoundLocalFunctionStatemen
 
                 if (constraintsNeedNullableAttribute || returnTypeNeedsNullableAttribute || parametersNeedNullableAttribute)
                 {
-                    _factory.CompilationState.ModuleBuilderOpt?.EnsureNullableAttributeExists();
+                    _factory.CompilationState.ModuleBuilderOpt?.EnsureNullableAttributeExists(recordUsage: true);
                 }
             }
 
@@ -840,7 +840,7 @@ private void CheckRefReadOnlySymbols(MethodSymbol symbol)
             if (symbol.ReturnsByRefReadonly ||
                 symbol.Parameters.Any(p => p.RefKind == RefKind.In))
             {
-                _factory.CompilationState.ModuleBuilderOpt?.EnsureIsReadOnlyAttributeExists();
+                _factory.CompilationState.ModuleBuilderOpt?.EnsureIsReadOnlyAttributeExists(recordUsage: true);
             }
         }
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs b/src/Compilers/CSharp/Portable/Symbols/Compilation_UsedAssemblies.cs
@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Threading;
@@ -9,40 +10,56 @@
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 
+#nullable enable
+
 namespace Microsoft.CodeAnalysis.CSharp
 {
     public partial class CSharpCompilation
     {
-        private ConcurrentSet<AssemblySymbol> _lazyUsedAssemblyReferences;
+        private ConcurrentSet<AssemblySymbol>? _lazyUsedAssemblyReferences;
         private bool _usedAssemblyReferencesFrozen;
 
         internal override ImmutableArray<MetadataReference> GetUsedAssemblyReferences(CancellationToken cancellationToken = default)
         {
-            ConcurrentSet<AssemblySymbol> usedAssemblies = GetCompleteSetOfUsedAssemblies(cancellationToken);
+            ConcurrentSet<AssemblySymbol>? usedAssemblies = GetCompleteSetOfUsedAssemblies(cancellationToken);
 
             if (usedAssemblies is null)
             {
                 return ImmutableArray<MetadataReference>.Empty;
             }
 
-            var builder = ArrayBuilder<MetadataReference>.GetInstance(usedAssemblies.Count);
+            var setOfReferences = new HashSet<MetadataReference>(ReferenceEqualityComparer.Instance);
 
             foreach (var reference in References)
             {
                 if (reference.Properties.Kind == MetadataImageKind.Assembly)
                 {
                     Symbol symbol = GetAssemblyOrModuleSymbol(reference);
-                    if (symbol is object && usedAssemblies.Contains((AssemblySymbol)symbol))
+                    if (symbol is object && usedAssemblies.Contains((AssemblySymbol)symbol) &&
+                        setOfReferences.Add(reference) &&
+                        GetBoundReferenceManager().MergedAssemblyReferencesMap.TryGetValue(reference, out ImmutableArray<MetadataReference> merged))
                     {
-                        builder.Add(reference);
+                        // Include all "merged" references as well because they might "define" used extern aliases.
+                        setOfReferences.AddAll(merged);
                     }
                 }
             }
 
+            // Use stable ordering for the result, matching the order in References.
+            var builder = ArrayBuilder<MetadataReference>.GetInstance(setOfReferences.Count);
+
+            foreach (var reference in References)
+            {
+                if (setOfReferences.Contains(reference))
+                {
+                    builder.Add(reference);
+                }
+            }
+
             return builder.ToImmutableAndFree();
         }
 
-        private ConcurrentSet<AssemblySymbol> GetCompleteSetOfUsedAssemblies(CancellationToken cancellationToken)
+        private ConcurrentSet<AssemblySymbol>? GetCompleteSetOfUsedAssemblies(CancellationToken cancellationToken)
         {
             if (!_usedAssemblyReferencesFrozen && !Volatile.Read(ref _usedAssemblyReferencesFrozen))
             {
@@ -129,7 +146,7 @@ void completeTheSetOfUsedAssemblies(bool seenErrors, CancellationToken cancellat
                             while (stack.Count != 0)
                             {
                                 AssemblySymbol current = stack.Pop();
-                                ConcurrentSet<AssemblySymbol> usedAssemblies;
+                                ConcurrentSet<AssemblySymbol>? usedAssemblies;
 
                                 switch (current)
                                 {
@@ -194,7 +211,7 @@ void completeTheSetOfUsedAssemblies(bool seenErrors, CancellationToken cancellat
             }
         }
 
-        internal bool AddUsedAssembly(AssemblySymbol assembly)
+        internal bool AddUsedAssembly(AssemblySymbol? assembly)
         {
             if (assembly is null || assembly == SourceAssembly || assembly.IsMissing)
             {
@@ -217,7 +234,7 @@ internal bool AddUsedAssembly(AssemblySymbol assembly)
             return result;
         }
 
-        internal void AddAssembliesUsedByTypeReference(TypeSymbol typeOpt)
+        internal void AddAssembliesUsedByTypeReference(TypeSymbol? typeOpt)
         {
             while (true)
             {

diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEModuleSymbol.cs
@@ -199,6 +199,11 @@ public override AssemblySymbol ContainingAssembly
             }
         }
 
+        internal override bool MightContainNoPiaLocalTypes()
+        {
+            return _module.ContainsNoPiaLocalTypes();
+        }
+
         public override ImmutableArray<Location> Locations
         {
             get

diff --git a/src/Compilers/CSharp/Portable/Symbols/MissingModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/MissingModuleSymbol.cs
@@ -98,6 +98,11 @@ public override NamespaceSymbol GlobalNamespace
             }
         }
 
+        internal sealed override bool MightContainNoPiaLocalTypes()
+        {
+            return false;
+        }
+
         public override int GetHashCode()
         {
             return assembly.GetHashCode();

diff --git a/src/Compilers/CSharp/Portable/Symbols/ModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/ModuleSymbol.cs
@@ -363,6 +363,8 @@ public NamespaceSymbol GetModuleNamespace(INamespaceSymbol namespaceSymbol)
             }
         }
 
+        internal abstract bool MightContainNoPiaLocalTypes();
+
         /// <summary>
         /// If this symbol represents a metadata module returns the underlying <see cref="ModuleMetadata"/>.
         /// 

diff --git a/src/Compilers/CSharp/Portable/Symbols/ReferenceManager.cs b/src/Compilers/CSharp/Portable/Symbols/ReferenceManager.cs
@@ -403,6 +403,8 @@ private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
 
                     Dictionary<MetadataReference, int> referencedAssembliesMap, referencedModulesMap;
                     ImmutableArray<ImmutableArray<string>> aliasesOfReferencedAssemblies;
+                    Dictionary<MetadataReference, ImmutableArray<MetadataReference>> mergedAssemblyReferencesMapOpt;
+
                     BuildReferencedAssembliesAndModulesMaps(
                         bindingResult,
                         references,
@@ -413,7 +415,8 @@ private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
                         supersedeLowerVersions,
                         out referencedAssembliesMap,
                         out referencedModulesMap,
-                        out aliasesOfReferencedAssemblies);
+                        out aliasesOfReferencedAssemblies,
+                        out mergedAssemblyReferencesMapOpt);
 
                     // Create AssemblySymbols for assemblies that can't use any existing symbols.
                     var newSymbols = new List<int>();
@@ -507,7 +510,8 @@ private bool CreateAndSetSourceAssemblyFullBind(CSharpCompilation compilation)
                                     moduleReferences,
                                     assemblySymbol.SourceModule.GetReferencedAssemblySymbols(),
                                     aliasesOfReferencedAssemblies,
-                                    assemblySymbol.SourceModule.GetUnifiedAssemblies());
+                                    assemblySymbol.SourceModule.GetUnifiedAssemblies(),
+                                    mergedAssemblyReferencesMapOpt);
 
                                 // Make sure that the given compilation holds on this instance of reference manager.
                                 Debug.Assert(ReferenceEquals(compilation._referenceManager, this) || HasCircularReference);

diff --git a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingModuleSymbol.cs
@@ -130,6 +130,11 @@ public override NamespaceSymbol GlobalNamespace
             }
         }
 
+        internal override bool MightContainNoPiaLocalTypes()
+        {
+            return _underlyingModule.MightContainNoPiaLocalTypes();
+        }
+
         public override bool IsImplicitlyDeclared
         {
             get { return _underlyingModule.IsImplicitlyDeclared; }

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceModuleSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceModuleSymbol.cs
@@ -110,7 +110,7 @@ internal bool AnyReferencedAssembliesAreLinked
             }
         }
 
-        internal bool MightContainNoPiaLocalTypes()
+        internal override bool MightContainNoPiaLocalTypes()
         {
             return AnyReferencedAssembliesAreLinked ||
                 ContainsExplicitDefinitionOfNoPiaLocalTypes;
-Original file line number
+Diff line change
@@ Expand Up / @@ -199,6 +199,11 @@ public override AssemblySymbol ContainingAssembly @@
                 }
             }
+            internal override bool MightContainNoPiaLocalTypes()
+            {
+                return _module.ContainsNoPiaLocalTypes();
+            }
             public override ImmutableArray<Location> Locations
             {
                 get
@@ Expand Down @@