Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when compiling internal attribute constructor #42192

Merged
merged 3 commits into from
Mar 16, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
74 changes: 74 additions & 0 deletions src/Compilers/CSharp/Test/Emit/Emit/CompilationEmitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal SomeAttribute() [](start = 4, length = 24)

Please add a regular internal method and verify that it is not emitted. #Closed

{
}
}
[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()
Copy link
Contributor

@AlekseyTs AlekseyTs Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal void F() [](start = 4, length = 17)

Please also add a test with a static constructor. #Closed

{
}
}
[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;
Expand Down