Skip to content

fix: GetHashCode in generated string comparers hash the value instead of the comparer#946

Merged
SteveDunn merged 4 commits into
SteveDunn:mainfrom
sirphilliptubell:fix/string-comparer-gethashcode-bug
Jun 29, 2026
Merged

fix: GetHashCode in generated string comparers hash the value instead of the comparer#946
SteveDunn merged 4 commits into
SteveDunn:mainfrom
sirphilliptubell:fix/string-comparer-gethashcode-bug

Conversation

@sirphilliptubell

Copy link
Copy Markdown

…not the comparer

__StringEqualityComparer.GetHashCode was calling _comparer.GetHashCode() with no argument, returning object.GetHashCode() on the comparer instance itself. Every value object therefore got the same hash code when using the generated Comparers, making hash-based collections degenerate to O(n).

Fixed to pass obj._value to the comparer. Added regression tests to each StringComparisons test class asserting that two unequal values produce different hash codes.

…not the comparer

__StringEqualityComparer.GetHashCode was calling _comparer.GetHashCode() with no argument, returning object.GetHashCode() on the comparer instance itself. Every value object therefore got the same hash code when using the generated Comparers, making hash-based collections degenerate to O(n).

Fixed to pass obj._value to the comparer. Added regression tests to each StringComparisons test class asserting that two unequal values produce different hash codes.
@SteveDunn

Copy link
Copy Markdown
Owner

Hi @sirphilliptubell - many thanks for the PR, and good spot re. the bug!

Would you mind running .\RunSnapshots.ps1 -v "minimal" -reset - this will reset the snapshots and the build should then be green.

Thanks again!

@SteveDunn SteveDunn left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution - much appreciated!

@SteveDunn
SteveDunn merged commit aba303f into SteveDunn:main Jun 29, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants