Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 2fb32bc

Browse files
committed
Fix OrderByDescending if comparer returns int.MinValue
int.MinValue is a valid "x is less than y" result for a comparer but OrderByDescending treats it incorrectly. Fix this. Fix #2239 Simplify comparison result inverting. As per suggestion at #2240 (comment) Simpler expression. As suggested at #2240 (comment) This reduces descending fix to a single branch in all cases. Parentheses to clarify precedence in logic. Replace boolean ^ with != Logically equivalent, and arguably better known. Silly typo in comment.
1 parent 0c6b87e commit 2fb32bc

File tree

2 files changed

+24
-1
lines changed

2 files changed

+24
-1
lines changed

src/System.Linq/src/System/Linq/Enumerable.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3105,7 +3105,10 @@ internal override int CompareKeys(int index1, int index2)
31053105
if (next == null) return index1 - index2;
31063106
return next.CompareKeys(index1, index2);
31073107
}
3108-
return descending ? -c : c;
3108+
// -c will result in a negative value for int.MinValue (-int.MinValue == int.MinValue).
3109+
// Flipping keys earlier is more likely to trigger something strange in a comparer,
3110+
// particularly as it comes to the sort being stable.
3111+
return (descending != (c > 0)) ? 1 : -1;
31093112
}
31103113
}
31113114

src/System.Linq/tests/EnumerableTests.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,26 @@ public void Max()
194194
Assert.Equal(-10, minusTen.Max());
195195
Assert.Equal(1000, thousand.Max());
196196
}
197+
198+
private class ExtremeComparer : IComparer<int>
199+
{
200+
public int Compare(int x, int y)
201+
{
202+
if (x == y)
203+
return 0;
204+
if (x < y)
205+
return int.MinValue;
206+
return int.MaxValue;
207+
}
208+
}
209+
210+
[Fact]
211+
public void OrderByExtremeComparer()
212+
{
213+
var outOfOrder = new[] { 7, 1, 0, 9, 3, 5, 4, 2, 8, 6 };
214+
Assert.Equal(Enumerable.Range(0, 10), outOfOrder.OrderBy(i => i, new ExtremeComparer()));
215+
Assert.Equal(Enumerable.Range(0, 10).Reverse(), outOfOrder.OrderByDescending(i => i, new ExtremeComparer()));
216+
}
197217
}
198218
}
199219

0 commit comments

Comments
 (0)