[Vm-dev] Re: [Pharo-project] problem with tempNamed: in Pharo 2.0
eliot.miranda at gmail.com
Tue May 1 19:29:41 UTC 2012
On Tue, May 1, 2012 at 9:13 AM, Mariano Martinez Peck <marianopeck at gmail.com
> On Tue, May 1, 2012 at 6:09 PM, Mariano Martinez Peck <
> marianopeck at gmail.com> wrote:
>> Hi guys. I noticed stef did this issue:
>> However, now I have the following test that fails in Pharo 2.0 but works
>> fine in 1.4:
>> | string context |
>> string := 'test'.
>> context := [self class. string asUppercase] asContext.
>> self assert: (context tempNamed: 'string') = 'test'
>> the current implementation of #tempNamed: is:
>> tempNamed: aName
>> "Returns the value of the temporaries, aName."
>> "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:
>> 13 <8F 00 00 05> closureNumCopied: 0 numArgs: 0 bytes 17 to 21
>> 17 <73> pushConstant: nil
>> 18 <77> pushConstant: 2
>> 19 <81 40> storeIntoTemp: 0
>> 21 <7D> blockReturn
>> 22 <7C> returnTop
>> 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."
>> | index |
>> index := (self tempNames indexOf: aName).
>> ^ index >= stackp
> Maybe the solution is to use #> rather than #>= ?
right. But tempNames is fundamentally broken for closures. 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 => means printIt returns...)
(Collection>>#inject:into:) methodNode schematicTempNamesString =>
'thisValue binaryBlock (nextValue)[each binaryBlock (nextValue)]'
This says that
a) at method level there are three temps, thisValue, binaryBlock and an
indirection vector, and in the indirection vector is one temp named
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.
This is all orchestrated by DebuggerMethodMap, so that
aContext method debuggerMap tempNamesForContext: aContext
answers a list of the flattened temp names in a context (flattening out
indirection vectors) and for the above would answer either #('thisValue'
'binaryBlock' 'nextValue') or #('each' 'binaryBlock' 'nextValue'), and
| map |
map := aContext method debuggerMap.
map namedTempAt: ((map tempNamesForContext: aContext) indexOf:
aTempName) in: aContext
gets the temp from the unflattened temps in a context. This is how the
debugger accesses temp names.
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*.
Now, one good way to understand this is through examples. inject:into: is
the canonical simple example I'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:
SystemNavigation new browseAllSelect:
[:m| | is |
is := InstructionStream on: m.
is scanFor: [:b| b = 138 and: [is followingByte between: 2 and: 127]]]
and the simplest example in a trunk 4.3 image I find is:
"Faster than the superclass implementation when you hold many instances of
the same value (which you probably do, otherwise you wouldn't be using a
| sum first |
first := true.
contents keysAndValuesDo: [ :value :count |
ifTrue: [ sum := value * count. first := false ]
ifFalse: [ sum := sum + (value * count) ] ].
first ifTrue: [ self errorEmptyCollection ].
which needs a two-element indirection vector because the block [ :value
:count |...] assigns to both sum and first. Hence
(Bag>>#sum) methodNode schematicTempNamesString => '(sum
first)[value count (sum first)]'
So in an activation of Bag>>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>>sum the value of first is (sumBlockContext
tempAt: 3) at: 2.
I can keep repeating myself until I'm blue in the face, but y'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.
>> ifTrue: [ nil]
>> ifFalse: [self tempAt: (self tempNames indexOf: aName)]
>> and previously it was:
>> tempNamed: aName
>> ^self tempAt: (self tempNames indexOf: aName)
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Vm-dev