Align addition of a synthesized override of object.Equals(object? obj…

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

* Align addition of a synthesized override of object.Equals(object? obj) in records with the latest design.

Related to #45296.

From specification:
The record type includes a synthesized override of object.Equals(object? obj). It is an error if the override is declared explicitly. The synthesized override returns Equals(other as R) where R is the record type.

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6277,4 +6277,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <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>
+  <data name="ERR_DoesNotOverrideMethodFromObject" xml:space="preserve">
+    <value>'{0}' does not override the method from 'object'.</value>
+  </data>
 </root>

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

@@ -1846,6 +1846,7 @@ internal enum ErrorCode
         ERR_BadRecordMemberForPositionalParameter = 8866,
         ERR_NoCopyConstructorInBaseType = 8867,
         ERR_CopyConstructorMustInvokeBaseCopyConstructor = 8868,
+        ERR_DoesNotOverrideMethodFromObject = 8869,
 
         #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

@@ -2325,7 +2325,7 @@ private static void AddNestedTypesToDictionary(Dictionary<string, ImmutableArray
 
         private class MembersAndInitializersBuilder
         {
-            public ArrayBuilder<Symbol> NonTypeNonIndexerMembers { get; private set; } = ArrayBuilder<Symbol>.GetInstance();
+            public ArrayBuilder<Symbol> NonTypeNonIndexerMembers { get; set; } = ArrayBuilder<Symbol>.GetInstance();
             public readonly ArrayBuilder<ArrayBuilder<FieldOrPropertyInitializer.Builder>> StaticInitializers = ArrayBuilder<ArrayBuilder<FieldOrPropertyInitializer.Builder>>.GetInstance();
             public readonly ArrayBuilder<ArrayBuilder<FieldOrPropertyInitializer.Builder>> InstanceInitializers = ArrayBuilder<ArrayBuilder<FieldOrPropertyInitializer.Builder>>.GetInstance();
             public readonly ArrayBuilder<SyntaxReference> IndexerDeclarations = ArrayBuilder<SyntaxReference>.GetInstance();
@@ -2965,8 +2965,8 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde
             ParameterListSyntax? paramList = builder.RecordDeclarationWithParameters?.ParameterList;
 
             var memberSignatures = s_duplicateMemberSignatureDictionary.Allocate();
-            var members = builder.NonTypeNonIndexerMembers;
-            foreach (var member in members)
+            var members = ArrayBuilder<Symbol>.GetInstance(builder.NonTypeNonIndexerMembers.Count + 1);
+            foreach (var member in builder.NonTypeNonIndexerMembers)
             {
                 if (!memberSignatures.ContainsKey(member))
                 {
@@ -3007,6 +3007,13 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde
             otherEqualsMethods.Free();
             memberSignatures.Free();
 
+
+            // We put synthesized record members first so that errors about conflicts show up on user-defined members rather than all
+            // go to the record declaration
+            members.AddRange(builder.NonTypeNonIndexerMembers);
+            builder.NonTypeNonIndexerMembers.Free();
+            builder.NonTypeNonIndexerMembers = members;
+
             return;
 
             SynthesizedRecordConstructor addCtor(RecordDeclarationSyntax declWithParameters)
@@ -3128,12 +3135,8 @@ ImmutableArray<PropertySymbol> addProperties(ImmutableArray<ParameterSymbol> rec
 
             void addObjectEquals(MethodSymbol thisEquals)
             {
-                var objEquals = new SynthesizedRecordObjEquals(this, thisEquals, memberOffset: members.Count);
-                if (!memberSignatures.ContainsKey(objEquals))
-                {
-                    // https://github.com/dotnet/roslyn/issues/44617: Don't add if the overridden method is sealed
-                    members.Add(objEquals);
-                }
+                var objEquals = new SynthesizedRecordObjEquals(this, thisEquals, memberOffset: members.Count, diagnostics);
+                members.Add(objEquals);
             }
 
             void addHashCode(PropertySymbol equalityContract)

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> DeclaredConstraintsForOverrideOrImplement) MakeParametersAndBindReturnType(DiagnosticBag diagnostics)
         {
             var syntax = GetSyntax();
             var withTypeParamsBinder = this.DeclaringCompilation.GetBinderFactory(syntax.SyntaxTree).GetBinder(syntax.ReturnType, syntax, this);
@@ -206,12 +206,11 @@ static void forceMethodTypeParameters(TypeWithAnnotations type, SourceOrdinaryMe
 
         protected override void ExtensionMethodChecks(DiagnosticBag diagnostics)
         {
-            var syntax = GetSyntax();
-            var location = locations[0];
-
             // errors relevant for extension methods
             if (IsExtensionMethod)
             {
+                var syntax = GetSyntax();
+                var location = locations[0];
                 var parameter0Type = this.Parameters[0].TypeWithAnnotations;
                 var parameter0RefKind = this.Parameters[0].RefKind;
                 if (!parameter0Type.Type.IsValidExtensionParameterType())

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

@@ -16,6 +16,11 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
+    /// <summary>
+    /// Unlike <see cref="SourceOrdinaryMethodSymbol"/>, this type doesn't depend
+    /// on any specific kind of syntax node associated with it. Any syntax node is good enough
+    /// for it.
+    /// </summary>
     internal abstract class SourceOrdinaryMethodSymbolBase : SourceMemberMethodSymbol
     {
         private readonly ImmutableArray<TypeParameterSymbol> _typeParameters;
@@ -238,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> DeclaredConstraintsForOverrideOrImplement) MakeParametersAndBindReturnType(DiagnosticBag diagnostics);
 
         protected abstract void ExtensionMethodChecks(DiagnosticBag diagnostics);
 
@@ -287,9 +292,7 @@ private void CompleteAsyncMethodChecks(DiagnosticBag diagnosticsOpt, Cancellatio
             }
         }
 
-        protected virtual void CompleteAsyncMethodChecksBetweenStartAndFinish()
-        {
-        }
+        protected abstract void CompleteAsyncMethodChecksBetweenStartAndFinish();
 
         public override ImmutableArray<TypeParameterSymbol> TypeParameters
         {

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

@@ -4,119 +4,43 @@
 
 #nullable enable
 
-using System;
-using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Reflection;
-using Microsoft.Cci;
+using System.Diagnostics;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
-    internal sealed class SynthesizedRecordObjEquals : SynthesizedInstanceMethodSymbol
+    internal sealed class SynthesizedRecordObjEquals : SynthesizedRecordObjectMethod
     {
         private readonly MethodSymbol _typedRecordEquals;
-        private readonly int _memberOffset;
 
-        public override NamedTypeSymbol ContainingType { get; }
-
-        public override ImmutableArray<ParameterSymbol> Parameters { get; }
-
-        public SynthesizedRecordObjEquals(NamedTypeSymbol containingType, MethodSymbol typedRecordEquals, int memberOffset)
+        public SynthesizedRecordObjEquals(SourceMemberContainerTypeSymbol containingType, MethodSymbol typedRecordEquals, int memberOffset, DiagnosticBag diagnostics)
+            : base(containingType, "Equals", memberOffset, diagnostics)
         {
-            var compilation = containingType.DeclaringCompilation;
             _typedRecordEquals = typedRecordEquals;
-            _memberOffset = memberOffset;
-            ContainingType = containingType;
-            Parameters = ImmutableArray.Create<ParameterSymbol>(SynthesizedParameterSymbol.Create(
-                this,
-                TypeWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Object), NullableAnnotation.Annotated),
-                ordinal: 0,
-                RefKind.None));
-            ReturnTypeWithAnnotations = TypeWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Boolean));
         }
 
-        public override string Name => "Equals";
-
-        public override MethodKind MethodKind => MethodKind.Ordinary;
-
-        public override int Arity => 0;
-
-        public override bool IsExtensionMethod => false;
-
-        public override bool HidesBaseMethodsByName => false;
-
-        public override bool IsVararg => false;
-
-        public override bool ReturnsVoid => false;
-
-        public override bool IsAsync => false;
-
-        public override RefKind RefKind => RefKind.None;
-
-        internal override LexicalSortKey GetLexicalSortKey() => LexicalSortKey.GetSynthesizedMemberKey(_memberOffset);
-
-        public override TypeWithAnnotations ReturnTypeWithAnnotations { get; }
-
-        public override FlowAnalysisAnnotations ReturnTypeFlowAnalysisAnnotations => FlowAnalysisAnnotations.None;
-
-        public override ImmutableHashSet<string> ReturnNotNullIfParameterNotNull => ImmutableHashSet<string>.Empty;
-
-        public override ImmutableArray<TypeWithAnnotations> TypeArgumentsWithAnnotations
-            => ImmutableArray<TypeWithAnnotations>.Empty;
-
-        public override ImmutableArray<TypeParameterSymbol> TypeParameters => ImmutableArray<TypeParameterSymbol>.Empty;
-
-        public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations => ImmutableArray<MethodSymbol>.Empty;
-
-        public override ImmutableArray<CustomModifier> RefCustomModifiers => ImmutableArray<CustomModifier>.Empty;
-
-        public override Symbol? AssociatedSymbol => null;
-
-        public override Symbol ContainingSymbol => ContainingType;
-
-        public override ImmutableArray<Location> Locations => ContainingType.Locations;
-
-        public override Accessibility DeclaredAccessibility => Accessibility.Public;
-
-        public override bool IsStatic => false;
-
-        public override bool IsVirtual => false;
-
-        public override bool IsOverride => true;
-
-        public override bool IsAbstract => false;
-
-        public override bool IsSealed => false;
-
-        public override bool IsExtern => false;
-
-        internal override bool HasSpecialName => false;
-
-        internal override MethodImplAttributes ImplementationAttributes => MethodImplAttributes.Managed;
-
-        internal override bool HasDeclarativeSecurity => false;
-
-        internal override MarshalPseudoCustomAttributeData? ReturnValueMarshallingInformation => null;
-
-        internal override bool RequiresSecurityObject => false;
-
-        internal override CallingConvention CallingConvention => CallingConvention.HasThis;
-
-        internal override bool GenerateDebugInfo => false;
-
-        public override DllImportData? GetDllImportData() => null;
-
-        internal override ImmutableArray<string> GetAppliedConditionalSymbols()
-            => ImmutableArray<string>.Empty;
-
-        internal override IEnumerable<SecurityAttribute> GetSecurityInformation()
-            => Array.Empty<SecurityAttribute>();
-
-        internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false;
+        protected override DeclarationModifiers MakeDeclarationModifiers(DeclarationModifiers allowedModifiers, DiagnosticBag diagnostics)
+        {
+            const DeclarationModifiers result = DeclarationModifiers.Public | DeclarationModifiers.Override;
+            Debug.Assert((result & ~allowedModifiers) == 0);
+            return result;
+        }
 
-        internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => true;
+        protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplement) 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(Binder.GetSpecialType(compilation, SpecialType.System_Object, location, diagnostics), NullableAnnotation.Annotated),
+                                                                    ordinal: 0, RefKind.None, "obj", isDiscard: false, Locations)),
+                    IsVararg: false,
+                    DeclaredConstraintsForOverrideOrImplement: ImmutableArray<TypeParameterConstraintClause>.Empty);
+        }
 
-        internal override bool SynthesizesLoweredBoundBody => true;
+        protected override int GetParameterCountFromSyntax() => 1;
 
         internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics)
         {
@@ -127,18 +51,7 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState,
             BoundExpression expression;
             if (ContainingType.IsStructType())
             {
-                // For structs:
-                //
-                //      return param is ContainingType i ? this.Equals(in i) : false;
-                expression = F.Conditional(
-                    F.Is(paramAccess, ContainingType),
-                    F.Call(
-                        F.This(),
-                        _typedRecordEquals,
-                        ImmutableArray.Create<RefKind>(RefKind.In),
-                        ImmutableArray.Create<BoundExpression>(F.Convert(ContainingType, paramAccess))),
-                    F.Literal(false),
-                    F.SpecialType(SpecialType.System_Boolean));
+                throw ExceptionUtilities.Unreachable;
             }
             else
             {

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

@@ -0,0 +1,40 @@
+// 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
+
+namespace Microsoft.CodeAnalysis.CSharp.Symbols
+{
+    /// <summary>
+    /// Common base for ordinary methods overriding methods from object synthesized by compiler for records.
+    /// </summary>
+    internal abstract class SynthesizedRecordObjectMethod : SynthesizedRecordOrdinaryMethod
+    {
+        protected SynthesizedRecordObjectMethod(SourceMemberContainerTypeSymbol containingType, string name, int memberOffset, DiagnosticBag diagnostics)
+            : base(containingType, name, memberOffset, diagnostics)
+        {
+        }
+
+        protected sealed override void MethodChecks(DiagnosticBag diagnostics)
+        {
+            base.MethodChecks(diagnostics);
+
+            var overridden = OverriddenMethod?.OriginalDefinition;
+
+            if (overridden is null || (overridden is SynthesizedRecordObjEquals && overridden.DeclaringCompilation == DeclaringCompilation))
+            {
+                return;
+            }
+
+            MethodSymbol leastOverridden = GetLeastOverriddenMethod(accessingTypeOpt: null);
+
+            if (leastOverridden is object &&
+                leastOverridden.ReturnType.SpecialType == ReturnType.SpecialType &&
+                leastOverridden.ContainingType.SpecialType != SpecialType.System_Object)
+            {
+                diagnostics.Add(ErrorCode.ERR_DoesNotOverrideMethodFromObject, Locations[0], this);
+            }
+        }
+    }
+}