Generate readonlyattribute if it does not exist (#18715)

* Call this.DeclaringCompilation.EnsureReadOnlyAttributeExists() when needed

* Promote EmbeddedAttribute to be used by both compilers

* Generate Embedded attributes on PE Module Builder

* Thread PEModuleBuilder through Symbol.AddSynthesizedAttributes()

* Fix failing tests

* Base case passing

* Refactor ReadOnlyAttribute to IsReadOnlyAttribute

* Fix build break

* More Tests

* Clean up

* PR Comments #1

PR Comments #3

PR Comments #4

PR Comments #5

PR Comments #6

PR Comments # 7

* Handle NoPIA

Added more tests

* Lambdas and Local functions no longer generate attributes in symbols

Added tests for constructors and operators

Signal need for synthesized attributes in local rewriter for lambdas an local functions

Clean up

* Clean up

* More PR Feedback

* Adding more tests

* More PR Comments

* More tests for explicit interface implementations

* Latest PR Comments

* Moved EnsureIsReadOnlyAttributeExists to AfterAddingTypeMembersChecks

* Fix failing tests

src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs

@@ -1116,7 +1116,12 @@ internal SingleLookupResult CheckViability(Symbol symbol, int arity, LookupOptio
                 ? ((AliasSymbol)symbol).GetAliasTarget(basesBeingResolved)
                 : symbol;
 
-            if (WrongArity(symbol, arity, diagnose, options, out diagInfo))
+            // Check for symbols marked with 'Microsoft.CodeAnalysis.Embedded' attribute
+            if (!this.Compilation.SourceModule.Equals(unwrappedSymbol.ContainingModule) && unwrappedSymbol.IsHiddenByCodeAnalysisEmbeddedAttribute())
+            {
+                return LookupResult.Empty();
+            }
+            else if (WrongArity(symbol, arity, diagnose, options, out diagInfo))
             {
                 return LookupResult.WrongArity(symbol, diagInfo);
             }

src/Compilers/CSharp/Portable/Binder/LookupResult.cs

@@ -157,6 +157,11 @@ internal static SingleLookupResult WrongArity(Symbol symbol, DiagnosticInfo erro
             return new SingleLookupResult(LookupResultKind.WrongArity, symbol, error);
         }
 
+        internal static SingleLookupResult Empty()
+        {
+            return new SingleLookupResult(LookupResultKind.Empty, null, null);
+        }
+
         internal static SingleLookupResult NotReferencable(Symbol symbol, DiagnosticInfo error)
         {
             return new SingleLookupResult(LookupResultKind.NotReferencable, symbol, error);

src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs

@@ -450,8 +450,17 @@ private BoundLambda ReallyBind(NamedTypeSymbol delegateType)
                 cacheKey.ParameterTypes,
                 cacheKey.ParameterRefKinds,
                 refKind,
-                returnType);
+                returnType,
+                diagnostics);
             lambdaBodyBinder = new ExecutableCodeBinder(_unboundLambda.Syntax, lambdaSymbol, ParameterBinder(lambdaSymbol, binder));
+
+            if (lambdaSymbol.RefKind == CodeAnalysis.RefKind.RefReadOnly)
+            {
+                binder.Compilation.EnsureIsReadOnlyAttributeExists(diagnostics, lambdaSymbol.DiagnosticLocation, modifyCompilationForRefReadOnly: false);
+            }
+
+            ParameterHelpers.EnsureIsReadOnlyAttributeExists(lambdaSymbol.Parameters, diagnostics, modifyCompilationForRefReadOnly: false);
+
             block = BindLambdaBody(lambdaSymbol, lambdaBodyBinder, diagnostics);
 
             ((ExecutableCodeBinder)lambdaBodyBinder).ValidateIteratorMethods(diagnostics);
@@ -525,7 +534,15 @@ private void ValidateUnsafeParameters(DiagnosticBag diagnostics, ImmutableArray<
         private BoundLambda ReallyInferReturnType(NamedTypeSymbol delegateType, ImmutableArray<TypeSymbol> parameterTypes, ImmutableArray<RefKind> parameterRefKinds)
         {
             var diagnostics = DiagnosticBag.GetInstance();
-            var lambdaSymbol = new LambdaSymbol(binder.Compilation, binder.ContainingMemberOrLambda, _unboundLambda, parameterTypes, parameterRefKinds, refKind: Microsoft.CodeAnalysis.RefKind.None, returnType: null);
+            var lambdaSymbol = new LambdaSymbol(
+                binder.Compilation,
+                binder.ContainingMemberOrLambda,
+                _unboundLambda,
+                parameterTypes,
+                parameterRefKinds,
+                refKind: CodeAnalysis.RefKind.None,
+                returnType: null,
+                diagnostics: diagnostics);
             Binder lambdaBodyBinder = new ExecutableCodeBinder(_unboundLambda.Syntax, lambdaSymbol, ParameterBinder(lambdaSymbol, binder));
             var block = BindLambdaBody(lambdaSymbol, lambdaBodyBinder, diagnostics);
 

src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj

@@ -512,6 +512,7 @@
     <Compile Include="Symbols\DiscardSymbol.cs" />
     <Compile Include="Symbols\Metadata\PE\TupleTypeDecoder.cs" />
     <Compile Include="Symbols\Source\GlobalExpressionVariable.cs" />
+    <Compile Include="Symbols\Synthesized\SynthesizedEmbeddedAttributeSymbol.cs" />
     <Compile Include="Symbols\Tuples\TupleEventSymbol.cs" />
     <Compile Include="Symbols\Tuples\TupleFieldSymbol.cs" />
     <Compile Include="Symbols\Tuples\TupleMethodSymbol.cs" />

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -5115,6 +5115,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>An expression of type '{0}' cannot be handled by a pattern of type '{1}' in C# {2}. Please use language version {3} or greater.</value>
   </data>
   <data name="ERR_ExplicitReadOnlyAttr" xml:space="preserve">
-    <value>Do not use 'System.Runtime.CompilerServices.ReadOnlyAttribute'. This is reserved for compiler usage.</value>
+    <value>Do not use 'System.Runtime.CompilerServices.IsReadOnlyAttribute'. This is reserved for compiler usage.</value>
+  </data>
+  <data name="ERR_TypeReserved" xml:space="preserve">
+    <value>The type name '{0}' is reserved to be used by the compiler.</value>
   </data>
 </root>

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

@@ -1988,6 +1988,10 @@ internal DiagnosticBag DeclarationDiagnostics
         private IEnumerable<Diagnostic> FreezeDeclarationDiagnostics()
         {
             _declarationDiagnosticsFrozen = true;
+
+            // Also freeze that flag, as symbols bound after getting the declaration diagnostics shouldn't need to modify it
+            _needsGeneratedIsReadOnlyAttribute_IsFrozen = true;
+
             var result = _lazyDeclarationDiagnostics?.AsEnumerable() ?? Enumerable.Empty<Diagnostic>();
             return result;
         }

src/Compilers/CSharp/Portable/Compiler/AnonymousTypeMethodBodySynthesizer.cs

@@ -29,7 +29,8 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState,
                 int statementIndex = 0;
 
                 //  explicit base constructor call
-                BoundExpression call = MethodCompiler.GenerateObjectConstructorInitializer(this, diagnostics);
+                Debug.Assert(ContainingType.BaseTypeNoUseSiteDiagnostics.SpecialType == SpecialType.System_Object);
+                BoundExpression call = MethodCompiler.GenerateBaseParameterlessConstructorInitializer(this, diagnostics);
                 if (call == null)
                 {
                     // This may happen if Object..ctor is not found or is unaccessible

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -145,7 +145,7 @@ public static void CompileMethodBodies(
             // compile additional and anonymous types if any
             if (moduleBeingBuiltOpt != null)
             {
-                var additionalTypes = moduleBeingBuiltOpt.GetAdditionalTopLevelTypes();
+                var additionalTypes = moduleBeingBuiltOpt.GetAdditionalTopLevelTypes(diagnostics);
                 if (!additionalTypes.IsEmpty)
                 {
                     methodCompiler.CompileSynthesizedMethods(additionalTypes, diagnostics);
@@ -1715,7 +1715,7 @@ internal static BoundExpression BindConstructorInitializer(
             {
                 if (baseType.SpecialType == SpecialType.System_Object)
                 {
-                    return GenerateObjectConstructorInitializer(constructor, diagnostics);
+                    return GenerateBaseParameterlessConstructorInitializer(constructor, diagnostics);
                 }
                 else if (baseType.IsErrorType() || baseType.IsStatic)
                 {
@@ -1816,42 +1816,47 @@ private static SyntaxToken GetImplicitConstructorBodyToken(CSharpSyntaxNode cont
             }
         }
 
-        internal static BoundCall GenerateObjectConstructorInitializer(MethodSymbol constructor, DiagnosticBag diagnostics)
+        internal static BoundCall GenerateBaseParameterlessConstructorInitializer(MethodSymbol constructor, DiagnosticBag diagnostics)
         {
-            NamedTypeSymbol objectType = constructor.ContainingType.BaseTypeNoUseSiteDiagnostics;
-            Debug.Assert(objectType.SpecialType == SpecialType.System_Object);
-            MethodSymbol objectConstructor = null;
+            NamedTypeSymbol baseType = constructor.ContainingType.BaseTypeNoUseSiteDiagnostics;
+            MethodSymbol baseConstructor = null;
             LookupResultKind resultKind = LookupResultKind.Viable;
+            Location diagnosticsLocation = constructor.Locations.IsEmpty ? NoLocation.Singleton : constructor.Locations[0];
 
-            foreach (MethodSymbol objectCtor in objectType.InstanceConstructors)
+            foreach (MethodSymbol ctor in baseType.InstanceConstructors)
             {
-                if (objectCtor.ParameterCount == 0)
+                if (ctor.ParameterCount == 0)
                 {
-                    objectConstructor = objectCtor;
+                    baseConstructor = ctor;
                     break;
                 }
             }
 
             // UNDONE: If this happens then something is deeply wrong. Should we give a better error?
-            if ((object)objectConstructor == null)
+            if ((object)baseConstructor == null)
+            {
+                diagnostics.Add(ErrorCode.ERR_BadCtorArgCount, diagnosticsLocation, baseType, /*desired param count*/ 0);
+                return null;
+            }
+
+            if (Binder.ReportUseSiteDiagnostics(baseConstructor, diagnostics, diagnosticsLocation))
             {
-                diagnostics.Add(ErrorCode.ERR_BadCtorArgCount, constructor.Locations[0], objectType, /*desired param count*/ 0);
                 return null;
             }
 
             // UNDONE: If this happens then something is deeply wrong. Should we give a better error?
             bool hasErrors = false;
             HashSet<DiagnosticInfo> useSiteDiagnostics = null;
-            if (!AccessCheck.IsSymbolAccessible(objectConstructor, constructor.ContainingType, ref useSiteDiagnostics))
+            if (!AccessCheck.IsSymbolAccessible(baseConstructor, constructor.ContainingType, ref useSiteDiagnostics))
             {
-                diagnostics.Add(ErrorCode.ERR_BadAccess, constructor.Locations[0], objectConstructor);
+                diagnostics.Add(ErrorCode.ERR_BadAccess, diagnosticsLocation, baseConstructor);
                 resultKind = LookupResultKind.Inaccessible;
                 hasErrors = true;
             }
 
             if (!useSiteDiagnostics.IsNullOrEmpty())
             {
-                diagnostics.Add(constructor.Locations.IsEmpty ? NoLocation.Singleton : constructor.Locations[0], useSiteDiagnostics);
+                diagnostics.Add(diagnosticsLocation, useSiteDiagnostics);
             }
 
             CSharpSyntaxNode syntax = constructor.GetNonNullSyntaxNode();
@@ -1860,7 +1865,7 @@ internal static BoundCall GenerateObjectConstructorInitializer(MethodSymbol cons
             return new BoundCall(
                 syntax: syntax,
                 receiverOpt: receiver,
-                method: objectConstructor,
+                method: baseConstructor,
                 arguments: ImmutableArray<BoundExpression>.Empty,
                 argumentNamesOpt: ImmutableArray<string>.Empty,
                 argumentRefKindsOpt: ImmutableArray<RefKind>.Empty,
@@ -1869,7 +1874,7 @@ internal static BoundCall GenerateObjectConstructorInitializer(MethodSymbol cons
                 invokedAsExtensionMethod: false,
                 argsToParamsOpt: ImmutableArray<int>.Empty,
                 resultKind: resultKind,
-                type: objectType,
+                type: baseType,
                 hasErrors: hasErrors)
             { WasCompilerGenerated = true };
         }

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

@@ -570,21 +570,13 @@ bool Cci.IMethodDefinition.RequiresSecurityObject
             }
         }
 
-        IEnumerable<Cci.ICustomAttribute> Cci.IMethodDefinition.ReturnValueAttributes
-        {
-            get
-            {
-                return GetReturnValueCustomAttributesToEmit();
-            }
-        }
-
-        private IEnumerable<CSharpAttributeData> GetReturnValueCustomAttributesToEmit()
+        IEnumerable<Cci.ICustomAttribute> Cci.IMethodDefinition.GetReturnValueAttributes(EmitContext context)
         {
             CheckDefinitionInvariant();
 
             ImmutableArray<CSharpAttributeData> userDefined = this.GetReturnTypeAttributes();
             ArrayBuilder<SynthesizedAttributeData> synthesized = null;
-            this.AddSynthesizedReturnTypeAttributes(ref synthesized);
+            this.AddSynthesizedReturnTypeAttributes((PEModuleBuilder)context.Module, ref synthesized);
 
             // Note that callers of this method (CCI and ReflectionEmitter) have to enumerate 
             // all items of the returned iterator, otherwise the synthesized ArrayBuilder may leak.

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

@@ -1,10 +1,13 @@
 // 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.Concurrent;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Reflection;
+using System.Threading;
+using Microsoft.Cci;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.Emit;
 using Roslyn.Utilities;
@@ -17,6 +20,9 @@ internal abstract class PEAssemblyBuilderBase : PEModuleBuilder, Cci.IAssemblyRe
         private readonly ImmutableArray<NamedTypeSymbol> _additionalTypes;
         private ImmutableArray<Cci.IFileReference> _lazyFiles;
 
+        private SynthesizedEmbeddedAttributeSymbol _lazyEmbeddedAttribute;
+        private SynthesizedEmbeddedAttributeSymbol _lazyIsReadOnlyAttribute;
+
         /// <summary>
         /// The behavior of the C# command-line compiler is as follows:
         ///   1) If the /out switch is specified, then the explicit assembly name is used.
@@ -56,9 +62,22 @@ public PEAssemblyBuilderBase(
 
         public override ISourceAssemblySymbolInternal SourceAssemblyOpt => _sourceAssembly;
 
-        internal override ImmutableArray<NamedTypeSymbol> GetAdditionalTopLevelTypes()
+        internal override ImmutableArray<NamedTypeSymbol> GetAdditionalTopLevelTypes(DiagnosticBag diagnostics)
         {
-            return _additionalTypes;
+            var builder = ArrayBuilder<NamedTypeSymbol>.GetInstance();
+            builder.AddRange(_additionalTypes);
+
+            CreateEmbeddedAttributesIfNeeded(diagnostics);
+            if ((object)_lazyEmbeddedAttribute != null)
+            {
+                builder.Add(_lazyEmbeddedAttribute);
+            }
+            if ((object)_lazyIsReadOnlyAttribute != null)
+            {
+                builder.Add(_lazyIsReadOnlyAttribute);
+            }
+
+            return builder.ToImmutableAndFree();
         }
 
         public sealed override IEnumerable<Cci.IFileReference> GetFiles(EmitContext context)
@@ -131,6 +150,65 @@ protected override void AddEmbeddedResourcesFromAddedModules(ArrayBuilder<Cci.Ma
         public override string Name => _metadataName;
         public AssemblyIdentity Identity => _sourceAssembly.Identity;
         public Version AssemblyVersionPattern => _sourceAssembly.AssemblyVersionPattern;
+
+        internal override SynthesizedAttributeData SynthesizeEmbeddedAttribute()
+        {
+            if ((object)_lazyEmbeddedAttribute != null)
+            {
+                return new SynthesizedAttributeData(
+                    _lazyEmbeddedAttribute.Constructor,
+                    ImmutableArray<TypedConstant>.Empty,
+                    ImmutableArray<KeyValuePair<string, TypedConstant>>.Empty);
+            }
+
+            return base.SynthesizeEmbeddedAttribute();
+        }
+
+        protected override SynthesizedAttributeData TrySynthesizeIsReadOnlyAttribute()
+        {
+            if ((object)_lazyIsReadOnlyAttribute != null)
+            {
+                return new SynthesizedAttributeData(
+                    _lazyIsReadOnlyAttribute.Constructor,
+                    ImmutableArray<TypedConstant>.Empty,
+                    ImmutableArray<KeyValuePair<string, TypedConstant>>.Empty);
+            }
+
+            return base.TrySynthesizeIsReadOnlyAttribute();
+        }
+
+        private void CreateEmbeddedAttributesIfNeeded(DiagnosticBag diagnostics)
+        {
+            if (this.NeedsGeneratedIsReadOnlyAttribute)
+            {
+                CreateEmbeddedAttributeIfNeeded(
+                    ref _lazyEmbeddedAttribute,
+                    diagnostics,
+                    AttributeDescription.CodeAnalysisEmbeddedAttribute);
+
+                CreateEmbeddedAttributeIfNeeded(
+                    ref _lazyIsReadOnlyAttribute,
+                    diagnostics,
+                    AttributeDescription.IsReadOnlyAttribute);
+            }
+        }
+
+        private void CreateEmbeddedAttributeIfNeeded(ref SynthesizedEmbeddedAttributeSymbol symbol, DiagnosticBag diagnostics, AttributeDescription description)
+        {
+            if ((object)symbol == null)
+            {
+                var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
+                var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
+                Debug.Assert((object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);
+
+                if (!(userDefinedAttribute is MissingMetadataTypeSymbol))
+                {
+                    diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.Locations[0], description.FullName);
+                }
+
+                symbol = new SynthesizedEmbeddedAttributeSymbol(description, _sourceAssembly.DeclaringCompilation, diagnostics);
+            }
+        }
     }
 
     internal sealed class PEAssemblyBuilder : PEAssemblyBuilderBase