2010/3/1 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
> 2010/3/1 Levente Uzonyi <leves at elte.hu>:
>> Hi,
>>
>>
>> I recently ran into a new decompiler issue which doesn't allow the debugger
>> to open. Details here: http://bugs.squeak.org/view.php?id=7467
>>
>> So I thought it's time to revisit my decision of making DecompilerTests a
>> subclass of LongTestCase, because I'm pretty sure noone is running these
>> tests. According to my measurements 60-70% of their runtime is spent with
>> garbage collecting. DecompilerTests >> #decompileClassesSelect: forces a
>> full GC for every class in the image (>2000 in a clear trunk image). Is this
>> GC still necessary?
>>
>> There are also two new methods besides EventSensor >> #eventTickler which
>> give a Syntax Error when running these tests. The new methods are
>> SHMCClassDefinition >> #withAllSuperclasses and SHMCClassDefinition >>
>> #allInstVarNames.
>> This problem occurs when a temporary is declared in an inlined block which
>> is the receiver of #whileTrue, #whileTrue:, #whileFalse or #whileFalse and
>> the variable is also referenced from a normal block.
>> For example:
>>        [
>>                | foo |
>>                [ foo := false ] value ] whileTrue
>> will be decompiled as: [[_r1 := false] value] whileTrue.
>>
>> A lot of decompiler tests are failing, because the temporaries are indexed
>> in a different order during the second decompilation.
>>
>> I hope someone with enough knowledge will fix these issues.
>>
>
> I have no special knowledge.
> The code is quite complex (measured with # of instVars, # of methods,
> and # line of codes in certain methods).
> But, thanks you provided a good test case.
>
> The first problem I see right from beginning is the order of trailer temp names:
> trailer tempNames -> '[input i index1 ps k count (digits)[each (digits)]]'
> I guess parentheses (digits) expresses a copiedValue.
>
> Let explore the byte codes to see if it fits:
>
> '1 0' readStream
> 81 <22> pushConstant: '1 0'
> 82 <D1> send: readStream
>
> [:input: | ...] block argument
> 83 <8F 01 00 67> closureNumCopied: 0 numArgs: 1 bytes 87 to 189
> 87      <73> pushConstant: nil => i
> 88      <73> pushConstant: nil => index1
> 89      <73> pushConstant: nil => ps
> 90      <73> pushConstant: nil => k
> 91      <73> pushConstant: nil => count
> 92      <73> pushConstant: nil => UNUSED SLOT ?????
> 93      <73> pushConstant: nil => copiedValues #( digits )
> 94      <10> pushTemp: 0               ( input )
> 95      <DE> send: skipSeparators
> 96      <87> pop
> 97      <50> pushLit: Integer
> 98      <10> pushTemp: 0              ( input )
> 99      <EF> send: readFrom:
> 100     <69> popIntoTemp: 1          ( i := )
> 101     <11> pushTemp: 1             ( i )
> 102     <75> pushConstant: 0
> 103     <B6> send: =
>
> whileFalse: [...
> 104     <A8 52> jumpTrue: 188
> 106     <8A 01> push: (Array new: 1)
> 108     <6F> popIntoTemp: 7         ( #( digits ) ) copiedValues... Note
> that this is at rank 8, not 7 as expected from trailer tempNames...
> 109     <76> pushConstant: 1
> 110     <6C> popIntoTemp: 4        ( k :=)
> 111     <75> pushConstant: 0
> 112     <6D> popIntoTemp: 5        ( count := )
> 113     <44> pushLit: Array
> 114     <25> pushConstant: 10
> 115     <75> pushConstant: 0
> 116     <F3> send: new:withAll:
> 117     <8E 00 07> popIntoTemp: 0 inVectorAt: 7  ( digits := )
>
> 120     <15> pushTemp: 5            ( count )
> 121     <11> pushTemp: 1            ( i )
> 122     <B2> send: <
>
> whileTrue: [
> 123     <AC 3A> jumpFalse: 183
> 125     <14> pushTemp: 4            ( k )
> 126     <D6> send: printString
> 127     <6B> popIntoTemp: 3        ( ps := )
> 128     <13> pushTemp: 3
> 129     <2C> pushConstant: $1
> 130     <EB> send: indexOf:
> 131     <81 42> storeIntoTemp: 2  ( index1 := )
> 133     <75> pushConstant: 0
> 134     <B6> send: =
> 135     <99> jumpFalse: 138
> 136     <71> pushConstant: true
> 137     <95> jumpTo: 144
> 138     <13> pushTemp: 3
> 139     <2A> pushConstant: $3
> 140     <12> pushTemp: 2            ( index1 )
> 141     <F9> send: indexOf:startingAt:
> 142     <75> pushConstant: 0
> 143     <B6> send: =
>
> ifTrue: [
> 144     <AC 1F> jumpFalse: 177
> 146     <15> pushTemp: 5            ( count )
> 147     <76> pushConstant: 1
> 148     <B0> send: +
> 149     <6D> popIntoTemp: 5        ( count := )
> 150     <13> pushTemp: 3            ( ps )
> 151     <17> pushTemp: 7            ( #( digits) )
> 152     <8F 11 00 13> closureNumCopied: 1 numArgs: 1 bytes 156 to 174
> 156             <8C 00 01> pushTemp: 0 inVectorAt: 1  ( digits )
> 159             <10> pushTemp: 0                                ( each )
> 160             <D7> send: asciiValue
> 161             <28> pushConstant: 47
> 162             <B1> send: -
> 163             <8C 00 01> pushTemp: 0 inVectorAt: 1  ( digits )
> 166             <10> pushTemp: 0                               ( each )
> 167             <D7> send: asciiValue
> 168             <28> pushConstant: 47
> 169             <B1> send: -
> 170             <C0> send: at:
> 171             <76> pushConstant: 1
> 172             <B0> send: +
> 173             <C1> send: at:put:
> 174             <7D> blockReturn
> 175     <CB> send: do:
> 176     <87> pop
> 177     <14> pushTemp: 4       ( k )
> 178     <76> pushConstant: 1
> 179     <B0> send: +
> 180     <6C> popIntoTemp: 4   ( k := )
> 181     <A3 C1> jumpTo: 120
> 183     <70> self
> 184     <DD> send: halt
> 185     <87> pop
> 186     <A3 A2> jumpTo: 94
> 188     <73> pushConstant: nil
> 189     <7D> blockReturn
> 190 <E0> send: in:
> 191 <7C> returnTop
>
> What look *** STRANGE *** is the UNUSED slot reserved in [:input | ...
> ] temp vector...
>
>
> Now decompiling in initSymbols: >>
> mapFromBlockStartsIn:toTempVarsFrom:constructor:
>
> I got this map: there are only 3 blocks: (the other onesare inlined)
> - the whole method (startpc=81)
> - the [:input | ...] block (startpc=87)
> - the [:each | ...] block (startpc=156)
>
> And the temps names are (for each block indexed by start pc)
> a Dictionary(
>    81->#()
>    87->#(
>           #('input' 1)
>           #('i' 2)
>           #('index1' 3)
>           #('ps' 4)
>           #('k' 5)
>           #('count' 6)
>           #('digits' #(7 1)))
>    156->#(
>           #('each' 1)
>           #('digits' #(2 1))) )
> I think we got a problem here... digits should be #(8 1) to fit the
> compiled code and the UNUSED slot...
>
> the tempVector has just 7 slots:
>    {{input} . {i} . {index1} . {ps} . {k} . {count} . {{digits}}}
> later replaced with:
>    {{input} . {i} . {index1} . {ps} . {k} . {count} . {_r7}}
>
> Then, when decompiling:
> 108     <6F> popIntoTemp: 7         ( #( digits ) ) copiedValues... Note
> that this is at rank 8, not 7 as expected from trailer tempNames...
> the assertion fails because we only have 7 temps vars and try to
> assign temp of rank 8...
>
> Now, just have to discover how the UNUSED slot was ever compiled...
>

Debugging this:
Compiler evaluate: '''1 0'' readStream in: [ :input |
    | i |
    [
    input skipSeparators.
    i := Integer readFrom: input.
    i = 0 ] whileFalse: [
        | k count digits |
        k := 1.
        count := 0.
        digits := Array new: 10 withAll: 0.
        [ count < i ] whileTrue: [
            | index1 ps |
            ps := k printString.
            ((index1 := ps indexOf: $1) = 0 or: [ (ps indexOf: $3
startingAt: index1) = 0 ]) ifTrue: [
                count := count + 1.
                ps do: [ :each | digits at: each asciiValue - 47 put:
(digits at: each asciiValue - 47) + 1 ] ].
                k := k + 1 ].
        self halt ] ] '

Visibly, the digits temp has two slots reserved, one for itself, one
for copiedValues <2-8>:
encoder blockExtentsToLocal
-> a Dictionary(
     (0 to: 10)->#()
     (2 to: 8)->{{input} . {i} . {index1} . {ps} . {k} . {count} .
{digits} . {<2-8>}}
     (4 to: 6)->{{each} . {<2-8>}} )

Nicolas

> Eliot might be more efficient than me...
>
> Nicolas
>
>>
>> Cheers,
>> Levente
>>
>>
>