Hi guys. When we implemented this possibility of pruning the closures when
they were clean, one of our crazy tests started to fail. It is not that the
test is very important, but I am interested in trying to understand why it
fails. The test (simplied) is:

    | aSortedCollection materialized |
    instanceVariableForTesting := false.

    aSortedCollection := SortedCollection sortBlock: [:a :b |
        instanceVariableForTesting
            ifTrue: [ a <= b ]
            ifFalse: [ a >= b ] ].

    materialized := self resultOfSerializeAndMaterialize: aSortedCollection.

    materialized addAll: #(2 3 1).
    aSortedCollection addAll: #(2 3 1).


Well, the thing is that the materialized SortedCollection, it is sortBLock,
the closure is quite crazy and the 'instanceVariableForTesting' doesn't
have the boolean but instead a #Processor    WTF??   So then the #addAll:
fails with a MustBeBoolean.

Do you think there is something I should check for this closure?  I think
it may have to be related with the instanceVariable...

Just for the record, what we are doing is something like:

serializeReferencesOf: aBlockClosure with: aSerialization

    1 to: aBlockClosure basicSize do: [ :index |
        aSerialization encodeReferenceTo: (aBlockClosure basicAt: index) ].

    aBlockClosure isClean
        ifTrue: [
            aSerialization stream nextPut: 1.
            aSerialization encodeReferenceTo:  aBlockClosure method.
            aSerialization encodeReferenceTo:  aBlockClosure receiver.
            ]
        ifFalse: [
            aSerialization stream nextPut: 0.
            aSerialization encodeReferenceTo:  aBlockClosure outerContext.
        ].

    aSerialization encodeReferenceTo:  aBlockClosure startpc.
    aSerialization encodeReferenceTo:  aBlockClosure numArgs.



-----

materializeReferencesOf: aBlockClosure with: aMaterialization

    | methodOrContext method context |
    1 to: aBlockClosure basicSize do: [ :index |
        aBlockClosure basicAt: index put: (aMaterialization
nextEncodedReference).
    ].

    (aMaterialization stream next = 1)
        ifTrue: [ " it was a clean BlockClosure, otherwise it should have
been 0"
            method := aMaterialization nextEncodedReference.
            aBlockClosure
                    outerContext:     (MethodContext
                                        sender: nil
                                        receiver: aMaterialization
nextEncodedReference
                                        method: method
                                        arguments: #() )
                    startpc: aMaterialization nextEncodedReference
                    numArgs: aMaterialization nextEncodedReference.

            ]
        ifFalse: [
                context := aMaterialization nextEncodedReference.
                aBlockClosure
                    outerContext: context
                    startpc: aMaterialization nextEncodedReference
                    numArgs: aMaterialization nextEncodedReference].


-----


Thanks in advance!




On Sun, Dec 4, 2011 at 8:33 PM, Mariano Martinez Peck <marianopeck at gmail.com
> wrote:

> Thanks Juan. With the latest version of Eliot, the following are failing:
>
>
>   self deny: [ | outerBlockTemp | [ outerBlockTemp printString ] isClean ]
> value.
>   self deny: [ | outerBlockTemp | [ outerBlockTemp :=7 ] isClean ] value.
>   self deny: [ tempVar + 1 ] isClean.
>   self deny: [ tempVar := 1 ] isClean.
>   self deny: [ ClassVar + 1 ] isClean.
>   self deny: [ ClassVar := 1 ] isClean.
>
> Thanks
>
>
> On Sat, Dec 3, 2011 at 3:01 AM, Juan Vuletich <juan at jvuletich.org> wrote:
>
>> Eliot Miranda wrote:
>>
>>
>>>
>>> On Fri, Dec 2, 20Tha11 at 4:01 PM, Juan Vuletich <juan at jvuletich.org<mailto:
>>> juan at jvuletich.org>> wrote:
>>>
>>>    Eliot Miranda wrote:
>>>
>>>        Hi Juan,
>>>
>>>        ...
>>>
>>>
>>>
>>>           I can say that it does indeed work. But I'd be really
>>>        grateful if
>>>           Eliot took a good look at that code!
>>>
>>>
>>>        There is a much better way, using
>>>        InstructionStream>>**interpreetNextInstructionFor: which
>>>        abstracts away from the bytecode encoding and allows one to
>>>        simply use messages.  e.g. look at
>>>
>>>        and (SortedCollection sortBlock: [:a :b| a compare: b
>>>        caseSensitive: false]) sortBlock abstractBytecodes evaluates to
>>>
>>>        an OrderedCollection(#**pushTemporaryVariable:
>>>        #pushTemporaryVariable: #pushConstant: #send:super:numArgs:)
>>>
>>>        So a check for clean blocks would check for a block's
>>>        abstractBytecodes not including some particular set of banned
>>>        messages (which I *think* is pushReceiver
>>>        pushReceiverVariable: popIntoReceiverVariable:
>>>        storeIntoReceiverVariable: methodReturnConstant:
>>>        methodReturnReceiver methodReturnTop).
>>>
>>>        e.g.
>>>
>>>        isClean
>>>             ^(self abstractBytecodes intersection: #(pushReceiver
>>>        pushReceiverVariable: popIntoReceiverVariable:
>>>        storeIntoReceiverVariable: methodReturnConstant:
>>>        methodReturnReceiver methodReturnTop)) isEmpty
>>>
>>>
>>>           Cheers,
>>>           Juan Vuletich
>>>
>>>
>>>
>>>
>>>        --         best,
>>>        Eliot
>>>
>>>
>>>    Thanks Eliot. However, it looks to me that this needs more tweaking.
>>>
>>> <blush>Damn right :)  I've done zero testing.  It was just an
>>> example</blush>
>>>
>>>    For instance, your method answers true for
>>>      [ xx size ] isClean
>>>    and
>>>      [ ^ nil ] isClean
>>>    while mine answers false for both. Maybe we could serialize non
>>>    local returns using the Compiler (although we won't be able to
>>>    return anywhere), but external variable acess is a problem.
>>>
>>>    Will keep playing with this for a while...
>>>
>>>
>>> Can you (can I??) write some tests?  What's xx in the above?  Ah! [^nil]
>>> isn't clean in my code since it should be pc <= end, not pc < end!!  Try
>>> the attached...
>>>
>>
>> This is a start:
>>
>> testIsClean
>>   "
>>   ClosureTests new testIsClean
>>   "
>>   | tempVar |
>>   tempVar := 1.
>>   self assert: [ 3 + 4 ] isClean.
>>   self assert: [ :a | a * 2 ] isClean.
>>   self assert: [ Smalltalk size ] isClean.
>>   self assert: [ :blockArg | blockArg printString ] isClean.
>>   self assert: [ | blockTemp | blockTemp printString ] isClean.
>>   self assert: [ | blockTemp | blockTemp :=7 ] isClean.
>>   self deny: [ | outerBlockTemp | [ outerBlockTemp printString ] isClean
>> ] value.
>>   self deny: [ | outerBlockTemp | [ outerBlockTemp :=7 ] isClean ] value.
>>   self deny: [ tempVar + 1 ] isClean.
>>   self deny: [ tempVar := 1 ] isClean.
>>   self deny: [ ivar + 1 ] isClean.
>>   self deny: [ ivar := 1 ] isClean.
>>   self deny: [ ^ true ] isClean.
>>   self deny: [ self printString ] isClean.
>>   self deny: [ ^ self ] isClean.
>>   self deny: [ ClassVar + 1 ] isClean.
>>   self deny: [ ClassVar := 1 ] isClean.
>>
>> This test passes with my implementation.
>>
>>
>>
>>>
>>>    Cheers,
>>>    Juan Vuletich
>>>
>>>
>>>
>>>
>>> --
>>> best,
>>> Eliot
>>>
>>>
>> Cheers,
>> Juan Vuletich
>>
>>
>
>
> --
> Mariano
> http://marianopeck.wordpress.com
>
>


-- 
Mariano
http://marianopeck.wordpress.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20111210/545ada0c/attachment.htm