2011/5/11 Frank Shearar <frank.shearar at angband.za.org>:
> On 2011/05/11 07:23, Nicolas Cellier wrote:
>>
>> 2011/5/11 Frank Shearar<frank.shearar at angband.za.org>:
>>>
>>> On 2011/05/11 00:47, commits at source.squeak.org wrote:
>>>>
>>>> David T. Lewis uploaded a new version of Kernel to project The Trunk:
>>>> http://source.squeak.org/trunk/Kernel-dtl.586.mcz
>>>>
>>>> ==================== Summary ====================
>>>>
>>>> Name: Kernel-dtl.586
>>>> Author: dtl
>>>> Time: 10 May 2011, 8:47:40.644 pm
>>>> UUID: 08000000-1508-8a15-1508-8a1514000000
>>>> Ancestors: Kernel-fbs.585
>>>>
>>>> Igor's fix for CompiledMethodTrailer>>encodeVarLengthSourcePointer, see
>>>> CompiledMethodTrailerTest>>testEncodingZeroSourcePointer for test
>>>>
>>>> and<http://lists.squeakfoundation.org/pipermail/squeak-dev/2011-May/159822.html>
>>>>  for discussion.
>>>>
>>>> =============== Diff against Kernel-fbs.585 ===============
>>>>
>>>> Item was changed:
>>>>   ----- Method: CompiledMethodTrailer>>encodeVarLengthSourcePointer (in
>>>> category 'encoding') -----
>>>>   encodeVarLengthSourcePointer
>>>>
>>>>        "source pointer must be>=0"
>>>> +       [data>= 0] assert.
>>>> -       (self assert: data>= 0).
>>>
>>> Igor obviously did find an actual problem, but how do we reach this point
>>> with data = 0? The above assert raises an exception... which is
>>> resumable.
>>> So I guess something's resuming the AssertionFailure?
>>>
>>> frank
>>>
>>
>> No, the assertion does not protect against data=0, only against data<0
>
> Excuse me while I go insert my brain. You're quite right.

This is revealing because I made the same mistake when I first read the diff.

Nicolas

>>
>> Using resumable Error is indeed questionnable, but this is not
>> specific to this method.
>>
>> Also, the use of an assertion block may seem cleaner (I think the goal
>> of Pharo was to remove Object:>>assert).
>> But it creates a BlockClosure. This could cost some CPU cycles in low
>> level tight loops.
>> We could as well defne Boolean>>assert.
>>
>> Of course, with a Block, execution is conditionnal, and we could
>> remove all assertions at once by modifying BlockClosure>>#assert.
>> But IMO this is not the way to do it, because you cannot decide for
>> third party code.
>
> In my humble opinion one should _never_ remove asserts like that. The old
> argument goes that one uses asserts when developing and then removes them in
> production. That sounds great, but the _real_ bugs only manifest in
> production, when you would _most_need_ those asserts!
>
> frank
>
>> Nicolas
>>
>>>>
>>>> +       encodedData :=
>>>> +               data = 0 ifTrue: [ #[0] ]
>>>> +               ifFalse: [ ByteArray streamContents: [:str |
>>>> -       encodedData := ByteArray streamContents: [:str |
>>>>                | value |
>>>>                value := data.
>>>>                [value>    0] whileTrue: [
>>>>                        value>    127 ifTrue: [ str nextPut: 128 + (value
>>>> bitAnd: 16r7F) ]
>>>>                                ifFalse: [ str nextPut: value. ].
>>>>                        value := value>>    7.
>>>>                        ].
>>>> +               ]].
>>>> -               ].
>>>>        encodedData := encodedData reversed copyWith: (self kindAsByte)!
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>