Align addition of a synthesized override of object.GetHashCode() in records with the latest design.#45518
Conversation
…ecords with the latest design. Related to dotnet#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.
src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs
Show resolved
Hide resolved
| /// <summary> | ||
| /// Returns true if reported an error | ||
| /// </summary> | ||
| internal static bool VerifyOerridesMethodFromObject(MethodSymbol overriding, DiagnosticBag diagnostics) |
There was a problem hiding this comment.
VerifyOerridesMethodFromObject [](start = 29, length = 30)
Typo: Oerrides->Overrides #Pending
| 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) |
There was a problem hiding this comment.
Implement [](start = 197, length = 9)
Should this be Implementation? #Pending
There was a problem hiding this comment.
Should this be Implementation?
Sounds fine to me as it is, are you proposing to make it longer? Do you think someone might get confused? If you'd like I can do a rename in a separate PR since this name wasn't introduced in this PR
In reply to: 447226886 [](ancestors = 447226886)
There was a problem hiding this comment.
Do you think someone might get confused?
Yes. I don't know what Implement means here. I assume it means implementation, but it's not clear.
If you'd like I can do a rename in a separate PR since this name wasn't introduced in this PR
That's fine. I'll mark this active again for you to keep track.
In reply to: 447269453 [](ancestors = 447269453,447226886)
src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordObjectMethod.cs
Show resolved
Hide resolved
|
@cston, @jcouv , @RikkiGibson, @dotnet/roslyn-compiler, @agocke Please review |
333fred
left a comment
There was a problem hiding this comment.
LGTM (commit 1) with renaming feedback either here or in a followup PR.
|
@cston, @jcouv , @RikkiGibson, @dotnet/roslyn-compiler, @agocke Please review, need a second sign-off |
2 similar comments
|
@cston, @jcouv , @RikkiGibson, @dotnet/roslyn-compiler, @agocke Please review, need a second sign-off |
|
@cston, @jcouv , @RikkiGibson, @dotnet/roslyn-compiler, @agocke Please review, need a second sign-off |
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.