<br><br><div class="gmail_quote">On Fri, Dec 9, 2011 at 3:01 PM, Mariano Martinez Peck <span dir="ltr"><<a href="mailto:marianopeck@gmail.com">marianopeck@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
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:<br>
<br> | aSortedCollection materialized |<br> instanceVariableForTesting := false.<br> <br> aSortedCollection := SortedCollection sortBlock: [:a :b | <br> instanceVariableForTesting <br> ifTrue: [ a <= b ] <br>
ifFalse: [ a >= b ] ].<br><br> materialized := self resultOfSerializeAndMaterialize: aSortedCollection.<br><br> materialized addAll: #(2 3 1).<br> aSortedCollection addAll: #(2 3 1).<br><br><br>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. <br>
<br>Do you think there is something I should check for this closure? I think it may have to be related with the instanceVariable...<br></blockquote><div><br></div><div>Clearly the closure's copied value (instanceVariableForTesting), its 1st indexable variable, should be a boolean. Just explore</div>
<div> | instanceVariableForTesting |</div> instanceVariableForTesting := false.<br> [:a :b | instanceVariableForTesting ifTrue: [ a <= b ] ifFalse: [ a >= b ] ]</div><div class="gmail_quote">and you'll see the closure's 1st indexable variable is false. So why is it being (de)serialized as a Processor? There's your bug.</div>
<div class="gmail_quote"><br><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><br>Just for the record, what we are doing is something like:<br><br>serializeReferencesOf: aBlockClosure with: aSerialization <br>
<br> 1 to: aBlockClosure basicSize do: [ :index |<br> aSerialization encodeReferenceTo: (aBlockClosure basicAt: index) ].<br> <br> aBlockClosure isClean<br> ifTrue: [<br> aSerialization stream nextPut: 1.<br>
aSerialization encodeReferenceTo: aBlockClosure method.<br> aSerialization encodeReferenceTo: aBlockClosure receiver.<br> ]<br> ifFalse: [ <br> aSerialization stream nextPut: 0.<br>
aSerialization encodeReferenceTo: aBlockClosure outerContext. <br> ].<br> <br> aSerialization encodeReferenceTo: aBlockClosure startpc.<br> aSerialization encodeReferenceTo: aBlockClosure numArgs.<br>
<br><br><br>-----<br><br>materializeReferencesOf: aBlockClosure with: aMaterialization<br><br> | methodOrContext method context | <br> 1 to: aBlockClosure basicSize do: [ :index |<br> aBlockClosure basicAt: index put: (aMaterialization nextEncodedReference).<br>
].<br><br> (aMaterialization stream next = 1)<br> ifTrue: [ " it was a clean BlockClosure, otherwise it should have been 0"<br> method := aMaterialization nextEncodedReference.<br> aBlockClosure <br>
outerContext: (MethodContext<br> sender: nil <br> receiver: aMaterialization nextEncodedReference <br> method: method <br>
arguments: #() )<br> startpc: aMaterialization nextEncodedReference <br> numArgs: aMaterialization nextEncodedReference. <br> ]<br>
ifFalse: [ <br> context := aMaterialization nextEncodedReference.<br> aBlockClosure <br> outerContext: context<br> startpc: aMaterialization nextEncodedReference <br>
numArgs: aMaterialization nextEncodedReference].<br> <br><br>-----<br><br><br>Thanks in advance!<div><div></div><div class="h5"><br><br> <br><br><br><div class="gmail_quote">On Sun, Dec 4, 2011 at 8:33 PM, Mariano Martinez Peck <span dir="ltr"><<a href="mailto:marianopeck@gmail.com" target="_blank">marianopeck@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks Juan. With the latest version of Eliot, the following are failing:<div><br><br> self deny: [ | outerBlockTemp | [ outerBlockTemp printString ] isClean ] value.<br>
self deny: [ | outerBlockTemp | [ outerBlockTemp :=7 ] isClean ] value.<br>
self deny: [ tempVar + 1 ] isClean.<br> self deny: [ tempVar := 1 ] isClean.<br></div><div> self deny: [ ClassVar + 1 ] isClean.<br> self deny: [ ClassVar := 1 ] isClean.<br><br></div>Thanks<br><br><br><div class="gmail_quote">
<div>On Sat, Dec 3, 2011 at 3:01 AM, Juan Vuletich <span dir="ltr"><<a href="mailto:juan@jvuletich.org" target="_blank">juan@jvuletich.org</a>></span> wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Eliot Miranda wrote:<div><div><div><div><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
On Fri, Dec 2, 20Tha11 at 4:01 PM, Juan Vuletich <<a href="mailto:juan@jvuletich.org" target="_blank">juan@jvuletich.org</a> <mailto:<a href="mailto:juan@jvuletich.org" target="_blank">juan@jvuletich.org</a>>> wrote:<br>
<br>
Eliot Miranda wrote:<br>
<br>
Hi Juan,<br>
<br>
...<br>
<br>
<br>
<br>
I can say that it does indeed work. But I'd be really<br>
grateful if<br>
Eliot took a good look at that code!<br>
<br>
<br>
There is a much better way, using<br>
InstructionStream>><u></u>interpreetNextInstructionFor: which<br>
abstracts away from the bytecode encoding and allows one to<br>
simply use messages. e.g. look at<br>
<br>
and (SortedCollection sortBlock: [:a :b| a compare: b<br>
caseSensitive: false]) sortBlock abstractBytecodes evaluates to<br>
<br>
an OrderedCollection(#<u></u>pushTemporaryVariable:<br>
#pushTemporaryVariable: #pushConstant: #send:super:numArgs:)<br>
<br>
So a check for clean blocks would check for a block's<br>
abstractBytecodes not including some particular set of banned<br>
messages (which I *think* is pushReceiver<br>
pushReceiverVariable: popIntoReceiverVariable:<br>
storeIntoReceiverVariable: methodReturnConstant:<br>
methodReturnReceiver methodReturnTop).<br>
<br>
e.g.<br>
<br>
isClean<br>
^(self abstractBytecodes intersection: #(pushReceiver<br>
pushReceiverVariable: popIntoReceiverVariable:<br>
storeIntoReceiverVariable: methodReturnConstant:<br>
methodReturnReceiver methodReturnTop)) isEmpty<br>
<br>
<br>
Cheers,<br>
Juan Vuletich<br>
<br>
<br>
<br>
<br>
-- best,<br>
Eliot<br>
<br>
<br>
Thanks Eliot. However, it looks to me that this needs more tweaking. <br>
<br>
<blush>Damn right :) I've done zero testing. It was just an example</blush><br>
<br>
For instance, your method answers true for<br>
[ xx size ] isClean<br>
and<br>
[ ^ nil ] isClean<br>
while mine answers false for both. Maybe we could serialize non<br>
local returns using the Compiler (although we won't be able to<br>
return anywhere), but external variable acess is a problem.<br>
<br>
Will keep playing with this for a while...<br>
<br>
<br>
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...<br>
</blockquote>
<br></div></div>
This is a start:<br>
<br>
testIsClean<br>
"<br>
ClosureTests new testIsClean<br>
"<br>
| tempVar |<br>
tempVar := 1.<br>
self assert: [ 3 + 4 ] isClean.<br>
self assert: [ :a | a * 2 ] isClean.<br>
self assert: [ Smalltalk size ] isClean.<br>
self assert: [ :blockArg | blockArg printString ] isClean.<br>
self assert: [ | blockTemp | blockTemp printString ] isClean.<br>
self assert: [ | blockTemp | blockTemp :=7 ] isClean.<br>
self deny: [ | outerBlockTemp | [ outerBlockTemp printString ] isClean ] value.<br>
self deny: [ | outerBlockTemp | [ outerBlockTemp :=7 ] isClean ] value.<br>
self deny: [ tempVar + 1 ] isClean.<br>
self deny: [ tempVar := 1 ] isClean.<br>
self deny: [ ivar + 1 ] isClean.<br>
self deny: [ ivar := 1 ] isClean.<br>
self deny: [ ^ true ] isClean.<br>
self deny: [ self printString ] isClean.<br>
self deny: [ ^ self ] isClean.<br>
self deny: [ ClassVar + 1 ] isClean.<br>
self deny: [ ClassVar := 1 ] isClean.<br>
<br>
This test passes with my implementation.<div><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Cheers,<br>
Juan Vuletich<br>
<br>
<br>
<br>
<br>
-- <br>
best,<br>
Eliot<br>
<br>
</blockquote>
<br></div>
Cheers,<span><font color="#888888"><br>
Juan Vuletich<br>
<br>
</font></span></div></div></blockquote></div><span><font color="#888888"><br><br clear="all"><br>-- <br>Mariano<br><a href="http://marianopeck.wordpress.com" target="_blank">http://marianopeck.wordpress.com</a><br>
<br>
</font></span></blockquote></div><br><br clear="all"><br></div></div><font color="#888888">-- <br>Mariano<br><a href="http://marianopeck.wordpress.com" target="_blank">http://marianopeck.wordpress.com</a><br><br>
</font><br><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>best,<div>Eliot</div><br>