MSortedCollection.IndexOf returns -1 always when there are duplicates

[pqnelson]

Expected Behaviour

The MSortedCollection.IndexOf should return the index for the rightmost occurrence of the "needle" in the "haystack".

Current Behaviour

When we have an MSortedCollection with duplicates, IndexOf will always return -1.

Possible Solution

The current implementation has a buggy while-loop, I think the fix would be to change the code to something like:

function MSortedCollection.IndexOf(aItem: Pointer): Integer;
var I: Integer;
begin
  IndexOf := -1;
  if Search(KeyOf(aItem), I) then
  begin
    if Duplicates then
      while (I+1 < Count) and (0 = fCompare(aItem, Items^[I+1])) do Inc(I);
    IndexOf := I; {since I+1 <= Count, we have I < Count always}
  end;
end;

Steps to Reproduce

Right now, I have written a unit test which has this method producing -1 for the collection ['a', 'b', 'c', 'c', 'd'] when looking for 'c' and there are duplicates (but correctly returns 2 when there are no duplicates):

{with duplicates}
procedure TMObjectsTests.MSortedCollectionIndexOfPresentTest1;
var
   entry, needle: PStr;
   key: Pointer;
   list: PSortedCollection;
   actual, expected: integer;
begin
   {setup}
   list := New(PSortedCollection, InitSorted(5,5, compareMStr));
   list.duplicates := True;
   entry := New(PStr, Init('a'));
   list.insert(entry);
   entry := New(PStr, Init('b'));
   list.insert(entry);
   entry := New(PStr, Init('c'));
   list.insert(entry);
   entry := New(PStr, Init('c'));
   list.insert(entry);
   entry := New(PStr, Init('d'));
   list.insert(entry);

   needle := New(PStr, Init('c'));
   key := needle;
   {test}
   expected:=3;
   actual := list.IndexOf(key);
   AssertEquals('The first entry is not assigned',expected,actual);
   {teardown}
   list.FreeItemsFrom(0);
   Dispose(PObject(key),Done);
end;

This unit test fails when duplicates are allowed.

[pqnelson]

I don't know what the "expected behaviour should be" for duplicate entries, whether it is looking for the specific pointer, or if it is looking for a pointer such that fCompare(key, pointer) = 0, so the proposed solution may not match the specification for the method.