<div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, May 1, 2012 at 1:12 PM, Mariano Martinez Peck <span dir="ltr">&lt;<a href="mailto:marianopeck@gmail.com" target="_blank">marianopeck@gmail.com</a>&gt;</span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br><br><br><div class="gmail_quote">On Tue, May 1, 2012 at 9:29 PM, Eliot Miranda <span dir="ltr">&lt;<a href="mailto:eliot.miranda@gmail.com" target="_blank">eliot.miranda@gmail.com</a>&gt;</span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_extra"><br><br><div class="gmail_quote"><div>On Tue, May 1, 2012 at 9:13 AM, Mariano Martinez Peck <span dir="ltr">&lt;<a href="mailto:marianopeck@gmail.com" target="_blank">marianopeck@gmail.com</a>&gt;</span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br><br><div class="gmail_quote"><div>On Tue, May 1, 2012 at 6:09 PM, Mariano Martinez Peck <span dir="ltr">&lt;<a href="mailto:marianopeck@gmail.com" target="_blank">marianopeck@gmail.com</a>&gt;</span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi guys. I noticed stef did this issue: <a href="http://code.google.com/p/pharo/issues/detail?id=5642" target="_blank">http://code.google.com/p/pharo/issues/detail?id=5642</a><br>However, now I have the following test that fails in Pharo 2.0 but works fine in 1.4:<br>




<br>| string context |<br>    string := &#39;test&#39;.<br>    context := [self class. string asUppercase] asContext.<br>    self assert: (context tempNamed: &#39;string&#39;) = &#39;test&#39;<br><br>the current implementation of #tempNamed: is:<br>




<br>tempNamed: aName<br>    &quot;Returns the value of the temporaries, aName.&quot;<br>    &quot;Implementation notes: temporary initialization in blocks simply uses pushNil to allocate and initialize each temp.  So if one inspects [|a|a:=2] and sends it self method symbolic you get:<br>




<br>    13 &lt;8F 00 00 05&gt; closureNumCopied: 0 numArgs: 0 bytes 17 to 21<br>    17     &lt;73&gt; pushConstant: nil<br>    18     &lt;77&gt; pushConstant: 2<br>    19     &lt;81 40&gt; storeIntoTemp: 0<br>    21     &lt;7D&gt; blockReturn<br>




    22 &lt;7C&gt; returnTop<br><br>    And when we check self asContext pc we get 17, which is *before* the nil is pushed. Therefore we should pay attention when querying a temporary if the temporary allocation was executed.&quot;<br>




<br>    | index |<br>    index := (self tempNames indexOf: aName).<br>    ^ index &gt;= stackp <br></blockquote></div><div><br><br>Maybe the solution is to use #&gt; rather than #&gt;=  ?<br></div></div></blockquote><div>


<br></div></div><div>right.  </div></div></div></blockquote><div><br>Ok, so I will integrate that as a first version.<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div class="gmail_extra"><div class="gmail_quote"><div>But tempNames is fundamentally broken for closures. </div></div></div></blockquote><div><br>Just to avoid confusing, #tempNames itself looks correct:<br><br>tempNames<br>

    &quot;Answer a SequenceableCollection of the names of the receiver&#39;s temporary <br>     variables, which are strings.&quot;<br><br>    ^ self debuggerMap tempNamesForContext: self<br></div></div></blockquote><div>
<br></div><div>Yes.  That&#39;s for ContextPart.  But it doesn&#39;t work for CompiledMethod, since temps may differ between a method and its blocks.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_extra"><div class="gmail_quote"><div>It does not answer temps in indirection vectors.  That is the whole point of schematicTempNamesString; it gives the topology of temps in a method.  e.g. (where =&gt; means printIt returns...)</div>


<div><br></div><div>(Collection&gt;&gt;#inject:into:) methodNode schematicTempNamesString    =&gt;    &#39;thisValue binaryBlock (nextValue)[each binaryBlock (nextValue)]&#39;</div><div><br></div><div>This says that</div>


<div>a) at method level there are three temps, thisValue, binaryBlock and an indirection vector, and in the indirection vector is one temp named nextValue.</div><div>b) in the block in inject:into: there are three temps, each (the argument), binaryBlock and an indirection vector, and in that indirection vector is a temp named nextValue.</div>


<div><br></div><div>This is all orchestrated by DebuggerMethodMap, so that</div><div><br></div><div>       aContext method debuggerMap tempNamesForContext: aContext</div></div></div></blockquote><div><br>So #tempNames implementation is correct?<br>
</div></div></blockquote><div><br></div><div>Yes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div>
 </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div> </div><div>answers a list of the flattened temp names in a context (flattening out indirection vectors) and for the above would answer either #(&#39;thisValue&#39; &#39;binaryBlock&#39; &#39;nextValue&#39;) or #(&#39;each&#39; &#39;binaryBlock&#39; &#39;nextValue&#39;), and</div>


<div><br></div><div>        | map |</div><div>        map := aContext method debuggerMap.</div><div>        map namedTempAt: ((map tempNamesForContext: aContext) indexOf: aTempName) in: aContext</div><div><br></div><div>

gets the temp from the unflattened temps in a context.  This is how the debugger accesses temp names.</div>
<div><br></div><div>So you need to throw away the broken tempName: implementation and use DebuggerMethodMap or some parse over schematicTempNamesString, because *with closures temps are not simply at contiguous offsets on the stack*.  Make sense?</div>

</div></div></blockquote><div><br>Actually, we do have:<br><br>namedTempAt: index<br>    &quot;Answer the value of the temp at index in the receiver&#39;s sequence of tempNames.&quot;<br>    ^self debuggerMap namedTempAt: index in: self<br>

<br>and<br><br>namedTempAt: index put: aValue<br>    &quot;Set the value of the temp at index in the receiver&#39;s sequence of tempNames.<br>     (Note that if the value is a copied value it is also set out along the lexical chain,<br>

      but alas not in along the lexical chain.).&quot;<br>    ^self debuggerMap namedTempAt: index put: aValue in: self<br><br>so, if I understand correctly all we need to do is to fix tempNamed: and tempNamedPut: so that the delegate to namedTempAt: and namedTempPut: rather than to tempAt: and tempAtPut:   ? <br>
</div></div></blockquote><div><br></div><div>Yes.</div><div> </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_extra"><div class="gmail_quote"><div>Now, one good way to understand this is through examples.  inject:into: is the canonical simple example I&#39;ve used for years.  But we can find more complex examples that may help.  So this query looks for all methods that contain a bytecode to create an indirection vector with 2 or more elements:</div>


<div><br></div><div><div>SystemNavigation new browseAllSelect:</div><div><span style="white-space:pre-wrap">        </span>[:m| | is |</div><div><span style="white-space:pre-wrap">        </span>is := InstructionStream on: m.</div>
<div><span style="white-space:pre-wrap">        </span>is scanFor: [:b| b = 138 and: [is followingByte between: 2 and: 127]]]</div></div><div><br></div><div>and the simplest example in a trunk 4.3 image I find is:</div>
<div><br></div><div>Bag&gt;&gt;sum</div><div><span style="white-space:pre-wrap">        </span>&quot;Faster than the superclass implementation when you hold many instances of the same value (which you probably do, otherwise you wouldn&#39;t be using a Bag).&quot;</div>


<div><span style="white-space:pre-wrap">        </span></div><div><span style="white-space:pre-wrap">        </span>| sum first |</div><div><span style="white-space:pre-wrap">        </span>first := true.</div>
<div><span style="white-space:pre-wrap">        </span>contents keysAndValuesDo: [ :value :count |</div><div><span style="white-space:pre-wrap">                </span>first </div><div><span style="white-space:pre-wrap">                        </span>ifTrue: [ sum := value * count. first := false ]</div>


<div><span style="white-space:pre-wrap">                        </span>ifFalse: [ sum := sum + (value * count) ] ].</div><div><span style="white-space:pre-wrap">        </span>first ifTrue: [ self errorEmptyCollection ].</div>
<div><span style="white-space:pre-wrap">        </span>^sum</div><div><br></div><div>which needs a two-element indirection vector because the block [ :value :count |...] assigns to both sum and first.  Hence</div>
<div><br></div><div>(Bag&gt;&gt;#sum) methodNode schematicTempNamesString    =&gt;     &#39;(sum first)[value count (sum first)]&#39;</div><div><br></div><div>So in an activation of Bag&gt;&gt;sum the value of first is (sumContext tempAt: 1) at: 2 (cuz tempAt: 1 is the indirection vector represented as (sum first) in schematic temps, and first is the second element.  But in an activation of the block in Bag&gt;&gt;sum the value of first is (sumBlockContext tempAt: 3) at: 2.</div>


<div><br></div><div><br></div><div>I can keep repeating myself until I&#39;m blue in the face, but y&#39;all have to put in the effort to understand this simple scheme.  Temps that are modified after they are closed-over end up in indirection vectors.</div>

<div>
<div><br></div></div></div></div></blockquote><div><br><br>Thanks, I am trying to undersand and fix it :)<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div class="gmail_extra"><div class="gmail_quote"><div><div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div><br> </div><div>
<blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

        ifTrue: [ nil]<br>        ifFalse: [self tempAt: (self tempNames indexOf: aName)]<br><br><br>and previously it was:<br>
<br>tempNamed: aName<br>    ^self tempAt: (self tempNames indexOf: aName)<br><br><br>ideas?<span><font color="#888888"><br>-- <br>Mariano<br><a href="http://marianopeck.wordpress.com" target="_blank">http://marianopeck.wordpress.com</a><br>
</font></span></blockquote></div></div><span><font color="#888888">-- <br>Mariano<br><a href="http://marianopeck.wordpress.com" target="_blank">http://marianopeck.wordpress.com</a><br></font></span></blockquote></div></div>
<span><font color="#888888">-- <br>best,<div>Eliot</div></font></span></div></blockquote></div>-- <br>Mariano<br><a href="http://marianopeck.wordpress.com" target="_blank">http://marianopeck.wordpress.com</a></blockquote>
</div>-- <br>best,<div>Eliot</div><br>
</div>