Fix crash when compiling internal attribute constructor (#42192)

* Fix crash when compiling internal attribute constructor * PR feedback * Add new rule to refout.md
dotnet · Mar 16, 2020 · 2b86338 · 2b86338
1 parent d626cc5
commit 2b86338
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 3 deletions.
diff --git a/docs/features/refout.md b/docs/features/refout.md
@@ -22,7 +22,7 @@ Ref assemblies further remove metadata (private members) from metadata-only asse
 
 - A ref assembly only has references for what it needs in the API surface. The real assembly may have additional references related to specific implementations. For instance, the ref assembly for `class C { private void M() { dynamic d = 1; ... } }` does not reference any types required for `dynamic`.
 - Private function-members (methods, properties and events) are removed. If there are no `InternalsVisibleTo` attributes, do the same for internal function-members.
-- But all types (including private or nested types) are kept in ref assemblies. All attributes are kept (even internal ones).
+- But all types (including private or nested types) are kept in ref assemblies. All attributes are kept (even internal ones), as well as their (internal) constructors.
 - All virtual methods are kept. Explicit interface implementations are kept. Explicitly-implemented properties and events are kept, as their accessors are virtual (and are therefore kept).
 - All fields of a struct are kept. (This is a candidate for post-C#-7.1 refinement)
 - Any resources included on the command-line are not emitted into ref assemblies (produced either with `/refout` or `/refonly`). (This was fixed in dev16)

diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs b/src/Compilers/CSharp/Portable/Emitter/Model/NamedTypeSymbolAdapter.cs
@@ -661,10 +661,14 @@ internal virtual bool IsMetadataSealed
         {
             CheckDefinitionInvariant();
 
+            // All constructors in attributes should be emitted.
+            // Don't compute IsAttributeType if IncludePrivateMembers is true, as we'll include it anyway.
+            bool alwaysIncludeConstructors = context.IncludePrivateMembers || DeclaringCompilation.IsAttributeType(this);
+
             foreach (var method in this.GetMethodsToEmit())
             {
                 Debug.Assert((object)method != null);
-                if (method.ShouldInclude(context))
+                if ((alwaysIncludeConstructors && method.MethodKind == MethodKind.Constructor) || method.ShouldInclude(context))
                 {
                     yield return method;
                 }
@@ -676,7 +680,7 @@ internal virtual bool IsMetadataSealed
             {
                 foreach (var m in generated)
                 {
-                    if (m.ShouldInclude(context))
+                    if ((alwaysIncludeConstructors && m.IsConstructor) || m.ShouldInclude(context))
                     {
                         yield return m;
                     }

diff --git a/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs b/src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs
@@ -510,6 +510,80 @@ public event System.Action PrivateRemover { add { } private remove { } }
                 );
         }
 
+        [Fact]
+        [WorkItem(38444, "https://github.com/dotnet/roslyn/issues/38444")]
+        public void EmitRefAssembly_InternalAttributeConstructor()
+        {
+            CSharpCompilation comp = CreateCompilation(@"
+using System;
+internal class SomeAttribute : Attribute
+{
+    internal SomeAttribute()
+    {
+    }
+}
+[Some]
+public class C
+{
+}
+");
+
+            using (var output = new MemoryStream())
+            using (var metadataOutput = new MemoryStream())
+            {
+                EmitResult emitResult = comp.Emit(output, metadataPEStream: metadataOutput,
+                    options: new EmitOptions(includePrivateMembers: false));
+                emitResult.Diagnostics.Verify();
+                Assert.True(emitResult.Success);
+
+                VerifyMethods(output, "C", new[] { "C..ctor()" });
+                VerifyMethods(metadataOutput, "C", new[] { "C..ctor()" });
+                VerifyMethods(output, "SomeAttribute", new[] { "SomeAttribute..ctor()" });
+                VerifyMethods(metadataOutput, "SomeAttribute", new[] { "SomeAttribute..ctor()" });
+            }
+        }
+
+        [Fact]
+        [WorkItem(38444, "https://github.com/dotnet/roslyn/issues/38444")]
+        public void EmitRefAssembly_InternalAttributeConstructor_DoesntIncludeMethodsOrStaticConstructors()
+        {
+            CSharpCompilation comp = CreateCompilation(@"
+using System;
+internal class SomeAttribute : Attribute
+{
+    internal SomeAttribute()
+    {
+    }
+
+    static SomeAttribute()
+    {
+    }
+
+    internal void F()
+    {
+    }
+}
+[Some]
+public class C
+{
+}
+");
+
+            using (var output = new MemoryStream())
+            using (var metadataOutput = new MemoryStream())
+            {
+                EmitResult emitResult = comp.Emit(output, metadataPEStream: metadataOutput,
+                    options: new EmitOptions(includePrivateMembers: false));
+                emitResult.Diagnostics.Verify();
+                Assert.True(emitResult.Success);
+
+                VerifyMethods(output, "C", new[] { "C..ctor()" });
+                VerifyMethods(metadataOutput, "C", new[] { "C..ctor()" });
+                VerifyMethods(output, "SomeAttribute", new[] { "SomeAttribute..ctor()", "SomeAttribute..cctor()", "void SomeAttribute.F()" });
+                VerifyMethods(metadataOutput, "SomeAttribute", new[] { "SomeAttribute..ctor()" });
+            }
+        }
+
         private static void VerifyMethods(MemoryStream stream, string containingType, string[] expectedMethods)
         {
             stream.Position = 0;