Align addition of a synthesized override of Base.Equals(Base? other) …

…in records with the latest design. (#45601)

Related to #45296.

From specification:
If the record type is derived from a base record type Base, the record type includes a synthesized override of the strongly-typed Equals(Base other). The synthesized override is sealed. It is an error if the override is declared explicitly. The synthesized override returns Equals((object?)other).

This change also adjusts code-gen to properly compare EqualityContracts.

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6278,9 +6278,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <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>
   <data name="ERR_DoesNotOverrideMethodFromObject" xml:space="preserve">
-    <value>'{0}' does not override the method from 'object'.</value>
+    <value>'{0}' does not override expected method from 'object'.</value>
   </data>
   <data name="ERR_SealedGetHashCodeInRecord" xml:space="preserve">
     <value>'{0}' cannot be sealed because containing 'record' is not sealed.</value>
   </data>
+  <data name="ERR_DoesNotOverrideBaseEquals" xml:space="preserve">
+    <value>'{0}' does not override expected method from '{1}'.</value>
+  </data>
 </root>

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

@@ -1856,6 +1856,7 @@ internal enum ErrorCode
         ERR_CopyConstructorMustInvokeBaseCopyConstructor = 8868,
         ERR_DoesNotOverrideMethodFromObject = 8869,
         ERR_SealedGetHashCodeInRecord = 8870,
+        ERR_DoesNotOverrideBaseEquals = 8871,
 
         #endregion diagnostics introduced for C# 9.0
         // Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)

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

@@ -3002,7 +3002,7 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde
             getOtherEquals(otherEqualsMethods, equalityContract);
 
             var thisEquals = addThisEquals(equalityContract, otherEqualsMethod: otherEqualsMethods.Count == 0 ? null : otherEqualsMethods[0]);
-            addOtherEquals(otherEqualsMethods, equalityContract, thisEquals);
+            addOtherEquals();
             addObjectEquals(thisEquals);
             addHashCode(equalityContract);
 
@@ -3148,15 +3148,15 @@ void addHashCode(PropertySymbol equalityContract)
                     ImmutableArray<CustomModifier>.Empty,
                     ImmutableArray<MethodSymbol>.Empty);
 
-                if (!memberSignatures.TryGetValue(targetMethod, out Symbol? contender))
+                if (!memberSignatures.TryGetValue(targetMethod, out Symbol? existingHashCodeMethod))
                 {
                     var hashCode = new SynthesizedRecordGetHashCode(this, equalityContract, memberOffset: members.Count, diagnostics);
                     members.Add(hashCode);
                 }
                 else
                 {
-                    var method = (MethodSymbol)contender;
-                    if (!SynthesizedRecordObjectMethod.VerifyOerridesMethodFromObject(method, diagnostics) && method.IsSealed && !IsSealed)
+                    var method = (MethodSymbol)existingHashCodeMethod;
+                    if (!SynthesizedRecordObjectMethod.VerifyOverridesMethodFromObject(method, SpecialType.System_Int32, diagnostics) && method.IsSealed && !IsSealed)
                     {
                         diagnostics.Add(ErrorCode.ERR_SealedGetHashCodeInRecord, method.Locations[0], method);
                     }
@@ -3210,21 +3210,12 @@ static void getOtherEquals(ArrayBuilder<MethodSymbol> otherEqualsMethods, Proper
                 }
             }
 
-            void addOtherEquals(ArrayBuilder<MethodSymbol> otherEqualsMethods, PropertySymbol equalityContract, MethodSymbol thisEquals)
+            void addOtherEquals()
             {
-                foreach (var otherEqualsMethod in otherEqualsMethods)
+                if (!BaseTypeNoUseSiteDiagnostics.IsObjectType())
                 {
-                    var method = new SynthesizedRecordEquals(
-                        this,
-                        parameterType: otherEqualsMethod.Parameters[0].Type,
-                        isOverride: true,
-                        equalityContract,
-                        otherEqualsMethod: thisEquals,
-                        memberOffset: members.Count);
-                    if (!memberSignatures.ContainsKey(method))
-                    {
-                        members.Add(method);
-                    }
+                    var method = new SynthesizedRecordBaseEquals(this, memberOffset: members.Count, diagnostics);
+                    members.Add(method);
                 }
             }
         }

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

@@ -106,7 +106,7 @@ protected override ImmutableArray<TypeParameterSymbol> MakeTypeParameters(CSharp
             }
         }
 
-        protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplement) MakeParametersAndBindReturnType(DiagnosticBag diagnostics)
+        protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplementation) MakeParametersAndBindReturnType(DiagnosticBag diagnostics)
         {
             var syntax = GetSyntax();
             var withTypeParamsBinder = this.DeclaringCompilation.GetBinderFactory(syntax.SyntaxTree).GetBinder(syntax.ReturnType, syntax, this);

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

@@ -243,7 +243,7 @@ protected override void MethodChecks(DiagnosticBag diagnostics)
             return;
         }
 
-        protected abstract (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplement) MakeParametersAndBindReturnType(DiagnosticBag diagnostics);
+        protected abstract (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplementation) MakeParametersAndBindReturnType(DiagnosticBag diagnostics);
 
         protected abstract void ExtensionMethodChecks(DiagnosticBag diagnostics);
 

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

@@ -0,0 +1,86 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+#nullable enable
+
+using System.Collections.Immutable;
+using System.Diagnostics;
+using System.Linq;
+
+namespace Microsoft.CodeAnalysis.CSharp.Symbols
+{
+    /// <summary>
+    /// If the record type is derived from a base record type Base, the record type includes
+    /// a synthesized override of the strongly-typed Equals(Base other). The synthesized
+    /// override is sealed. It is an error if the override is declared explicitly.
+    /// The synthesized override returns Equals((object?)other).
+    /// </summary>
+    internal sealed class SynthesizedRecordBaseEquals : SynthesizedRecordOrdinaryMethod
+    {
+        public SynthesizedRecordBaseEquals(SourceMemberContainerTypeSymbol containingType, int memberOffset, DiagnosticBag diagnostics)
+            : base(containingType, WellKnownMemberNames.ObjectEquals, memberOffset, diagnostics)
+        {
+        }
+
+        protected override DeclarationModifiers MakeDeclarationModifiers(DeclarationModifiers allowedModifiers, DiagnosticBag diagnostics)
+        {
+            const DeclarationModifiers result = DeclarationModifiers.Public | DeclarationModifiers.Override | DeclarationModifiers.Sealed;
+            Debug.Assert((result & ~allowedModifiers) == 0);
+            return result;
+        }
+
+        protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplementation) MakeParametersAndBindReturnType(DiagnosticBag diagnostics)
+        {
+            var compilation = DeclaringCompilation;
+            var location = ReturnTypeLocation;
+            return (ReturnType: TypeWithAnnotations.Create(Binder.GetSpecialType(compilation, SpecialType.System_Boolean, location, diagnostics)),
+                    Parameters: ImmutableArray.Create<ParameterSymbol>(
+                                    new SourceSimpleParameterSymbol(owner: this,
+                                                                    TypeWithAnnotations.Create(ContainingType.BaseTypeNoUseSiteDiagnostics, NullableAnnotation.Annotated),
+                                                                    ordinal: 0, RefKind.None, "", isDiscard: false, Locations)),
+                    IsVararg: false,
+                    DeclaredConstraintsForOverrideOrImplementation: ImmutableArray<TypeParameterConstraintClause>.Empty);
+        }
+
+        protected override int GetParameterCountFromSyntax() => 1;
+
+        protected override void MethodChecks(DiagnosticBag diagnostics)
+        {
+            base.MethodChecks(diagnostics);
+
+            var overridden = OverriddenMethod;
+
+            if (overridden is null)
+            {
+                return;
+            }
+
+            if (overridden is object &&
+                !overridden.ContainingType.Equals(ContainingType.BaseTypeNoUseSiteDiagnostics, TypeCompareKind.AllIgnoreOptions))
+            {
+                diagnostics.Add(ErrorCode.ERR_DoesNotOverrideBaseEquals, Locations[0], this, ContainingType.BaseTypeNoUseSiteDiagnostics);
+            }
+        }
+
+        internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics)
+        {
+            var F = new SyntheticBoundNodeFactory(this, this.SyntaxNode, compilationState, diagnostics);
+
+            try
+            {
+                var retExpr = F.Call(
+                    F.This(),
+                    ContainingType.GetMembersUnordered().OfType<SynthesizedRecordObjEquals>().Single(),
+                    F.Convert(F.SpecialType(SpecialType.System_Object), F.Parameter(Parameters[0])));
+
+                F.CloseMethod(F.Block(F.Return(retExpr)));
+            }
+            catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
+            {
+                diagnostics.Add(ex.Diagnostic);
+                F.CloseMethod(F.ThrowNull());
+            }
+        }
+    }
+}

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

@@ -178,8 +178,17 @@ internal override IEnumerable<SecurityAttribute> GetSecurityInformation()
             internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics)
             {
                 var F = new SyntheticBoundNodeFactory(this, this.GetNonNullSyntaxNode(), compilationState, diagnostics);
-                F.CurrentFunction = this;
-                F.CloseMethod(F.Block(F.Return(F.Typeof(ContainingType))));
+
+                try
+                {
+                    F.CurrentFunction = this;
+                    F.CloseMethod(F.Block(F.Return(F.Typeof(ContainingType))));
+                }
+                catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
+                {
+                    diagnostics.Add(ex.Diagnostic);
+                    F.CloseMethod(F.ThrowNull());
+                }
             }
         }
     }

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

@@ -167,87 +167,101 @@ internal override IEnumerable<SecurityAttribute> GetSecurityInformation()
         internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics)
         {
             var F = new SyntheticBoundNodeFactory(this, ContainingType.GetNonNullSyntaxNode(), compilationState, diagnostics);
-            var other = F.Parameter(Parameters[0]);
-            BoundExpression? retExpr;
 
-            if (IsOverride)
+            try
             {
-                // This method is an override of a strongly-typed Equals method from a base record type.
-                // The definition of the method is as follows, and _otherEqualsMethod
-                // is the method to delegate to (see B.Equals(A), C.Equals(A), C.Equals(B) above):
-                //
-                // override bool Equals(Base other) => Equals(other as Derived);
-                retExpr = F.Call(
-                    F.This(),
-                    _otherEqualsMethod!,
-                    F.As(other, ContainingType));
-            }
-            else
-            {
-                // This method is the strongly-typed Equals method where the parameter type is
-                // the containing type.
+                var other = F.Parameter(Parameters[0]);
+                BoundExpression? retExpr;
 
-                if (_otherEqualsMethod is null)
-                {
-                    // There are no base record types.
-                    // The definition of the method is as follows (see A.Equals(A) above):
-                    //
-                    // virtual bool Equals(T other) =>
-                    //     other != null &&
-                    //     EqualityContract == other.EqualityContract &&
-                    //     field1 == other.field1 && ... && fieldN == other.fieldN;
-
-                    // other != null
-                    Debug.Assert(!other.Type.IsStructType());
-                    retExpr = F.ObjectNotEqual(other, F.Null(F.SpecialType(SpecialType.System_Object)));
-
-                    // EqualityContract == other.EqualityContract
-                    var contractsEqual = F.Binary(
-                        BinaryOperatorKind.ObjectEqual,
-                        F.SpecialType(SpecialType.System_Boolean),
-                        F.Property(F.This(), _equalityContract),
-                        F.Property(other, _equalityContract));
-
-                    retExpr = retExpr is null ? contractsEqual : F.LogicalAnd(retExpr, contractsEqual);
-                }
-                else
+                if (IsOverride)
                 {
-                    // There are base record types.
+                    // This method is an override of a strongly-typed Equals method from a base record type.
                     // The definition of the method is as follows, and _otherEqualsMethod
-                    // is the corresponding method on the nearest base record type to
-                    // delegate to (see B.Equals(B), C.Equals(C) above):
+                    // is the method to delegate to (see B.Equals(A), C.Equals(A), C.Equals(B) above):
                     //
-                    // virtual bool Equals(Derived other) =>
-                    //     base.Equals((Base)other) &&
-                    //     field1 == other.field1 && ... && fieldN == other.fieldN;
+                    // override bool Equals(Base other) => Equals(other as Derived);
                     retExpr = F.Call(
-                        F.Base(_otherEqualsMethod.ContainingType),
+                        F.This(),
                         _otherEqualsMethod!,
-                        F.Convert(_otherEqualsMethod.Parameters[0].Type, other));
+                        F.As(other, ContainingType));
                 }
-
-                // field1 == other.field1 && ... && fieldN == other.fieldN
-                // https://github.com/dotnet/roslyn/issues/44895: Should compare fields from non-record base classes.
-                var fields = ArrayBuilder<FieldSymbol>.GetInstance();
-                foreach (var f in ContainingType.GetFieldsToEmit())
+                else
                 {
-                    if (!f.IsStatic)
+                    // This method is the strongly-typed Equals method where the parameter type is
+                    // the containing type.
+
+                    if (_otherEqualsMethod is null)
                     {
-                        fields.Add(f);
+                        // There are no base record types.
+                        // The definition of the method is as follows (see A.Equals(A) above):
+                        //
+                        // virtual bool Equals(T other) =>
+                        //     other != null &&
+                        //     EqualityContract == other.EqualityContract &&
+                        //     field1 == other.field1 && ... && fieldN == other.fieldN;
+
+                        // other != null
+                        Debug.Assert(!other.Type.IsStructType());
+                        retExpr = F.ObjectNotEqual(other, F.Null(F.SpecialType(SpecialType.System_Object)));
+
+                        // EqualityContract == other.EqualityContract
+                        var contractsEqual = F.Call(receiver: null, F.WellKnownMethod(WellKnownMember.System_Type__op_Equality),
+                                                    F.Property(F.This(), _equalityContract),
+                                                    F.Property(other, _equalityContract));
+
+                        retExpr = retExpr is null ? (BoundExpression)contractsEqual : F.LogicalAnd(retExpr, contractsEqual);
                     }
+                    else
+                    {
+                        if (_otherEqualsMethod.ReturnType.SpecialType != SpecialType.System_Boolean)
+                        {
+                            // There was a problem with overriding, an error was reported elsewhere
+                            F.CloseMethod(F.ThrowNull());
+                            return;
+                        }
+
+                        // There are base record types.
+                        // The definition of the method is as follows, and _otherEqualsMethod
+                        // is the corresponding method on the nearest base record type to
+                        // delegate to (see B.Equals(B), C.Equals(C) above):
+                        //
+                        // virtual bool Equals(Derived other) =>
+                        //     base.Equals((Base)other) &&
+                        //     field1 == other.field1 && ... && fieldN == other.fieldN;
+                        retExpr = F.Call(
+                            F.Base(_otherEqualsMethod.ContainingType),
+                            _otherEqualsMethod!,
+                            F.Convert(_otherEqualsMethod.Parameters[0].Type, other));
+                    }
+
+                    // field1 == other.field1 && ... && fieldN == other.fieldN
+                    // https://github.com/dotnet/roslyn/issues/44895: Should compare fields from non-record base classes.
+                    var fields = ArrayBuilder<FieldSymbol>.GetInstance();
+                    foreach (var f in ContainingType.GetFieldsToEmit())
+                    {
+                        if (!f.IsStatic)
+                        {
+                            fields.Add(f);
+                        }
+                    }
+                    if (fields.Count > 0)
+                    {
+                        retExpr = MethodBodySynthesizer.GenerateFieldEquals(
+                            retExpr,
+                            other,
+                            fields,
+                            F);
+                    }
+                    fields.Free();
                 }
-                if (fields.Count > 0)
-                {
-                    retExpr = MethodBodySynthesizer.GenerateFieldEquals(
-                        retExpr,
-                        other,
-                        fields,
-                        F);
-                }
-                fields.Free();
-            }
 
-            F.CloseMethod(F.Block(F.Return(retExpr)));
+                F.CloseMethod(F.Block(F.Return(retExpr)));
+            }
+            catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
+            {
+                diagnostics.Add(ex.Diagnostic);
+                F.CloseMethod(F.ThrowNull());
+            }
         }
     }
 }