TreeSubSet.Min and TreeSubSet.Max work incorrectly with custom comparer

[petrogavriluk]

Description

The code below gives the wrong result:

var comparer = Comparer<int>.Create((x, y) => x - y);  
var set = new SortedSet<int>(new[]{1, 4}, comparer);  
var view = set.GetViewBetween(1, 2);  
Console.WriteLine(view.Max); // Prints 4 instead of 1  

According to the documentation of IComparer.Compare() it can return any integer, not only [-1, 0, 1]. So this comparer should be ok.

Configuration

I run it on .net 5, but I think it is present in other versions as well.

Regression?

A similar issue was mentioned in 20474 (without comparer though). So I think it is regression.

Other information

I have found the reason why it happens in code:

runtime/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs

Line 135 in 57bfe47

if (comp == 1)

runtime/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs

Line 164 in 57bfe47

if (comp == -1)

The comparison result is checked against 1 and -1, instead of (comp > 0) and (comp < 0).

[ghost]

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

The code below gives the wrong result:

var comparer = Comparer<int>.Create((x, y) => x - y);  
var set = new SortedSet<int>(new[]{1, 4}, comparer);  
var view = set.GetViewBetween(1, 2);  
Console.WriteLine(view.Max); // Prints 4 instead of 1  

According to the documentation of IComparer.Compare() it can return any integer, not only [-1, 0, 1]. So this comparer should be ok.

Configuration

I run it on .net 5, but I think it is present in other versions as well.

Regression?

A similar issue was mentioned in 20474 (without comparer though). So I think it is regression.

Other information

I have found the reason why it happens in code:

runtime/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs

Line 135 in 57bfe47

if (comp == 1)

runtime/src/libraries/System.Collections/src/System/Collections/Generic/SortedSet.TreeSubSet.cs

Line 164 in 57bfe47

if (comp == -1)

The comparison result is checked against 1 and -1, instead of (comp > 0) and (comp < 0).

Author:	petrogavriluk
Assignees:	-
Labels:	area-System.Collections, untriaged
Milestone:	-

[eiriktsarpalis]

This seems like a bug introduced by dotnet/corefx#16771. It seems like a trivial fix, would you be willing to contribute a PR?

[petrogavriluk]

I haven't contributed here yet, but yes, I can try.
Should I also add tests?

[eiriktsarpalis]

Thanks, assigning the issue to you. A couple of tests reproducing your scenario for both min and max should suffice.

Please let me know if you have any questions on the dev workflow.