Record copy ctor must invoke base copy ctor (#45022)

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

@@ -3853,6 +3853,7 @@ private BoundExpression BindConstructorInitializerCore(
             {
                 TypeSymbol constructorReturnType = constructor.ReturnType;
                 Debug.Assert(constructorReturnType.IsVoidType()); //true of all constructors
+                NamedTypeSymbol baseType = containingType.BaseTypeNoUseSiteDiagnostics;
 
                 // Get the bound arguments and the argument names.
                 // : this(__arglist()) is legal
@@ -3949,19 +3950,23 @@ private BoundExpression BindConstructorInitializerCore(
 
                 MemberResolutionResult<MethodSymbol> memberResolutionResult;
                 ImmutableArray<MethodSymbol> candidateConstructors;
-                if (TryPerformConstructorOverloadResolution(
-                    initializerType,
-                    analyzedArguments,
-                    WellKnownMemberNames.InstanceConstructorName,
-                    errorLocation,
-                    false, // Don't suppress result diagnostics
-                    diagnostics,
-                    out memberResolutionResult,
-                    out candidateConstructors,
-                    allowProtectedConstructorsOfBaseType: true))
+                bool found = TryPerformConstructorOverloadResolution(
+                                    initializerType,
+                                    analyzedArguments,
+                                    WellKnownMemberNames.InstanceConstructorName,
+                                    errorLocation,
+                                    false, // Don't suppress result diagnostics
+                                    diagnostics,
+                                    out memberResolutionResult,
+                                    out candidateConstructors,
+                                    allowProtectedConstructorsOfBaseType: true);
+                MethodSymbol resultMember = memberResolutionResult.Member;
+
+                validateRecordCopyConstructor(constructor, baseType, resultMember, errorLocation, diagnostics);
+
+                if (found)
                 {
                     bool hasErrors = false;
-                    MethodSymbol resultMember = memberResolutionResult.Member;
 
                     if (resultMember == constructor)
                     {
@@ -3990,7 +3995,6 @@ private BoundExpression BindConstructorInitializerCore(
                     var arguments = analyzedArguments.Arguments.ToImmutable();
                     var refKinds = analyzedArguments.RefKinds.ToImmutableOrNull();
                     var argsToParamsOpt = memberResolutionResult.Result.ArgsToParamsOpt;
-
                     if (!hasErrors)
                     {
                         hasErrors = !CheckInvocationArgMixing(
@@ -4041,6 +4045,36 @@ private BoundExpression BindConstructorInitializerCore(
             {
                 analyzedArguments.Free();
             }
+
+            static void validateRecordCopyConstructor(MethodSymbol constructor, NamedTypeSymbol baseType, MethodSymbol resultMember, Location errorLocation, DiagnosticBag diagnostics)
+            {
+                if (IsUserDefinedRecordCopyConstructor(constructor))
+                {
+                    if (baseType.SpecialType == SpecialType.System_Object)
+                    {
+                        if (resultMember is null || resultMember.ContainingType.SpecialType != SpecialType.System_Object)
+                        {
+                            // Record deriving from object must use `base()`, not `this()`
+                            diagnostics.Add(ErrorCode.ERR_CopyConstructorMustInvokeBaseCopyConstructor, errorLocation);
+                        }
+
+                        return;
+                    }
+
+                    // Unless the base type is 'object', the constructor should invoke a base type copy constructor
+                    if (resultMember is null || !SynthesizedRecordCopyCtor.HasCopyConstructorSignature(resultMember))
+                    {
+                        diagnostics.Add(ErrorCode.ERR_CopyConstructorMustInvokeBaseCopyConstructor, errorLocation);
+                    }
+                }
+            }
+        }
+
+        internal static bool IsUserDefinedRecordCopyConstructor(MethodSymbol constructor)
+        {
+            return constructor.ContainingType is SourceNamedTypeSymbol sourceType &&
+                sourceType.IsRecord &&
+                SynthesizedRecordCopyCtor.HasCopyConstructorSignature(constructor);
         }
 
         private BoundExpression BindImplicitObjectCreationExpression(ImplicitObjectCreationExpressionSyntax node, DiagnosticBag diagnostics)

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

@@ -3355,8 +3355,10 @@ private BoundNode BindConstructorBody(ConstructorDeclarationSyntax constructor,
             Debug.Assert(bodyBinder != null);
 
             if (constructor.Initializer?.IsKind(SyntaxKind.ThisConstructorInitializer) != true &&
-                ContainingType.GetMembersUnordered().OfType<SynthesizedRecordConstructor>().Any())
+                ContainingType.GetMembersUnordered().OfType<SynthesizedRecordConstructor>().Any() &&
+                !SynthesizedRecordCopyCtor.IsCopyConstructor(this.ContainingMember()))
             {
+                // Note: we check the constructor initializer of copy constructors elsewhere
                 Error(diagnostics, ErrorCode.ERR_UnexpectedOrMissingConstructorInitializerInRecord, constructor.Initializer?.ThisOrBaseKeyword ?? constructor.Identifier);
             }
 

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6253,4 +6253,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_BadRecordMemberForPositionalParameter" xml:space="preserve">
     <value>Record member '{0}' must be a readable instance property of type '{1}' to match positional parameter '{2}'.</value>
   </data>
+  <data name="ERR_NoCopyConstructorInBaseType" xml:space="preserve">
+    <value>No accessible copy constructor found in base type '{0}'.</value>
+  </data>
+  <data name="ERR_CopyConstructorMustInvokeBaseCopyConstructor" xml:space="preserve">
+    <value>A copy constructor in a record must call a copy constructor of the base, or a parameterless object constructor if the record inherits from object.</value>
+  </data>
 </root>

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

@@ -977,7 +977,8 @@ private void CompileMethod(
                     // Do not emit initializers if we are invoking another constructor of this class.
                     includeInitializersInBody = !processedInitializers.BoundInitializers.IsDefaultOrEmpty &&
                                                 !HasThisConstructorInitializer(methodSymbol) &&
-                                                !(methodSymbol is SynthesizedRecordCopyCtor); // A record copy constructor is special, regular initializers are not supposed to be executed by it.
+                                                !(methodSymbol is SynthesizedRecordCopyCtor) &&
+                                                !Binder.IsUserDefinedRecordCopyConstructor(methodSymbol); // A record copy constructor is special, regular initializers are not supposed to be executed by it.
 
                     body = BindMethodBody(methodSymbol, compilationState, diagsForCurrentMethod, out importChain, out originalBodyNested, out forSemanticModel);
                     if (diagsForCurrentMethod.HasAnyErrors() && body != null)
@@ -1846,6 +1847,11 @@ internal static BoundExpression BindImplicitConstructorInitializer(
                 }
             }
 
+            if (constructor is SynthesizedRecordCopyCtor copyCtor)
+            {
+                return GenerateBaseCopyConstructorInitializer(copyCtor, diagnostics);
+            }
+
             // Now, in order to do overload resolution, we're going to need a binder. There are
             // two possible situations:
             //
@@ -1978,6 +1984,53 @@ internal static BoundCall GenerateBaseParameterlessConstructorInitializer(Method
             { WasCompilerGenerated = true };
         }
 
+        private static BoundCall GenerateBaseCopyConstructorInitializer(SynthesizedRecordCopyCtor constructor, DiagnosticBag diagnostics)
+        {
+            NamedTypeSymbol containingType = constructor.ContainingType;
+            NamedTypeSymbol baseType = containingType.BaseTypeNoUseSiteDiagnostics;
+            Location diagnosticsLocation = constructor.Locations.FirstOrNone();
+
+            HashSet<DiagnosticInfo> useSiteDiagnostics = null;
+            MethodSymbol baseConstructor = SynthesizedRecordCopyCtor.FindCopyConstructor(baseType, containingType, ref useSiteDiagnostics);
+
+            if (baseConstructor is null)
+            {
+                diagnostics.Add(ErrorCode.ERR_NoCopyConstructorInBaseType, diagnosticsLocation, baseType);
+                return null;
+            }
+
+            if (Binder.ReportUseSiteDiagnostics(baseConstructor, diagnostics, diagnosticsLocation))
+            {
+                return null;
+            }
+
+            if (!useSiteDiagnostics.IsNullOrEmpty())
+            {
+                diagnostics.Add(diagnosticsLocation, useSiteDiagnostics);
+            }
+
+            CSharpSyntaxNode syntax = constructor.GetNonNullSyntaxNode();
+            BoundExpression receiver = new BoundThisReference(syntax, constructor.ContainingType) { WasCompilerGenerated = true };
+            BoundExpression argument = new BoundParameter(syntax, constructor.Parameters[0]);
+
+            return new BoundCall(
+                syntax: syntax,
+                receiverOpt: receiver,
+                method: baseConstructor,
+                arguments: ImmutableArray.Create(argument),
+                argumentNamesOpt: default,
+                argumentRefKindsOpt: default,
+                isDelegateCall: false,
+                expanded: false,
+                invokedAsExtensionMethod: false,
+                argsToParamsOpt: default,
+                resultKind: LookupResultKind.Viable,
+                binderOpt: null,
+                type: baseConstructor.ReturnType,
+                hasErrors: false)
+            { WasCompilerGenerated = true };
+        }
+
         /// <summary>
         /// Returns true if the method is a constructor and has a this() constructor initializer.
         /// </summary>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1832,6 +1832,8 @@ internal enum ErrorCode
         ERR_BadRecordBase = 8864,
         ERR_BadInheritanceFromRecord = 8865,
         ERR_BadRecordMemberForPositionalParameter = 8866,
+        ERR_NoCopyConstructorInBaseType = 8867,
+        ERR_CopyConstructorMustInvokeBaseCopyConstructor = 8868,
 
         #endregion
 

src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs

@@ -27,7 +27,7 @@ private SourceConstructorSymbol(
              ConstructorDeclarationSyntax syntax,
              MethodKind methodKind,
              DiagnosticBag diagnostics) :
-             base(containingType, location, syntax, methodKind, diagnostics)
+             base(containingType, location, syntax)
         {
             bool hasBlockBody = syntax.Body != null;
             _isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null;

src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs

@@ -19,9 +19,7 @@ internal abstract class SourceConstructorSymbolBase : SourceMemberMethodSymbol
         protected SourceConstructorSymbolBase(
             SourceMemberContainerTypeSymbol containingType,
             Location location,
-            CSharpSyntaxNode syntax,
-            MethodKind methodKind,
-            DiagnosticBag diagnostics)
+            CSharpSyntaxNode syntax)
             : base(containingType, syntax.GetReference(), ImmutableArray.Create(location), SyntaxFacts.HasYieldOperations(syntax))
         {
             Debug.Assert(

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs

@@ -764,6 +764,14 @@ public override bool IsImplicitClass
             }
         }
 
+        internal bool IsRecord
+        {
+            get
+            {
+                return this.declaration.Declarations[0].Kind == DeclarationKind.Record;
+            }
+        }
+
         public override bool IsImplicitlyDeclared
         {
             get

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

@@ -18,7 +18,7 @@ public SynthesizedRecordConstructor(
              SourceMemberContainerTypeSymbol containingType,
              RecordDeclarationSyntax syntax,
              DiagnosticBag diagnostics) :
-             base(containingType, syntax.ParameterList!.GetLocation(), syntax, MethodKind.Constructor, diagnostics)
+             base(containingType, syntax.ParameterList!.GetLocation(), syntax)
         {
             this.MakeFlags(MethodKind.Constructor, containingType.IsAbstract ? DeclarationModifiers.Protected : DeclarationModifiers.Public, returnsVoid: true, isExtensionMethod: false);
         }

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

@@ -4,6 +4,8 @@
 
 #nullable enable
 
+using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using Microsoft.CodeAnalysis.PooledObjects;
 
@@ -30,13 +32,15 @@ public SynthesizedRecordCopyCtor(
 
         public override ImmutableArray<ParameterSymbol> Parameters { get; }
 
+        public override Accessibility DeclaredAccessibility => Accessibility.Protected;
+
         internal override LexicalSortKey GetLexicalSortKey() => LexicalSortKey.GetSynthesizedMemberKey(_memberOffset);
 
         internal override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory F, ArrayBuilder<BoundStatement> statements, DiagnosticBag diagnostics)
         {
             // Tracking issue for copy constructor in inheritance scenario: https://github.com/dotnet/roslyn/issues/44902
             // Write assignments to fields
-            //
+            // .ctor(DerivedRecordType original) : base((BaseRecordType)original)
             // {
             //     this.field1 = parameter.field1
             //     ...
@@ -51,5 +55,42 @@ 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
+            foreach (var member in containingType.InstanceConstructors)
+            {
+                if (HasCopyConstructorSignature(member) &&
+                    !member.HasUnsupportedMetadata &&
+                    AccessCheck.IsSymbolAccessible(member, within, ref useSiteDiagnostics))
+                {
+                    return member;
+                }
+            }
+
+            return null;
+        }
+
+        internal static bool IsCopyConstructor(Symbol member)
+        {
+            if (member is MethodSymbol { MethodKind: MethodKind.Constructor } method)
+            {
+                return HasCopyConstructorSignature(method);
+            }
+
+            return false;
+        }
+
+        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].RefKind == RefKind.None;
+        }
     }
 }

src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

@@ -197,6 +197,11 @@
         <target state="translated">Výraz typu {0} nelze zpracovat vzorem typu {1}. Použijte prosím verzi jazyka {2} nebo vyšší, aby odpovídala otevřenému typu se vzorem konstanty.</target>
         <note />
       </trans-unit>
+      <trans-unit id="ERR_CopyConstructorMustInvokeBaseCopyConstructor">
+        <source>A copy constructor in a record must call a copy constructor of the base, or a parameterless object constructor if the record inherits from object.</source>
+        <target state="new">A copy constructor in a record must call a copy constructor of the base, or a parameterless object constructor if the record inherits from object.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="ERR_DeconstructParameterNameMismatch">
         <source>The name '{0}' does not match the corresponding 'Deconstruct' parameter '{1}'.</source>
         <target state="translated">Název {0} neodpovídá příslušnému parametru Deconstruct {1}.</target>
@@ -492,6 +497,11 @@
         <target state="translated">{0}: Typ použitý v příkazu using musí být implicitně převoditelný na System.IDisposable. Neměli jste v úmyslu použít await using místo using?</target>
         <note />
       </trans-unit>
+      <trans-unit id="ERR_NoCopyConstructorInBaseType">
+        <source>No accessible copy constructor found in base type '{0}'.</source>
+        <target state="new">No accessible copy constructor found in base type '{0}'.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="ERR_NoOutputDirectory">
         <source>Output directory could not be determined</source>
         <target state="new">Output directory could not be determined</target>

src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

@@ -197,6 +197,11 @@
         <target state="translated">Ein Ausdruck vom Typ "{0}" kann nicht von einem Muster vom Typ "{1}" behandelt werden. Verwenden Sie Sprachversion {2} oder höher, um einen offenen Typ mit einem konstanten Muster abzugleichen.</target>
         <note />
       </trans-unit>
+      <trans-unit id="ERR_CopyConstructorMustInvokeBaseCopyConstructor">
+        <source>A copy constructor in a record must call a copy constructor of the base, or a parameterless object constructor if the record inherits from object.</source>
+        <target state="new">A copy constructor in a record must call a copy constructor of the base, or a parameterless object constructor if the record inherits from object.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="ERR_DeconstructParameterNameMismatch">
         <source>The name '{0}' does not match the corresponding 'Deconstruct' parameter '{1}'.</source>
         <target state="translated">Der Name "{0}" stimmt nicht mit dem entsprechenden Deconstruct-Parameter "{1}" überein.</target>
@@ -492,6 +497,11 @@
         <target state="translated">"{0}": Der in einer using-Anweisung verwendete Typ muss implizit in "System.IDisposable" konvertierbar sein. Meinten Sie "await using" anstelle von "using"?</target>
         <note />
       </trans-unit>
+      <trans-unit id="ERR_NoCopyConstructorInBaseType">
+        <source>No accessible copy constructor found in base type '{0}'.</source>
+        <target state="new">No accessible copy constructor found in base type '{0}'.</target>
+        <note />
+      </trans-unit>
       <trans-unit id="ERR_NoOutputDirectory">
         <source>Output directory could not be determined</source>
         <target state="new">Output directory could not be determined</target>