Skip to content

Fix Delegate.GetHashCode for type-equivalent delegates#130542

Merged
AaronRobinsonMSFT merged 19 commits into
mainfrom
copilot/add-test-for-delegate-equality
Jul 14, 2026
Merged

Fix Delegate.GetHashCode for type-equivalent delegates#130542
AaronRobinsonMSFT merged 19 commits into
mainfrom
copilot/add-test-for-delegate-equality

Conversation

Copilot AI commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Fix Delegate.GetHashCode to return same hashcodes for type-equivalent delegates pointing to the same method. Delegate.Equals returns true for such delegates. GetHashCode contract requires hashcode to be the same in that case.

Found during codereview of #99200

Copilot AI and others added 2 commits July 11, 2026 01:02
…gate types

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
…bute

Replace plain DelegateTypeA/DelegateTypeB (same signature but different names,
which can never be type-equivalent) with a proper cross-assembly type-equivalent
delegate pair using [TypeIdentifier] attribute as defined in .NET type equivalence
docs.

- Add TestDelegateEquivalentTypeAssembly with System.Tests.EquivalentDelegate
  decorated with [TypeIdentifier("guid-scope", "guid-identifier")]
- Declare System.Tests.EquivalentDelegate with the same [TypeIdentifier] values
  in DelegateTests.cs (namespace-level, not nested, to avoid enclosing-type
  equivalence requirements)
- Update tests to use [ConditionalFact(IsTypeEquivalenceSupported)] and load the
  second assembly at runtime via Assembly.LoadFrom
- Add ProjectReference in System.Runtime.Tests.csproj so the assembly is built
  and deployed alongside the test DLL

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DelegateTests.cs Outdated
…ests

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI requested review from Copilot and jkotas and removed request for Copilot July 11, 2026 04:57
…em.TestEquivalentTypes; remove AutoGenerateBindingRedirects

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

@jkotas jkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot Address feedback

Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DelegateTests.cs Outdated
Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DelegateTests.cs Outdated
Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DelegateTests.cs Outdated
Comment thread src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DelegateTests.cs Outdated
…ivalentTo asserts

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot July 11, 2026 13:26

Copilot AI commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Address feedback

All feedback addressed in the latest commit:

  • TestEquivalentTypes/System.TestEquivalentTypes.cs is now added to System.Runtime.Tests.csproj via <Compile Include> — the EquivalentDelegate definition at the bottom of DelegateTests.cs has been removed.
  • The two tests are merged into TypeEquivalentDelegatesPointingToSameMethod_AreEqualAndHaveSameHashCode.
  • Added Assert.False(typeof(EquivalentDelegate).Equals(otherDelegateType)) and Assert.True(typeof(EquivalentDelegate).IsEquivalentTo(otherDelegateType)).
  • Switched from Type.GetType to Assembly.Load("System.TestEquivalentTypes").GetType(...) to reliably load the satellite assembly and fix the Assert.NotNull CI failure.

Co-authored-by: Michał Petryka <35800402+MichalPetryka@users.noreply.github.com>
@jkotas jkotas temporarily deployed to copilot-pat-pool July 13, 2026 03:03 — with GitHub Actions Inactive
@jkotas jkotas temporarily deployed to copilot-pat-pool July 13, 2026 03:03 — with GitHub Actions Inactive
@jkotas jkotas marked this pull request as ready for review July 13, 2026 03:03
@jkotas jkotas changed the title Fix delegate type equivalence tests to use proper TypeIdentifier-based equivalence Fix Delegate.GetHashCode for type equivalent delegates Jul 13, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs
… instead of 0

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@jkotas I've opened a new pull request, #130597, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs
Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs Outdated
Co-authored-by: Michał Petryka <35800402+MichalPetryka@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs
@MichalPetryka

Copy link
Copy Markdown
Contributor

Can we merge this when CI is done so that I can rebase my other PR?

Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs
Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs Outdated
Comment thread src/coreclr/System.Private.CoreLib/src/System/Delegate.CoreCLR.cs Outdated
Co-authored-by: Aaron R Robinson <arobins@microsoft.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

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.

5 participants