Skip to content

Commit

Permalink
Align addition of a synthesized override of object.GetHashCode() in r…
Browse files Browse the repository at this point in the history
…ecords with the latest design. (#45518)

Related to #45296.

From specification:
The record type includes a synthesized override of object.GetHashCode(). The method can be declared explicitly. It is an error if the explicit declaration is sealed unless the record type is sealed.
  • Loading branch information
AlekseyTs authored Jul 1, 2020
1 parent ca0aea9 commit f4d410e
Show file tree
Hide file tree
Showing 20 changed files with 847 additions and 114 deletions.
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6280,4 +6280,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_DoesNotOverrideMethodFromObject" xml:space="preserve">
<value>'{0}' does not override the 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>
</root>
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1855,6 +1855,7 @@ internal enum ErrorCode
ERR_NoCopyConstructorInBaseType = 8867,
ERR_CopyConstructorMustInvokeBaseCopyConstructor = 8868,
ERR_DoesNotOverrideMethodFromObject = 8869,
ERR_SealedGetHashCodeInRecord = 8870,

#endregion diagnostics introduced for C# 9.0
// Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2974,6 +2974,8 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde
}
}

CSharpCompilation compilation = this.DeclaringCompilation;

// Positional record
if (!(paramList is null))
{
Expand Down Expand Up @@ -3009,7 +3011,7 @@ private void AddSynthesizedRecordMembersIfNecessary(MembersAndInitializersBuilde


// 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
// going to the record declaration
members.AddRange(builder.NonTypeNonIndexerMembers);
builder.NonTypeNonIndexerMembers.Free();
builder.NonTypeNonIndexerMembers = members;
Expand Down Expand Up @@ -3133,12 +3135,32 @@ void addObjectEquals(MethodSymbol thisEquals)

void addHashCode(PropertySymbol equalityContract)
{
var hashCode = new SynthesizedRecordGetHashCode(this, equalityContract, memberOffset: members.Count);
if (!memberSignatures.ContainsKey(hashCode))
{
// https://github.com/dotnet/roslyn/issues/44617: Don't add if the overridden method is sealed
var targetMethod = new SignatureOnlyMethodSymbol(
WellKnownMemberNames.ObjectGetHashCode,
this,
MethodKind.Ordinary,
Cci.CallingConvention.HasThis,
ImmutableArray<TypeParameterSymbol>.Empty,
ImmutableArray<ParameterSymbol>.Empty,
RefKind.None,
isInitOnly: false,
TypeWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Int32)),
ImmutableArray<CustomModifier>.Empty,
ImmutableArray<MethodSymbol>.Empty);

if (!memberSignatures.TryGetValue(targetMethod, out Symbol? contender))
{
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)
{
diagnostics.Add(ErrorCode.ERR_SealedGetHashCodeInRecord, method.Locations[0], method);
}
}
}

PropertySymbol addEqualityContract()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,106 +12,31 @@

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class SynthesizedRecordGetHashCode : SynthesizedInstanceMethodSymbol
internal sealed class SynthesizedRecordGetHashCode : SynthesizedRecordObjectMethod
{
private readonly int _memberOffset;
private readonly PropertySymbol _equalityContract;

public override NamedTypeSymbol ContainingType { get; }

public SynthesizedRecordGetHashCode(NamedTypeSymbol containingType, PropertySymbol equalityContract, int memberOffset)
public SynthesizedRecordGetHashCode(SourceMemberContainerTypeSymbol containingType, PropertySymbol equalityContract, int memberOffset, DiagnosticBag diagnostics)
: base(containingType, WellKnownMemberNames.ObjectGetHashCode, memberOffset, diagnostics)
{
_memberOffset = memberOffset;
ContainingType = containingType;
_equalityContract = equalityContract;
}

public override string Name => "GetHashCode";

public override ImmutableArray<ParameterSymbol> Parameters => ImmutableArray<ParameterSymbol>.Empty;

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 => TypeWithAnnotations.Create(
isNullableEnabled: true,
ContainingType.DeclaringCompilation.GetSpecialType(SpecialType.System_Int32));

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;

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_Int32, location, diagnostics)),
Parameters: ImmutableArray<ParameterSymbol>.Empty,
IsVararg: false,
DeclaredConstraintsForOverrideOrImplement: ImmutableArray<TypeParameterConstraintClause>.Empty);
}

internal override bool SynthesizesLoweredBoundBody => true;
protected override int GetParameterCountFromSyntax() => 0;

internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#nullable enable

using System.Collections.Immutable;
using System.Diagnostics;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
Expand All @@ -15,18 +14,11 @@ internal sealed class SynthesizedRecordObjEquals : SynthesizedRecordObjectMethod
private readonly MethodSymbol _typedRecordEquals;

public SynthesizedRecordObjEquals(SourceMemberContainerTypeSymbol containingType, MethodSymbol typedRecordEquals, int memberOffset, DiagnosticBag diagnostics)
: base(containingType, "Equals", memberOffset, diagnostics)
: base(containingType, WellKnownMemberNames.ObjectEquals, memberOffset, diagnostics)
{
_typedRecordEquals = typedRecordEquals;
}

protected override DeclarationModifiers MakeDeclarationModifiers(DeclarationModifiers allowedModifiers, DiagnosticBag diagnostics)
{
const DeclarationModifiers result = DeclarationModifiers.Public | DeclarationModifiers.Override;
Debug.Assert((result & ~allowedModifiers) == 0);
return result;
}

protected override (TypeWithAnnotations ReturnType, ImmutableArray<ParameterSymbol> Parameters, bool IsVararg, ImmutableArray<TypeParameterConstraintClause> DeclaredConstraintsForOverrideOrImplement) MakeParametersAndBindReturnType(DiagnosticBag diagnostics)
{
var compilation = DeclaringCompilation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#nullable enable

using System.Diagnostics;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
/// <summary>
Expand All @@ -16,25 +18,49 @@ protected SynthesizedRecordObjectMethod(SourceMemberContainerTypeSymbol containi
{
}

protected sealed override DeclarationModifiers MakeDeclarationModifiers(DeclarationModifiers allowedModifiers, DiagnosticBag diagnostics)
{
const DeclarationModifiers result = DeclarationModifiers.Public | DeclarationModifiers.Override;
Debug.Assert((result & ~allowedModifiers) == 0);
return result;
}

protected sealed override void MethodChecks(DiagnosticBag diagnostics)
{
base.MethodChecks(diagnostics);
VerifyOerridesMethodFromObject(this, diagnostics);
}

var overridden = OverriddenMethod?.OriginalDefinition;
/// <summary>
/// Returns true if reported an error
/// </summary>
internal static bool VerifyOerridesMethodFromObject(MethodSymbol overriding, DiagnosticBag diagnostics)
{
bool reportAnError = false;

if (overridden is null || (overridden is SynthesizedRecordObjEquals && overridden.DeclaringCompilation == DeclaringCompilation))
if (!overriding.IsOverride)
{
return;
reportAnError = true;
}
else
{
var overridden = overriding.OverriddenMethod?.OriginalDefinition;

MethodSymbol leastOverridden = GetLeastOverriddenMethod(accessingTypeOpt: null);
if (overridden is object && !(overridden.ContainingType is SourceMemberContainerTypeSymbol { IsRecord: true } && overridden.ContainingModule == overriding.ContainingModule))
{
MethodSymbol leastOverridden = overriding.GetLeastOverriddenMethod(accessingTypeOpt: null);

if (leastOverridden is object &&
leastOverridden.ReturnType.SpecialType == ReturnType.SpecialType &&
leastOverridden.ContainingType.SpecialType != SpecialType.System_Object)
reportAnError = leastOverridden.ReturnType.SpecialType == overriding.ReturnType.SpecialType &&
leastOverridden.ContainingType.SpecialType != SpecialType.System_Object;
}
}

if (reportAnError)
{
diagnostics.Add(ErrorCode.ERR_DoesNotOverrideMethodFromObject, Locations[0], this);
diagnostics.Add(ErrorCode.ERR_DoesNotOverrideMethodFromObject, overriding.Locations[0], overriding);
}

return reportAnError;
}
}
}
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">Cílový modul runtime nepodporuje pro člena rozhraní přístupnost na úrovni Protected, Protected internal nebo Private protected.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">Die Zugriffsoptionen "protected", "protected internal" oder "private protected" werden von der Zielruntime für einen Member einer Schnittstelle nicht unterstützt.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">El entorno de ejecución de destino no admite la accesibilidad protegida, protegida interna o protegida privada para un miembro de una interfaz.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">Le runtime cible ne prend pas en charge l'accessibilité 'protected', 'protected internal' ou 'private protected' d'un membre d'interface.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">Il runtime di destinazione non supporta l'accessibilità 'protected', 'protected internal' o 'private protected' per un membro di un'interfaccia.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">ターゲット ランタイムは、インターフェイスのメンバーに対して 'protected'、'protected internal'、'private protected' アクセシビリティをサポートしていません。</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">대상 런타임이 인터페이스 멤버의 'protected', 'protected internal' 또는 'private protected' 접근성을 지원하지 않습니다.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,11 @@
<target state="translated">Docelowe środowisko uruchomieniowe nie obsługuje specyfikatorów dostępu „protected”, „protected internal” i „private protected” dla składowej interfejsu.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SealedGetHashCodeInRecord">
<source>'{0}' cannot be sealed because containing 'record' is not sealed.</source>
<target state="new">'{0}' cannot be sealed because containing 'record' is not sealed.</target>
<note />
</trans-unit>
<trans-unit id="ERR_SimpleProgramDisallowsMainType">
<source>Cannot specify /main if there is a compilation unit with top-level statements.</source>
<target state="new">Cannot specify /main if there is a compilation unit with top-level statements.</target>
Expand Down
Loading

0 comments on commit f4d410e

Please sign in to comment.