On Tue, May 18, 2010 at 4:10 PM, Igor Stasenko <siguctua@gmail.com> wrote:

On 19 May 2010 00:17, Eliot Miranda <eliot.miranda@gmail.com> wrote:
> Hi Hans-Martin,
>
> On Tue, May 18, 2010 at 12:15 PM, Hans-Martin Mosner <hmm@heeg.de> wrote:
>>
>> Hello,
>> while looking at the decompiled code of some OMeta methods I noticed
>> that the temp name handling in the decompiler seems to be broken.
>> When you compile a method with temps that are referenced from blocks,
>> the decompiler tends to rearrange the temp names, so this method
>>
>> test
>> | one two |
>> two := 2.
>> ^{[one := 1].
>> [ [one + two] value]}
>>
>> gets decompiled to this:
>>
>> test
>> | one two |
>> one := 2.
>> ^ {[two := 1]. [[two + one] value]}
>
> OK, the issue in Squeak 4.1 is that the
> CodeHolder>>decompiledSourceIntoContents method is out of date. Find
> attached.
>
>> But that's only a nuisance. The bug really shows up when you decompile a
>> method compiled using OMeta.
>> To see what I mean, load the OMeta2 package and have a look at method
>> OMeta2AndOrOpt>>and.
>
> Any way of reproducing this without OMeta, e.g. in a workspace?
> e.g. to test the above I used things like
> | mn m |
> mn := Compiler new compile: 'test
>    | one two |
>    two := 2.
>    ^{[one := 1].
>    [ [one + two] value]}' in: nil class classified: nil notifying: nil
> ifFail: nil.
> m := (mn generate: #(0 0 0 0)) copyWithTempsFromMethodNode: mn.
> {m decompile. m tempNamesString}
> where in 4.1 "generate: #(0 0 0 0)" needs to be replaced with "generate:
> CompiledMethodTrailer defaultMethodTrailer" (although asMethodTrailer on
> ArrayedCollection would be nice for backward-compatibility).

There is a Behavior>>#defaultMethodTrailer , which is backwards compatible.

CompiledMethod class>>newBytes:trailerBytes:nArgs:nTemps:nStack:nLits:primitive: (and hence BytecodeAgnosticMethodNode>>generate:) is no longer backward-compatible. Call it with the old meaningless #(0 0 0 0) and you get an error. One might be able to provide backward-compatibility if there was an implementation of asMethodTrailer in Arrayed Collection that deferred to CompiledMethodTrailer and the relevant part of newBytes:trailerBytes:nArgs:nTemps:nStack:nLits:primitive: read:

^ trailer asMethodTrailer createMethod: numberOfBytes

header: (nArgs bitShift: 24) +

(nTemps bitShift: 18) +

(largeBit bitShift: 17) +

(nLits bitShift: 9) +

primBits.

In any case the selector keyword is "trailerBytes:" not "methodTrailer:" or "trailer". IMO you should add a new method that takes a trailer and revert the old one to read

newBytes: numberOfBytes trailerBytes: trailer nArgs: nArgs nTemps: nTemps nStack: stackSize nLits: nLits primitive: primitiveIndex

"Answer an instance of me. The header is specified by the message

arguments. The remaining parts are not as yet determined."

^self newBytes: numberOfBytes

trailer: trailer asMethodTrailer

nArgs: nArgs

nTemps: nTemps

nStack: stackSize

nLits: nLits

primitive: primitiveIndex

So, one should use it instead of putting meaningless #(0 0 0 0) everywhere.

Its not an issue of #(0 0 0 0) being meaningless. Its an issue of not breaking the old way where #(0 0 0 0) was used to mean empty.

best

Eliot

>>
>> Cheers,
>> Hans-Martin
>>
>
>
>
>
>

--
Best regards,
Igor Stasenko AKA sig.