Skip to content

Commit

Permalink
Handle custom modifiers on copy ctor (#45136)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv authored Jun 17, 2020
1 parent 2ad1435 commit d702fa6
Show file tree
Hide file tree
Showing 3 changed files with 305 additions and 8 deletions.
39 changes: 32 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,46 @@ internal override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory F,

internal static MethodSymbol? FindCopyConstructor(NamedTypeSymbol containingType, NamedTypeSymbol within, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
{
// We should handle ambiguities once we consider custom modifiers, as we do in overload resolution
// https://github.com/dotnet/roslyn/issues/45077
MethodSymbol? bestCandidate = null;
int bestModifierCountSoFar = -1; // stays as -1 unless we hit an ambiguity
foreach (var member in containingType.InstanceConstructors)
{
if (HasCopyConstructorSignature(member) &&
!member.HasUnsupportedMetadata &&
AccessCheck.IsSymbolAccessible(member, within, ref useSiteDiagnostics))
{
return member;
// If one has fewer custom modifiers, that is better
// (see OverloadResolution.BetterFunctionMember)

if (bestCandidate is null && bestModifierCountSoFar < 0)
{
bestCandidate = member;
continue;
}

if (bestModifierCountSoFar < 0)
{
bestModifierCountSoFar = bestCandidate.CustomModifierCount();
}

var memberModCount = member.CustomModifierCount();
if (memberModCount > bestModifierCountSoFar)
{
continue;
}

if (memberModCount == bestModifierCountSoFar)
{
bestCandidate = null;
continue;
}

bestCandidate = member;
bestModifierCountSoFar = memberModCount;
}
}

return null;
return bestCandidate;
}

internal static bool IsCopyConstructor(Symbol member)
Expand All @@ -86,10 +113,8 @@ internal static bool IsCopyConstructor(Symbol member)
internal static bool HasCopyConstructorSignature(MethodSymbol member)
{
NamedTypeSymbol containingType = member.ContainingType;
// We should relax the comparison to AllIgnoreOptions, so that a copy constructor with a custom modifier is recognized
// https://github.com/dotnet/roslyn/issues/45077
return member is MethodSymbol { IsStatic: false, ParameterCount: 1, Arity: 0 } method &&
method.Parameters[0].Type.Equals(containingType, TypeCompareKind.CLRSignatureCompareOptions) &&
method.Parameters[0].Type.Equals(containingType, TypeCompareKind.AllIgnoreOptions) &&
method.Parameters[0].RefKind == RefKind.None;
}
}
Expand Down
272 changes: 272 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4330,6 +4330,57 @@ .maxstack 2
}");
}

[Fact, WorkItem(44902, "https://github.com/dotnet/roslyn/issues/44902")]
public void CopyCtor_NotInRecordType()
{
var source =
@"public class C
{
public object Property { get; set; }
public int field = 42;
public C(C c)
{
}
}
public class D : C
{
public int field2 = 43;
public D(D d) : base(d)
{
}
}
";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics();

var verifier = CompileAndVerify(comp);
verifier.VerifyIL("C..ctor(C)", @"
{
// Code size 15 (0xf)
.maxstack 2
IL_0000: ldarg.0
IL_0001: ldc.i4.s 42
IL_0003: stfld ""int C.field""
IL_0008: ldarg.0
IL_0009: call ""object..ctor()""
IL_000e: ret
}");

verifier.VerifyIL("D..ctor(D)", @"
{
// Code size 16 (0x10)
.maxstack 2
IL_0000: ldarg.0
IL_0001: ldc.i4.s 43
IL_0003: stfld ""int D.field2""
IL_0008: ldarg.0
IL_0009: ldarg.1
IL_000a: call ""C..ctor(C)""
IL_000f: ret
}");
}

[Fact, WorkItem(44902, "https://github.com/dotnet/roslyn/issues/44902")]
public void CopyCtor_UserDefinedButDoesNotDelegateToBaseCopyCtor()
{
Expand Down Expand Up @@ -5111,6 +5162,227 @@ public record C : B {
);
}

[Fact, WorkItem(45077, "https://github.com/dotnet/roslyn/issues/45077")]
public void CopyCtor_AmbiguitiesInMetadata()
{
// IL for a minimal `public record B { }` with injected copy constructors
var ilSource_template = @"
.class public auto ansi beforefieldinit B extends [mscorlib]System.Object
{
INJECT
.method public hidebysig specialname newslot virtual instance class B '<>Clone' () cil managed
{
IL_0000: ldarg.0
IL_0001: newobj instance void B::.ctor(class B)
IL_0006: ret
}
.method public hidebysig specialname rtspecialname instance void .ctor () cil managed
{
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Object::.ctor()
IL_0006: ret
}
}
";

var source = @"
public record C : B
{
public static void Main()
{
var c = new C();
_ = c with { };
}
}";

// We're going to inject various copy constructors into record B (at INJECT marker), and check which one is used
// by derived record C
// The RAN and THROW markers are shorthands for method bodies that print "RAN" and throw, respectively.

// .ctor(B) vs. .ctor(modopt B)
verifyBoth(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B '' ) cil managed
RAN
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
THROW
");

// .ctor(modopt B) alone
verify(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
RAN
");

// .ctor(B) vs. .ctor(modreq B)
verifyBoth(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B '' ) cil managed
RAN
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modreq(int64) '' ) cil managed
THROW
");

// .ctor(modopt B) vs. .ctor(modreq B)
verifyBoth(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
RAN
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modreq(int64) '' ) cil managed
THROW
");

// .ctor(B) vs. .ctor(modopt1 B) and .ctor(modopt2 B)
verifyBoth(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B '' ) cil managed
RAN
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
THROW
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int32) '' ) cil managed
THROW
");
// .ctor(B) vs. .ctor(modopt1 B) and .ctor(modreq B)
verifyBoth(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B '' ) cil managed
RAN
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
THROW
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modreq(int32) '' ) cil managed
THROW
");

// .ctor(modeopt1 B) vs. .ctor(modopt2 B)
verifyBoth(@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
THROW
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int32) '' ) cil managed
THROW
", isError: true);

// private .ctor(B) vs. .ctor(modopt1 B) and .ctor(modopt B)
verifyBoth(@"
.method private hidebysig specialname rtspecialname instance void .ctor ( class B '' ) cil managed
RAN
",
@"
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int64) '' ) cil managed
THROW
.method public hidebysig specialname rtspecialname instance void .ctor ( class B modopt(int32) '' ) cil managed
THROW
", isError: true);

void verifyBoth(string inject1, string inject2, bool isError = false)
{
verify(inject1 + inject2, isError);
verify(inject2 + inject1, isError);
}

void verify(string inject, bool isError = false)
{
var ranBody = @"
{
IL_0000: ldstr ""RAN""
IL_0005: call void [mscorlib]System.Console::WriteLine(string)
IL_000a: ret
}
";

var throwBody = @"
{
IL_0000: ldnull
IL_0001: throw
}
";

var comp = CreateCompilationWithIL(new[] { source, IsExternalInitTypeDefinition },
ilSource: ilSource_template.Replace("INJECT", inject).Replace("RAN", ranBody).Replace("THROW", throwBody),
parseOptions: TestOptions.RegularPreview, options: TestOptions.DebugExe);

var expectedDiagnostics = isError ? new[] {
// (2,15): error CS8867: No accessible copy constructor found in base type 'B'.
// public record C : B
Diagnostic(ErrorCode.ERR_NoCopyConstructorInBaseType, "C").WithArguments("B").WithLocation(2, 15)
} : new DiagnosticDescription[] { };

comp.VerifyDiagnostics(expectedDiagnostics);
if (expectedDiagnostics is null)
{
CompileAndVerify(comp, expectedOutput: "RAN");
}
}
}

[Fact, WorkItem(45077, "https://github.com/dotnet/roslyn/issues/45077")]
public void CopyCtor_AmbiguitiesInMetadata_GenericType()
{
// IL for a minimal `public record B<T> { }` with modopt in nested position of parameter type
var ilSource = @"
.class public auto ansi beforefieldinit B`1<T> extends [mscorlib]System.Object implements class [mscorlib]System.IEquatable`1<class B`1<!T>>
{
.method family hidebysig specialname rtspecialname instance void .ctor ( class B`1<!T modopt(int64)> '' ) cil managed
{
IL_0000: ldstr ""RAN""
IL_0005: call void [mscorlib]System.Console::WriteLine(string)
IL_000a: ret
}
.method public hidebysig specialname newslot virtual instance class B`1<!T> '<>Clone' () cil managed
{
IL_0000: ldarg.0
IL_0001: newobj instance void class B`1<!T>::.ctor(class B`1<!0>)
IL_0006: ret
}
.method public hidebysig specialname rtspecialname instance void .ctor () cil managed
{
IL_0000: ldarg.0
IL_0001: call instance void [mscorlib]System.Object::.ctor()
IL_0006: ret
}
.method public newslot virtual instance bool Equals ( class B`1<!T> '' ) cil managed
{
IL_0000: ldnull
IL_0001: throw
}
}
";

var source = @"
public record C<T> : B<T> { }
public class Program
{
public static void Main()
{
var c = new C<string>();
_ = c with { };
}
}";


var comp = CreateCompilationWithIL(new[] { source, IsExternalInitTypeDefinition },
ilSource: ilSource,
parseOptions: TestOptions.RegularPreview, options: TestOptions.DebugExe);

comp.VerifyDiagnostics();
CompileAndVerify(comp, expectedOutput: "RAN");
}

[Fact]
public void Deconstruct_Simple()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public NotNullIfNotNullAttribute(string parameterName) { }
protected const string IsExternalInitTypeDefinition = @"
namespace System.Runtime.CompilerServices
{
public sealed class IsExternalInit
public static class IsExternalInit
{
}
}
Expand Down

0 comments on commit d702fa6

Please sign in to comment.