Skip to content

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

Merged
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:Records_11
Jun 29, 2020
Merged

Align addition of a synthesized override of object.Equals(object? obj) in records with the latest design.#45475
AlekseyTs merged 2 commits intodotnet:masterfrom
AlekseyTs:Records_11

Conversation

@AlekseyTs
Copy link
Contributor

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.

…) in records with the latest design.

Related to dotnet#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.
@AlekseyTs
Copy link
Contributor Author

@jcouv, @agocke, @cston , @RikkiGibson, @dotnet/roslyn-compiler Please review.

: base(containingType, "Equals", containingType.Locations[0], (CSharpSyntaxNode)containingType.SyntaxReferences[0].GetSyntax(), MethodKind.Ordinary,
isIterator: false, isExtensionMethod: false, isPartial: false, hasBody: true, diagnostics)
{
var compilation = containingType.DeclaringCompilation;
Copy link
Contributor Author

@AlekseyTs AlekseyTs Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var compilation = containingType.DeclaringCompilation; [](start = 12, length = 54)

This line is no longer needed. #Closed



// 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
Copy link
Member

@333fred 333fred Jun 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go [](start = 15, length = 2)

Small nit: go -> going #Pending

Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (commit 2)

@AlekseyTs
Copy link
Contributor Author

@jcouv, @agocke, @cston, @dotnet/roslyn-compiler Please review, need a second sign-off.


var overridden = OverriddenMethod?.OriginalDefinition;

if (overridden is null || (overridden is SynthesizedRecordObjEquals && overridden.DeclaringCompilation == DeclaringCompilation))
Copy link
Contributor Author

@AlekseyTs AlekseyTs Jun 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SynthesizedRecordObjEquals [](start = 53, length = 26)

This should be adjusted to handle a general case, but this PR doesn't have any. Fixed in the change that builds on top of this PR. #Resolved

@AlekseyTs
Copy link
Contributor Author

@jcouv, @agocke, @cston, @dotnet/roslyn-compiler Please review, need a second sign-off.

@AlekseyTs AlekseyTs merged commit 35370eb into dotnet:master Jun 29, 2020
@ghost ghost added this to the Next milestone Jun 29, 2020
333fred added a commit to 333fred/roslyn that referenced this pull request Jun 29, 2020
* upstream/master: (1226 commits)
  Remove unnecessary Clone() (dotnet#45469)
  Align addition of a synthesized override of object.Equals(object? obj) in records with the latest design. (dotnet#45475)
  Move SymbolSearch down to EditorFeatures (dotnet#45505)
  VisitType in MethodToClassRewriter for function pointers.
  Fix up nondeterminism in serializing naming style preferences
  Update dependencies from https://github.com/dotnet/arcade build 20200626.2 (dotnet#45482)
  Fix typo
  Move to vnext
  Add constant inerpolated strings to the test plan, update status for records.
  Don't emit ldftn when the result is unused.
  PR Feedback: * Additional tests for nested function contexts. * Override VisitFunctionPointerLoad in MethodToClassRewriter. * Adjust debug asserts.
  Add records to compiler test plan (dotnet#45434)
  Expand comment in CreateRecoverableText
  Replace binary serialization of encoding with a custom serializer. (dotnet#45374)
  LangVersion 9 (dotnet#44911)
  Avoid loading document text in AbstractObjectBrowserLibraryManager.DocumentChangedAsync
  Allow TryGetTextVersion to pass through to the initial source
  Ensure recoverable text is in temporary storage
  Fix test
  Updates the option page type GUID to match the one in pkgdef
  ...
333fred added a commit that referenced this pull request Jun 30, 2020
…e_168

* upstream/master: (102 commits)
  Change contrast ratio to get close to 1.5:1 (#45526)
  Revert "Move SymbolSearch down to EditorFeatures (#45505)"
  Delay accessibility checks to avoid cycles (#45441)
  Prevent trying to convert metadata references into circular project references
  Remove unnecessary Clone() (#45469)
  Align addition of a synthesized override of object.Equals(object? obj) in records with the latest design. (#45475)
  Move SymbolSearch down to EditorFeatures (#45505)
  VisitType in MethodToClassRewriter for function pointers.
  Fix up nondeterminism in serializing naming style preferences
  Update dependencies from https://github.com/dotnet/arcade build 20200626.2 (#45482)
  Fix typo
  Move to vnext
  Add constant inerpolated strings to the test plan, update status for records.
  Don't emit ldftn when the result is unused.
  PR Feedback: * Additional tests for nested function contexts. * Override VisitFunctionPointerLoad in MethodToClassRewriter. * Adjust debug asserts.
  Add records to compiler test plan (#45434)
  Expand comment in CreateRecoverableText
  Replace binary serialization of encoding with a custom serializer. (#45374)
  LangVersion 9 (#44911)
  Avoid loading document text in AbstractObjectBrowserLibraryManager.DocumentChangedAsync
  ...
@dibarbet dibarbet modified the milestones: Next, 16.7.P4 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants