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@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@jvuletich.org> wrote:

...

Eliot Miranda wrote:

...

On Fri, Dec 2, 20Tha11 at 4:01 PM, Juan Vuletich <juan@jvuletich.org<mailto: juan@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