On 08.09.2011 14:37, Michael Roberts wrote:
So I found the bug with the first assignment. It is caused by a difference between squeak and Pharo in SortedCollection. (on my phone so will be brief)
In Pharo the debugger map collection gets sorted and asked for an index for a given pc. It is supposed to answer 1 because there is no match and it is expected to be the first index for insertion. However when the asSortedCollection gets done the internal start index of the collection is 2. This manifests the bug. 2 is answered as the insertion position but this is a value internal to the collection.
Squeak does not have this bug because it forces a reset of the start index to 1. Hope that makes sense. I don't know what the correct fix is. We copy the reset:1 type code to force the collection to have start from 1 property. We check semantics of index for insertion. Is it the position of the element of index relative to pointers? Or we change the debugger to calculate the offset (but this feels wrong)
So this is an opportunity for some test cases and fixes around sorted collection if we know what the correct semantics are. This leak of '2' out of the collection is only wrong if 2 is a valid state. If that makes sense...
Cheers Mike Hum, indexForInserting: is _supposed_ to answer the internal index... Depending on the startIndex being 1 for doing the correct at: later is just wrong. Squeak potentially has the same highlight bug for different methds, as insert:before: used when converting to SortedCollection in no way ensures startIndex remains 1 just because it started as it.
I assume it was written this way for performance reasons, now that we have #findBinary: aBlock, we can rewrite the code achieving same performance using an ordinary collection, something like: "Maintain a sorted array of intervals" sortedSourceMap ifNil: [sortedSourceMap := self abstractSourceMap values sorted]. "Look up which interval contains pc using a binary search" sortedSourceMap findBinary: [: one | (one includes: pc) ifTrue: [0] ifFalse: [one first > pc ifTrue: [-1] ifFalse: [1]] ] (with some extra code needed to deal with the cases where pc < first interval or > last interval) Cheers, Henry