Skip to content

Fix ImmutableHashSet.SetEquals correctness for mismatched comparers#127633

Open
aw0lid wants to merge 1 commit intodotnet:mainfrom
aw0lid:fix/ImmutableHashSetequals-comparer-logic
Open

Fix ImmutableHashSet.SetEquals correctness for mismatched comparers#127633
aw0lid wants to merge 1 commit intodotnet:mainfrom
aw0lid:fix/ImmutableHashSetequals-comparer-logic

Conversation

@aw0lid
Copy link
Copy Markdown
Contributor

@aw0lid aw0lid commented May 1, 2026

Summary

This PR fixes a correctness issue in SetEquals introduced in my previous PR #126309.

The Count check was moved inside the Comparer equality block. This ensures that when comparers differ, we don't return a false negative and instead fall back to the safe path.

Example of the issue fixed:

// This should return true, but was returning false
var main = ImmutableHashSet.Create(StringComparer.OrdinalIgnoreCase, "a");
var other = ImmutableHashSet.Create("a", "A");
Console.WriteLine(main.SetEquals(other));

Changes

  • Moved if (count != origin.Count) inside the EqualityComparer<IEqualityComparer<T>>.Default.Equals check.
  • This ensures mismatched comparers safely bypass the fast-path and proceed to a proper set comparison.

Testing

In addition to the fix, I have added comprehensive unit tests covering various scenarios to ensure correctness:

  • Mismatched Comparers (Ordinal vs. OrdinalIgnoreCase): Verified that SetEquals returns true when logically equal but with different comparers, and false when logically different.
  • ICollection with Duplicates: Verified the fallback path correctly handles collections like List<T> with duplicate elements.
  • Count Optimizations:
    • Verified that mismatched comparers correctly bypass the fast-path count check.
    • Verified that SetEquals still performs early-exit when other.Count < origin.Count.
  • Fast-Path Validation: Ensured that when comparers match, the optimized count-based comparison still works as expected.
  • Edge Cases: Included tests for empty sets with different comparers and content-specific mismatches.

Related to: #126309

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label May 1, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

@aw0lid aw0lid force-pushed the fix/ImmutableHashSetequals-comparer-logic branch from e3cbd3f to 67de92b Compare May 1, 2026 12:11
@aw0lid aw0lid force-pushed the fix/ImmutableHashSetequals-comparer-logic branch from 67de92b to c831c55 Compare May 3, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Collections community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants