My vision about trailers, that new trailers can be formed at different stages of compilation where you could have a separate layer, which controlling explicitly, what kind of trailers to use.
While by using a combination of #(0 0 0 0) and then #asMethodTrailer you make it hard for such potential layer to control explicitly, what trailers to generate. Its too late to send #asMethodTrailer at method generation stage, since you don't know to whom delegate the responsibility for generating the right trailers.
Meanwhile, since there is no such layer on horizon, feel free to add a backwards compatibility patch. :)
On 19 May 2010 02:57, Eliot Miranda eliot.miranda@gmail.com wrote:
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.