The Trunk: Kernel-dtl.586.mcz

> 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).

>    
> + 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)!
>
>
>
>

> On 2011/05/11 00:47, [hidden email] 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
>

>>
>> +       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)!
>>
>>
>>
>>
>
>
>

> 2011/5/11 Frank Shearar<[hidden email]>:
>> On 2011/05/11 00:47, [hidden email] 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

>
> 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.

> 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)!
>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>

> On 2011/05/11 07:23, Nicolas Cellier wrote:
>>
>> 2011/5/11 Frank Shearar<[hidden email]>:
>>>
>>> On 2011/05/11 00:47, [hidden email] 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.

>>
>> 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)!
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>

> On 2011/05/11 00:47, [hidden email] 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?