On Tue, 2 Mar 2010, Nicolas Cellier wrote:

> I don't mind optimization there, asOrderedCollection was already in
> Eliot's code...

Well, you kept Eliot's optimization (#isArray check) instead of 
just sending #asOrderedCollection.

> What worries me is that this is not a complete fix...
>
> For example, try
>
> (EventSensor>>#eventTickler) decompileString.
>
> or try your own example:
>
> Compiler evaluate: ' [
>  | foo |
>  self halt.
>  [ foo := false ] value ] whileTrue'
>

I see. I thought there are three separate issues with the decompiler but 
they seem to be closely related to each other.


Levente

> It has to do with the definingScope not being the current
> readingScopes nor the actualScope...
> I fear more bugs in case of shadowed temps (not possible interactively
> though, Compiler is pedantic).
>
> Nicolas
>
> 2010/3/2 Levente Uzonyi <leves at elte.hu>:
>> Thanks for the fix Nicolas.
>> Too bad #asOrderedCollection creates a copy even if it's sent to an
>> OrderedCollection. I wonder if there's a way to fix it without breaking
>> existing code.
>>
>>
>> Levente
>>
>> On Mon, 1 Mar 2010, commits at source.squeak.org wrote:
>>
>>> Nicolas Cellier uploaded a new version of Compiler to project The Trunk:
>>> http://source.squeak.org/trunk/Compiler-nice.129.mcz
>>>
>>> ==================== Summary ====================
>>>
>>> Name: Compiler-nice.129
>>> Author: nice
>>> Time: 1 March 2010, 11:24:30.271 pm
>>> UUID: a27b827d-1354-1147-bc1a-edeb6e1f0e97
>>> Ancestors: Compiler-nice.128
>>>
>>> Fix Compiler/Decompiler temporary slot mismatch
>>> http://bugs.squeak.org/view.php?id=7467
>>>
>>> See also my comments in response to
>>>
>>> http://lists.squeakfoundation.org/pipermail/squeak-dev/2010-March/145280.html
>>>
>>> Eliot, please check....
>>>
>>> =============== Diff against Compiler-nice.128 ===============
>>>
>>> Item was changed:
>>>  InstructionStream subclass: #Decompiler
>>>        instanceVariableNames: 'constructor method instVars tempVars
>>> constTable stack statements lastPc exit caseExits lastJumpPc lastReturnPc
>>> limit hasValue blockStackBase numLocalTemps blockStartsToTempVars
>>> tempVarCount'
>>>        classVariableNames: 'ArgumentFlag CascadeFlag CaseFlag IfNilFlag'
>>>        poolDictionaries: ''
>>>        category: 'Compiler-Kernel'!
>>>
>>> + !Decompiler commentStamp: 'nice 3/1/2010 19:56' prior: 0!
>>> - !Decompiler commentStamp: '<historical>' prior: 0!
>>>  I decompile a method in three phases:
>>>        Reverser: postfix byte codes -> prefix symbolic codes (nodes and
>>> atoms)
>>>        Parser: prefix symbolic codes -> node tree (same as the compiler)
>>>        Printer: node tree -> text (done by the nodes)
>>>
>>>
>>>  instance vars:
>>>
>>> +       constructor <DecompilerConstructor> an auxiliary knowing how to
>>> generate Abstract Syntax Tree (node tree)
>>> +       method <CompiledMethod> the method being decompiled
>>> +       instVars <Array of: String> the instance variables of the class
>>> implementing method
>>> +       tempVars <String | (OrderedCollection of: String)> hold the names
>>> of temporary variables (if known)
>>> +               NOTE: POLYMORPHISM WILL BE RESOLVED IN #initSymbols:
>>> +       constTable <Collection of: ParseNode> parse node associated with
>>> byte encoded constants (nil true false 0 1 -1 etc...)
>>> +       stack <OrderedCollection of: (ParseNode | String | Integer) >
>>> multipurpose...
>>> +       statements <OrderedCollection of: ParseNode> the statements of the
>>> method being decompiled
>>> +       lastPc <Integer>
>>> +       exit <Integer>
>>> +       caseExits <OrderedCollection of: Integer> - stack of exit
>>> addresses that have been seen in the branches of caseOf:'s
>>> +       lastJumpPc <Integer>
>>> +       lastReturnPc <Integer>
>>> +       limit <Integer>
>>> +       hasValue <Boolean>
>>> +       blockStackBase <Integer>
>>> +       numLocaltemps <Integer | Symbol> - number of temps local to a
>>> block; also a flag indicating decompiling a block
>>> +       blockStartsToTempVars <Dictionary key: Integer value:
>>> (OrderedCollection of: String)>
>>> +       tempVarCount <Integer> number of temp vars used by the method!
>>> -       constructor
>>> -       method
>>> -       instVars
>>> -       tempVars
>>> -       constTable
>>> -       stack
>>> -       statements
>>> -       lastPc
>>> -       exit
>>> -       caseExits       - stack of exit addresses that have been seen in
>>> the branches of caseOf:'s
>>> -       lastJumpPc
>>> -       lastReturnPc
>>> -       limit
>>> -       hasValue
>>> -       blockStackBase
>>> -       numLocaltemps - number of temps local to a block; also a flag
>>> indicating decompiling a block!
>>>
>>> Item was added:
>>> + ----- Method: BlockNode>>makeTemporariesRemovable (in category 'code
>>> generation (closures)') -----
>>> + makeTemporariesRemovable
>>> +       "Utilities for when we want to remove some temporaries."
>>> +
>>> +       temporaries isArray ifTrue:
>>> +               [temporaries := temporaries asOrderedCollection].!
>>>
>>> Item was changed:
>>>  ----- Method: BlockNode>>addRemoteTemp:rootNode: (in category 'code
>>> generation (closures)') -----
>>>  addRemoteTemp: aTempVariableNode rootNode: rootNode "<MethodNode>"
>>>        "Add aTempVariableNode to my actualScope's sequence of
>>>         remote temps.  If I am an optimized block then the actual
>>>         scope is my actualScopeIfOptimized, otherwise it is myself."
>>> -       temporaries isArray ifTrue:
>>> -               [temporaries := temporaries asOrderedCollection].
>>>        remoteTempNode == nil ifTrue:
>>>                [remoteTempNode := RemoteTempVectorNode new
>>>                                                                name: self
>>> remoteTempNodeName
>>>                                                                index:
>>> arguments size + temporaries size
>>>                                                                type:
>>> LdTempType
>>>                                                                scope: 0.
>>>                 actualScopeIfOptimized
>>>                        ifNil:
>>> +                               [self addTempNode: remoteTempNode.
>>> -                               [temporaries addLast: remoteTempNode.
>>>                                 remoteTempNode definingScope: self]
>>>                        ifNotNil: [actualScopeIfOptimized addHoistedTemps:
>>> { remoteTempNode }]].
>>>        remoteTempNode addRemoteTemp: aTempVariableNode encoder: rootNode
>>> encoder.
>>>        "use remove:ifAbsent: because the deferred analysis for optimized
>>>         loops can result in the temp has already been hoised into the
>>> root."
>>> +       self actualScope removeTempNode: aTempVariableNode ifAbsent: [].
>>> -       temporaries remove: aTempVariableNode ifAbsent: [].
>>>        ^remoteTempNode!
>>>
>>> Item was added:
>>> + ----- Method: BlockNode>>addTempNode: (in category 'code generation
>>> (closures)') -----
>>> + addTempNode: aTempVariableNode
>>> +       "Utilities for when we want to add some temporaries."
>>> +
>>> +       self makeTemporariesRemovable.
>>> +       ^temporaries add: aTempVariableNode!
>>>
>>> Item was added:
>>> + ----- Method: BlockNode>>removeTempNode:ifAbsent: (in category 'code
>>> generation (closures)') -----
>>> + removeTempNode: aTempVariableNode ifAbsent: aBlock
>>> +       "Utilities for when we want to remove some temporaries."
>>> +
>>> +       self makeTemporariesRemovable.
>>> +       ^temporaries remove: aTempVariableNode ifAbsent: aBlock
>>> +       !
>>>
>>>
>>>
>>
>>
>
>