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.