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

> On 19 May 2010 00:17, Eliot Miranda <eliot.miranda at gmail.com> wrote:
> > Hi Hans-Martin,
> >
> > On Tue, May 18, 2010 at 12:15 PM, Hans-Martin Mosner <hmm at 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.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20100518/c150f620/attachment.htm