Handle custom modifiers on copy ctor (#45136)

src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordCopyCtor.cs

@@ -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)
@@ -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;
         }
     }

src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs

@@ -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()
         {
@@ -5111,6 +5162,227 @@ public void CopyCtor_InaccessibleInMetadata()
                 );
         }
 
+        [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()
         {

src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs

@@ -211,7 +211,7 @@ public sealed class NotNullIfNotNullAttribute : Attribute
         protected const string IsExternalInitTypeDefinition = @"
 namespace System.Runtime.CompilerServices
 {
-    public sealed class IsExternalInit
+    public static class IsExternalInit
     {
     }
 }