On Sun, Jul 10, 2011 at 10:53 AM, Levente Uzonyi <leves at elte.hu> wrote:

> On Mon, 11 Jul 2011, Ben Coman wrote:
>
>  I am writing some code that uses OrderedCollection and was tracing through
>> it with the debugger when the debugger itself threw a DNU exception.  I
>> narrowed down reproduction of this to executing the following line, clicking
>> <Debug> then eight times <Into> :
>>
>>>    self halt. OrderedCollection new: 5.
>>>
>>
>> As far as I can determine, the root cause is that
>> "OrderedCollectionInspector>>**fieldList" calls "object size" which has
>> the following implementation:
>>
>>>    OrderedCollection>>size
>>>       ^ lastIndex - firstIndex + 1
>>>
>>
>> where "lastIndex" is nil (and also "firstIndex" is nil).
>> To narrow it down further, the error can also be reproduced by executing
>> the following:
>>
>>> OrderedCollectionInspector openOn: OrderedCollection basicNew.
>>>
>>
>> I fixed this with:
>>
>>>    OrderedCollection>>size
>>>       lastIndex ifNil: [ ^0 ].
>>>       ^ lastIndex - firstIndex + 1
>>>
>>
>> It would be more correct to also have included "firstIndex ifNil: [ ^0 ]"
>> but I assume both conditions occur at the same time, so I left it out for
>> performance. Not knowing how performance issue are balanced in Squeak, I was
>> concerned that the "size" method might be called very often, and that even
>> this one extra line might want to be avoided - I assume that  this case only
>> occurs in debugging at times when the object has not been fully initialized,
>> and this error would not occur from application code.  So I went looking for
>> an
>>
>
> Right.
>
>  alternative.  Back in the caller, rather than:
>>
>>>    OrderedCollection>>fieldList
>>>        object ifNil: [ ^ OrderedCollection new].
>>>        ^ self baseFieldList ,
>>>            (object size <= (self i1 + self i2)
>>>
>> <snip>
>>
>> this corrected the problem:
>>
>>>    OrderedCollection>>fieldList
>>>        object ifNil: [ ^ OrderedCollection new].
>>>       [ object size ] on: Exception do: [ ^ self baseFieldList ].
>>>        ^ self baseFieldList ,
>>>            (object size <= (self i1 + self i2)
>>>
>> <snip>
>> I assume this would also be required for OrderedCollection>>**
>> selectedObjectIndex.
>>
>
> Yes, OrderedCollectionInspector should be changed, because that's what
> causes the problem. I'd simply create an #objectSize method for it and use
> that instead of object size.
>
>  However now in the debugger inst-var sub-pane, selecting self shows
>> "<error in printString: evaluate "self printString" to debug>, and in so
>> doing reveals that the "self size" call fails in:
>>
>>>    OrderedCollection>>do: elementBlock separatedBy: separatorBlock
>>>        1 to: self size do:
>>>
>>
> IMHO this is not a problem. #printString doesn't have to work for
> uninitialized objects.
>

+100.  One can't distort production code for the benefit of debugging.  One
can make the debugger robust, e.g. in the presence of faulty printing code,
and the debugger and inspector already are robust in this way.  I don't see
the need of any changes here (other than the arguable firstIndex ifNil:
[^0]".


>
>
> Levente
>
>  indicating a similar fix is potentially required for the following
>> methods:
>>  OrderedCollection>>add: newObject afterIndex: index
>>  OrderedCollection>>add: newObject beforeIndex: index
>>  OrderedCollection>>at: index ifAbsentPut: block
>>  OrderedCollection>>collect: aBlock
>>  OrderedCollection>>**copyReplaceFrom: start to: stop with:
>> replacementCollection
>>  OrderedCollection>>**makeRoomAtFirst
>>  OrderedCollection>>**makeRoomAtLast
>>  OrderedCollection>>**postCopyFrom: startIndex to: endIndex
>>  OrderedCollection>>with: otherCollection collect: twoArgBlock
>>  OrderedCollection>>**withIndexCollect: elementAndIndexBlock
>> which is not as clean as modifying OrderedCollection>>size, and still a
>> bit uncertain.
>>
>> What is the community's thoughts on this?
>>
>>
>> Finally, if any of these are desirable fixes, I would appreciate using my
>> first contribution as a practice for submitting to the Inbox (if someone
>> could direct me to some documentation that describes how to do this.)
>>
>> cheers, Ben
>>
>>
>>
>>
>>
>>
>>
>>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20110710/0e2efde6/attachment.htm