{"id":14,"date":"2008-11-14T15:41:52","date_gmt":"2008-11-14T22:41:52","guid":{"rendered":"http:\/\/www.mirandabanda.org\/cogblog\/?p=14"},"modified":"2008-11-16T18:41:18","modified_gmt":"2008-11-17T01:41:18","slug":"mechanised-modifications-and-miscellaneous-measurements","status":"publish","type":"post","link":"http:\/\/www.mirandabanda.org\/cogblog\/2008\/11\/14\/mechanised-modifications-and-miscellaneous-measurements\/","title":{"rendered":"Mechanised Modifications and Miscellaneous Measurements"},"content":{"rendered":"

You read the last post and I can tell you’re unhappy with the size of the remote temp bytecodes. I can hear you grumbling that it has got to hurt performance. "But these bytecodes are really rare in practice" I claim. "Prove it", you say. "I took a look and there are loads of the damn things", you say, and of course you’re right.<\/p>\n

Let’s measure how often these crop up in practice. I’m running these doits in a freshly bootstrapped Croquet 1.0.18 image. You can evaluate these in your own bootstrap image if you’re so inclined.<\/p>\n

\t        CompiledMethod instanceCount 67650<\/p>\n

OK, so we have quite a few methods. Since the compiler arranges that the first bytecode will be a pushNewArray opcode if there are remote temps in a method we can scan for these quite quickly. Note that this approach won’t find temporaries declared inside a block that are only accessed within nested blocks, but it is good enough. We can save complicatons by filtering out quick methods which return self, an instance variable nil, true or false as these don’t have bytecodes. We also need to filter out the use of pushNewArray for creating tuples. These have the sign bit set on the following byte, which is the number of elements in the array.<\/p>\n

\t        (SystemNavigation default allSelect:
\n\t        \t        [:m|
\n\t        \t        m isQuick not
\n\t        \t        and: [(m at: m initialPC) = 138
\n\t        \t        and: [(m at: m initialPC + 1) <= 127]]]) size 2428<\/p>\n

OK, so fully 2428 \/ 67650 asFloat = 0.03589061345158906, or 3.6% of methods contain remote temps. Argh! What’s going on?<\/p>\n

Well remember that the bootstrap has just been run in a system that up until now used BlueBook blocks where all temporaries are at method level and are accessed indirectly from within blocks. So up until now there has been little incentive to declare block-local temps at block-level scope. They end up being declared (and decompiled and pretty-printed) at method level anyway. Of course there is a case where a method-level temporary is purposefully used only at block level, when it is read before written. These shouldn’t be changed. Take for example the following method; I’m using ‡ to draw your attention to relevant detail. <\/p>\n

Collection methods for enumerating<\/i>
\ndetectMax:<\/b> <\/font>aBlock<\/font>
\n\t        <\/font>"Evaluate aBlock with each of the receiver’s elements as the argument.
\n\t        Answer the element for which aBlock evaluates to the highest magnitude.
\n\t        If collection empty, return nil. This method might also be called elect:."<\/font><\/p>\n

\t‡       <\/font>|<\/font> <\/font>maxElement<\/font> <\/font>maxValue<\/font> <\/font>val<\/font> <\/font>|<\/font>
\n\t        <\/font>self<\/font> <\/font>do:<\/font> [:<\/font>each<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>maxValue<\/font> <\/font>==<\/font> <\/font>nil<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font> <\/font>[<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>val<\/font> <\/font>:=<\/b> <\/font>aBlock<\/font> <\/font>value:<\/font> <\/font>each<\/font>)<\/font> <\/font>><\/font> <\/font>maxValue<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>
\n\t        \t        \t        \t        \t        <\/font>maxElement<\/font> <\/font>:=<\/b> <\/font>each<\/font>.
\n\t        \t        \t        \t        \t        <\/font>maxValue<\/font> <\/font>:=<\/b> <\/font>val<\/font>]<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>"first element"<\/font>
\n\t        \t        \t        \t        <\/font>maxElement<\/font> <\/font>:=<\/b> <\/font>each<\/font>.
\n\t        \t        \t        \t        <\/font>maxValue<\/font> <\/font>:=<\/b> <\/font>aBlock<\/font> <\/font>value:<\/font> <\/font>each<\/font>]<\/font>.
\n\t        \t        \t        \t        <\/font>"Note that there is no way to get the first element that works
\n\t        \t        \t        \t        for all kinds of Collections. Must test every one."<\/font>].
\n\t        <\/font>^<\/font> <\/font>maxElement<\/font><\/p>\n

We can change this by moving val to block-level scope to<\/font>
\ndetectMax:<\/b> <\/font>aBlock<\/font>
\n\t        <\/font>"Evaluate aBlock with each of the receiver’s elements as the argument.
\n\t        Answer the element for which aBlock evaluates to the highest magnitude.
\n\t        If collection empty, return nil. This method might also be called elect:."<\/font><\/p>\n

\t        <\/font>|<\/font> <\/font>maxElement<\/font> <\/font>maxValue<\/font> <\/font>|<\/font>
\n\t‡       <\/font>self<\/font> <\/font>do:<\/font> [:<\/font>each<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>val<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>maxValue<\/font> <\/font>==<\/font> <\/font>nil<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font> <\/font>[<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>val<\/font> <\/font>:=<\/b> <\/font>aBlock<\/font> <\/font>value:<\/font> <\/font>each<\/font>)<\/font> <\/font>><\/font> <\/font>maxValue<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>
\n\t        \t        \t        \t        \t        <\/font>maxElement<\/font> <\/font>:=<\/b> <\/font>each<\/font>.
\n\t        \t        \t        \t        \t        <\/font>maxValue<\/font> <\/font>:=<\/b> <\/font>val<\/font>]<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>"first element"<\/font>
\n\t        \t        \t        \t        <\/font>maxElement<\/font> <\/font>:=<\/b> <\/font>each<\/font>.
\n\t        \t        \t        \t        <\/font>maxValue<\/font> <\/font>:=<\/b> <\/font>aBlock<\/font> <\/font>value:<\/font> <\/font>each<\/font>]<\/font>.
\n\t        \t        \t        \t        <\/font>"Note that there is no way to get the first element that works
\n\t        \t        \t        \t        for all kinds of Collections. Must test every one."<\/font>].
\n\t        <\/font>^<\/font> <\/font>maxElement
\n<\/font>
\nbut we would break it if we changed it to<\/p>\n

detectMax:<\/b> <\/font>aBlock<\/font>
\n\t        <\/font>"Evaluate aBlock with each of the receiver’s elements as the argument.
\n\t        Answer the element for which aBlock evaluates to the highest magnitude.
\n\t        If collection empty, return nil. This method might also be called elect:."<\/font><\/p>\n

\t        <\/font>|<\/font> <\/font>maxElement<\/font> <\/font>|<\/font>
\n\t‡       <\/font>self<\/font> <\/font>do:<\/font> [:<\/font>each<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>maxValue<\/font> <\/font>val<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>maxValue<\/font> <\/font>==<\/font> <\/font>nil<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font> <\/font>[<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>val<\/font> <\/font>:=<\/b> <\/font>aBlock<\/font> <\/font>value:<\/font> <\/font>each<\/font>)<\/font> <\/font>><\/font> <\/font>maxValue<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>
\n\t        \t        \t        \t        \t        <\/font>maxElement<\/font> <\/font>:=<\/b> <\/font>each<\/font>.
\n\t        \t        \t        \t        \t        <\/font>maxValue<\/font> <\/font>:=<\/b> <\/font>val<\/font>]<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>"first element"<\/font>
\n\t        \t        \t        \t        <\/font>maxElement<\/font> <\/font>:=<\/b> <\/font>each<\/font>.
\n\t        \t        \t        \t        <\/font>maxValue<\/font> <\/font>:=<\/b> <\/font>aBlock<\/font> <\/font>value:<\/font> <\/font>each<\/font>]<\/font>.
\n\t        \t        \t        \t        <\/font>"Note that there is no way to get the first element that works
\n\t        \t        \t        \t        for all kinds of Collections. Must test every one."<\/font>].
\n\t        <\/font>^<\/font> <\/font>maxElement<\/font>
\nbecause maxValue would be nil on every iteration of the loop.<\/font> With this in mind let’s try and look closer and try and identify which temporaries are declared at method level but only used at block level.<\/p>\n

Review the previous post on the Closure Compiler and its closure analysis. TempVariableNodes have readingScopes and writingScopes instance variables tracking their references. We can detect temporaries that are declared at method level but have no reads or writes at method level. We need to parse methods, perform the closure analysis and then examine the remote temps. We can save time by using the filter above to only parse methods that have remote temps:
\n<\/font>
\n\t        (SystemNavigation default allSelect:
\n\t        \t        [:m|
\n\t        \t        m isQuick not
\n\t        \t        and: [(m at: m initialPC) = 138
\n\t        \t        and: [(m at: m initialPC + 1) <= 127
\n\t        \t        and: [| methodNode |
\n\t        \t        \t        methodNode := m methodClass parserClass new
\n\t        \t        \t        \t        \t        \t        \t        \t        parse: m getSourceFromFile
\n\t        \t        \t        \t        \t        \t        \t        \t        class: m methodClass.
\n\t        \t        \t        methodNode ensureClosureAnalysisDone.
\n\t        \t        \t        (methodNode instVarNamed: ‘temporaries’) last remoteTemps anySatisfy:
\n\t        \t        \t        \t        [:t|
\n\t        \t        \t        \t        ((t instVarNamed: ‘readingScopes’)
\n\t        \t        \t        \t        \t        ifNil: [true]
\n\t        \t        \t        \t        \t        ifNotNil: [:rs| (rs includesKey: methodNode block) not])
\n\t        \t        \t        \t        and: [(t instVarNamed: ‘writingScopes’)
\n\t        \t        \t        \t        \t        \t        ifNil: [true]
\n\t        \t        \t        \t        \t        \t        ifNotNil: [:ws| (ws includesKey: methodNode block) not]]]]]]]) size<\/p>\n

1507. A quiet year. The Aztec New Fire ceremony is held for the last time. da Vinvi completes the Mona Lisa. But no major conflicts. 1507 is apparently 7015 to 7016 in the Byzantine calendar. Don’t you just love<\/i> Wikipedia<\/a>? Not much seems to have happened for sure in 1507 BC, although we have apparently discovered the first remains of domesticated ferrets here abouts. Now where were we? Ah yes…<\/p>\n

So in fact only 2428 – 1507 = 921, or 921 \/ 67650.0 = 1.4% have an apparently legitimate need for remote temps. But that leaves at least 1507 methods that need editing to eliminate the unnecessary and bloated bytecodes. That’s a lot of editing. Sigh.<\/p>\n

Perhaps we can fix this automatically. We can already find the temps that need moving. Now we have to find where to move them to. How many methods are there that contain exactly one block? These should be easy to fix.<\/p>\n

\t        (SystemNavigation default allSelect:
\n\t        \t        [:m|
\n\t        \t        m isQuick not
\n\t        \t        and: [(m at: m initialPC) = 138
\n\t        \t        and: [(m at: m initialPC + 1) <= 127
\n\t        \t        and: [ | numBlocks |
\n\t        \t        \t        numBlocks := 0.
\n\t        \t        \t        (InstructionStream on: m) scanFor:
\n\t        \t        \t        \t        [:b|
\n\t        \t        \t        \t        b = 143 ifTrue: [numBlocks := numBlocks + 1].
\n\t        \t        \t        \t        numBlocks > 1].
\n\t        \t        \t        numBlocks = 1
\n\t        \t        and: [| methodNode |
\n\t        \t        \t        methodNode := m methodClass parserClass new
\n        \t        \t        \t        \t        \t        \t        \t        parse: m getSourceFromFile
\n        \t        \t        \t        \t        \t        \t        \t        class: m methodClass.
\n\t        \t        \t        methodNode ensureClosureAnalysisDone.
\n\t        \t        \t        (methodNode instVarNamed: ‘temporaries’) last remoteTemps anySatisfy:
\n\t        \t        \t        \t        [:t|
\n\t        \t        \t        \t        ((t instVarNamed: ‘readingScopes’)
\n\t        \t        \t        \t        \t        ifNil: [true]
\n\t        \t        \t        \t        \t        ifNotNil: [:rs| (rs includesKey: methodNode block) not])
\n\t        \t        \t        \t        and: [(t instVarNamed: ‘writingScopes’)
\n\t        \t        \t        \t        \t        \t        ifNil: [true]
\n\t        \t        \t        \t        \t        \t        ifNotNil: [:ws| (ws includesKey: methodNode block) not]]]]]]]]) size<\/p>\n

645. That’s 43%. Nearly half. This seems worth-while to me. What do we need to do? For the methods in question we have to find the set of method-level temp names that are only accessed remotely, edit them out of the method level declaration and add them to a block-level declaration. This is going to be complicated enough that we should probably use a class or two instead of doits. Besides I find syntax highlighting really helps. Let’s put a halt where the action begins and we can root around to make sense of things.<\/p>\n

Object <\/font>subclass:<\/font> <\/font>#TempScopeEditor<\/font>
\n\t        <\/font>instanceVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>category:<\/font> <\/font>‘Cog-Scripts’<\/font><\/font><\/p>\n

TempScopeEditor methods for source editing<\/i><\/font>
\nfixBlockLocalTempsInSingleBlockMethods<\/b>
\n\t        <\/font>"Move temporaries declared at method level but used only at block
\n\t         level to block level scope in methods that only contan one block
\n\t         (because this should be easier than dealing with multiple block cases)."<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>candidateMethodsSuchThat:<\/font>
\n\t        \t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>numBlocks<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        \t        \t        <\/font>(<\/font>InstructionStream <\/font>on:<\/font> <\/font>m<\/font>)<\/font> <\/font>scanFor:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>b<\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>b<\/font> <\/font>=<\/font> <\/font>143<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>numBlocks<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>numBlocks<\/font> <\/font>><\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>=<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>do:<\/font>\t        [:<\/font>source<\/font> :<\/font>class<\/font> :<\/font>methodNode<\/font> :<\/font>remoteTemps<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToMove<\/font> <\/font>blockNode<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>tempsToMove<\/font> <\/font>:=<\/b> <\/font>remoteTemps<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>.
\n\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font> <\/font>last<\/font>.
\n\t        \t        \t        <\/font>self<\/font> <\/font>halt<\/font>]<\/font>]<\/font><\/p>\n

We’ll adapt the script above to invoke the important block with some of the extra work already done:<\/p>\n

TempScopeEditor methods for accessing
\n<\/i><\/font>candidateMethodsSuchThat:<\/b> <\/font>methodSelectBlock<\/font> <\/font>do:<\/b> <\/font>quadBlock<\/font>
\n\t        <\/font>"Evaluate quadBlock with the source, class, methodNode and
\n\t         remoteTemporaries of all method that contain method-level
\n\t         remote temporaries for which methodSelectBlock answers true."<\/font>
\n\t        SystemNavigation <\/font>default<\/font> <\/font>allSelect:<\/font>
\n\t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>methodNode<\/font> <\/font>remoteTemps<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>(<\/font>m<\/font> <\/font>isQuick<\/font> <\/font>not<\/font>
\n\t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>m<\/font> <\/font>at:<\/font> <\/font>m<\/font> <\/font>initialPC<\/font>)<\/font> <\/font>=<\/font> <\/font>138<\/font>
\n\t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>m<\/font> <\/font>at:<\/font> <\/font>m<\/font> <\/font>initialPC<\/font> <\/font>+<\/font> <\/font>1<\/font>)<\/font> <\/font><=<\/font> <\/font>127<\/font>
\n\t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>methodSelectBlock<\/font> <\/font>value:<\/font> <\/font>m<\/font>)<\/font>
\n\t        \t        <\/font>and:<\/font> <\/font>[<\/font>methodNode<\/font> <\/font>:=<\/b> <\/font>m<\/font> <\/font>methodClass<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>parse:<\/font> <\/font>m<\/font> <\/font>getSourceFromFile<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>class:<\/font> <\/font>m<\/font> <\/font>methodClass<\/font>.
\n\t        \t        \t        <\/font>methodNode<\/font> <\/font>ensureClosureAnalysisDone<\/font>.
\n\t        \t        \t        <\/font>remoteTemps<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>instVarNamed:<\/font> <\/font>‘temporaries’<\/font><\/font>)<\/font> <\/font>last<\/font> <\/font>remoteTemps<\/font>.
\n\t        \t        \t        <\/font>remoteTemps<\/font> <\/font>anySatisfy:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[<\/font>true<\/font>]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[<\/font>true<\/font>]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/font>]<\/font>]<\/font>]<\/font>]<\/font>)<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        <\/font>[<\/font>quadBlock<\/font>
\n\t        \t        \t        \t        <\/font>value:<\/font> <\/font>m<\/font> <\/font>getSourceFromFile<\/font> <\/font>asString<\/font>
\n\t        \t        \t        \t        <\/font>value:<\/font> <\/font>m<\/font> <\/font>methodClass<\/font>
\n\t        \t        \t        \t        <\/font>value:<\/font> <\/font>methodNode<\/font>
\n\t        \t        \t        \t        <\/font>value:<\/font> <\/font>remoteTemps<\/font>]<\/font>.
\n\t        \t        <\/font>false<\/font>]<\/p>\n

And we need a visitor to fish the block node out of the parse tree:<\/p>\n

ParseNodeVisitor <\/font>subclass:<\/font> <\/font>#BlockNodeCollectingVisitor<\/font>
\n\t        <\/font>instanceVariableNames:<\/font> <\/font>‘blockNodes’<\/font><\/font>
\n\t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>category:<\/font> <\/font>‘Cog-Scripts’<\/font><\/font><\/p>\n

<\/font>BlockNodeCollectingVisitor methods for accessing<\/i><\/font>
\n<\/font>blockNodes<\/b>
\n\t        <\/font>^<\/font>blockNodes<\/p>\n

<\/font>BlockNodeCollectingVisitor methods for visiting<\/i><\/font>
\n<\/font>visitBlockNode:<\/b> <\/font>aBlockNode<\/font>
\n\t        (blockNodes <\/font>ifNil:<\/font> <\/font>[<\/font>blockNodes <\/font>:=<\/b> OrderedCollection <\/font>new<\/font>]<\/font>) <\/font>addLast:<\/font> <\/font>aBlockNode<\/font> <\/p>\n

My first mistake is in assuming the last block will be the block I’m interested in. But the visitor finds all blocks, including optimized ones that will be inlined (as in ifTrue: [a block the compiler inlines]):<\/p>\n

\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>last<\/font>.<\/p>\n

Better. Now let’s get the start of the block from its source range, the same source range the debugger uses to high-light methods while debugging:<\/p>\n

\t        \t        \t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font>)<\/font> <\/font>first<\/font>.<\/p>\n

Let’s evaluate
\n<\/font>
\n\t        TempScopeEditor new fixBlockLocalTempsInSingleBlockMethods<\/p>\n

amd root around at the halt. I want to evaluate<\/p>\n

\t        source copyFrom: 1 to: startOfBlock<\/p>\n

in the debugger to see what’s what. And… Bang!<\/strong><\/em> We’re now two days into the future (with little progress having been made). Since this is Cinéma vérité<\/a> I need to relate what happened. The VM crashed hard as soon as I evaluated source copyFrom: 1 to: startOfBlock<\/i>. I spent two days trying to track down what I thought was a VM bug but was of course my own fault. My original version of the method in BytecodeEncoder to compute the size of a bytecode looked like this:<\/p>\n

BytecodeEncoder methods for opcode sizing<\/i>
\nsizeOpcodeSelector:<\/b> <\/font>selector<\/font> <\/font>withArguments:<\/b> <\/font>args<\/font>
\n\t        <\/font>|<\/font> <\/font>start<\/font> <\/font>|<\/font>
\n\t        stream <\/font>ifNil:<\/font> [stream <\/font>:=<\/b> WriteStream <\/font>on:<\/font> <\/font>(<\/font>ByteArray <\/font>new:<\/font> <\/font>64<\/font>)<\/font>].
\n\t        <\/font>start<\/font> <\/font>:=<\/b> stream <\/font>position<\/font>.
\n\t        <\/font>^<\/font>[<\/font>self<\/font> <\/font>perform:<\/font> <\/font>selector<\/font> <\/font>withArguments:<\/font> <\/font>args<\/font>.
\n\t         stream <\/font>position<\/font> <\/font>–<\/font> <\/font>start<\/font>]
\n\t        \t        <\/font>ensure:<\/font> [stream <\/font>position:<\/font> <\/font>start<\/font>]<\/p>\n

Later on I realized that the ensure: served little purpose. If there was an error while generating the code the compilation was useless and restoring the stream position pointless. What was important was to see the error and correct it. So I modified the method to<\/p>\n

<\/font>sizeOpcodeSelector:<\/b> <\/font>selector<\/font> <\/font>withArguments:<\/b> <\/font>args<\/font>
\n\t        <\/font>|<\/font> <\/font>start<\/font> <\/font>result<\/font> <\/font>|<\/font>
\n\t        stream <\/font>ifNil:<\/font> [stream <\/font>:=<\/b> WriteStream <\/font>on:<\/font> <\/font>(<\/font>ByteArray <\/font>new:<\/font> <\/font>64<\/font>)<\/font>].
\n\t        <\/font>start<\/font> <\/font>:=<\/b> stream <\/font>position<\/font>.
\n\t        <\/font>self<\/font> <\/font>perform:<\/font> <\/font>selector<\/font> <\/font>withArguments:<\/font> <\/font>args<\/font>.
\n\t        <\/font>result<\/font> <\/font>:=<\/b> stream <\/font>position<\/font> <\/font>–<\/font> <\/font>start<\/font>.
\n\t        stream <\/font>position:<\/font> <\/font>start<\/font>.
\n\t        <\/font>^<\/font>result<\/font><\/p>\n

and then early on while writing this post I realized that remembering the position was silly. I might as well position the stream to zero and simply answer its resulting position. I could then also use a smaller stream for sizing since only one opcode’s worth of bytecode was ever going to be written to the stream:<\/p>\n

<\/font>sizeOpcodeSelector:<\/b> <\/font>genSelector<\/font> <\/font>withArguments:<\/b> <\/font>args<\/font>
\n\t        stream <\/font>ifNil:<\/font> [stream <\/font>:=<\/b> WriteStream <\/font>on:<\/font> <\/font>(<\/font>ByteArray <\/font>new:<\/font> <\/font>64<\/font>)<\/font>].
\n\t        stream <\/font>position:<\/font> <\/font>0<\/font>.
\n\t        <\/font>self<\/font> <\/font>perform:<\/font> <\/font>genSelector<\/font> <\/font>withArguments:<\/font> <\/font>args<\/font>.
\n\t        <\/font>^<\/font>stream <\/font>position<\/font><\/p>\n

Much nicer… …except that it resulted in a memory corruption that crashed the VM in the garbage collector. And fool that I am, I spent two days tracking down the bug thinking it was a VM bug related to my new closure bytecodes rather than to my immediately preceeding compiler "tweaking".<\/p>\n

The bug resulted from DebuggerMethodMap, the abstraction I introduced to insulate the Debugger from different bytecode sets. A DebuggerMethodMap holds onto a BytecodeAgnosticMethodNode and uses it to compute both the set of temporary names to display in the debugger and the map of pc to source range for high-lighting the current expression. Like the existing code it replaced the temp names and source extents are derived from doing a scratch recompilation of the mehtod source. When the DebuggerMethodMap instance derived the source map after deriving the temp names in displaying the <\/font>fixBlockLocalTempsInSingleBlockMethods method in the debugger it caused the BytecodeAgnosticMethodNode to reuse its BytecodeEncoder. The encoder already had its stream initialised to the first method it generated to produce the temp names. So when<\/p>\n

\t        stream <\/font>position:<\/font> <\/font>0<\/font>.<\/p>\n

was evaluated this reset the stream to point at the header of the previously generated CompiledMethod whose header word was promptly corrupted when generating the bytecode to be sized. Soon thereafter the allocations done in evaluating <\/font>source copyFrom: 1 to: startOfBlock<\/i> caused the garbage collector to run which encountered the corrupted CompiledMethod header which completely screwed up the garbage collector’s parsing of the heap and caused the hard crash. Ouch.<\/p>\n

There are a few lessons to be learnt from this. One, which my own hubris means I have to relearn at alarming frequency is that if one has just changed something and the VM crashes soon thereafter it’s almost certainly one’s own bloody fault, dimwit!! Another is that a system is only as safe as one makes it. Array accesses in Smalltalk are safe, being bounds and type checked… except for blue-book CompiledMethod whose at:put: method does not check that a byte is being stored at or after a CompiledMethod’s initialPC (this at least is fixable). I can hear Tim saying re blue-book CompiledMethods "I told you so!" :).<\/p>\n

OK, so two days further on how should we fix this? Since all we need is the count of bytes written we can provide BytecodeEncoder with its own nextPut: and postion and we’re done:<\/p>\n

Encoder <\/font>subclass:<\/font> <\/font>#BytecodeEncoder<\/font>
\n\t        <\/font>instanceVariableNames:<\/font> <\/font>‘stream position rootNode blockExtentsToLocals’<\/font><\/font>
\n\t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>category:<\/font> <\/font>‘Compiler-Kernel’<\/font><\/font>
\n<\/font>
\nBytecodeEncoder methods for opcode sizing<\/i>
\n<\/font>nextPut:<\/b> <\/font>aByte<\/font>
\n\t        <\/font>"For sizing make the encoder its own stream and
\n\t         keep track of position with this version of nextPut:"<\/font>
\n\t        position <\/font>:=<\/b> position <\/font>+<\/font> <\/font>1<\/font><\/p>\n

<\/font>sizeOpcodeSelector:<\/b> <\/font>genSelector<\/font> <\/font>withArguments:<\/b> <\/font>args<\/font>
\n\t        stream <\/font>:=<\/b> <\/font>self<\/font>.
\n\t        position <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        <\/font>self<\/font> <\/font>perform:<\/font> <\/font>genSelector<\/font> <\/font>withArguments:<\/font> <\/font>args<\/font>.
\n\t        <\/font>^<\/font>position<\/p>\n

Much nicer.<\/p>\n

Now this might seem much more like a documentary to you than Kino-Pravda<\/a> but alas soon after writing the first version of the above mini-tragedy (it being driven by my own character flaws) I lost the VM bug story by inadvertently closing the workspace containing my carefully formatted post and saving the image. Bang!<\/em> Hence I have spent some time in the editing room recreating this. Ouch #2.<\/p>\n

So where were we? Right; we were trying to edit methods with misplaced block temporaries. Now I can evaluate <\/font>source copyFrom: 1 to: startOfBlock<\/i>. but it includes the entire block, not just the first ‘[‘. Hmm. My second mistake is in assuming the source range of a block is the entire block when in fact it is the last statement. Please wait while I modify the compiler… (perhaps this is more like Magical Realism<\/a> …Done. Now each BlockNode assigns its entire source range to its closureCreationNode:<\/p>\n

BlockNode methods for accessing<\/font><\/i>
\n<\/font>closureCreationNode<\/b>
\n\t        closureCreationNode <\/font>ifNil:<\/font>
\n\t        \t        [closureCreationNode <\/font>:=<\/b> LeafNode <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>key:<\/font> <\/font>#closureCreationNode<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>code:<\/font> <\/font>nil<\/font>].
\n\t        <\/font>^<\/font>closureCreationNode<\/p>\n

<\/font>BlockNode methods for initialize-release<\/i><\/font>
\n<\/font>noteSourceRangeStart:<\/b> <\/font>start<\/font> <\/font>end:<\/b> <\/font>end<\/font> <\/font>encoder:<\/b> <\/font>encoder<\/font>
\n\t        <\/font>"Note two source ranges for this node. One is for the debugger
\n\t         and is of the last expression, the result of the block. One is for
\n\t         source analysis and is for the entire block."<\/font>
\n\t        <\/font>encoder<\/font>
\n\t        \t        <\/font>noteSourceRange:<\/font> (<\/font>start<\/font> <\/font>to:<\/font> <\/font>end<\/font>)
\n\t        \t        <\/font>forNode:<\/font> <\/font>self<\/font> <\/font>closureCreationNode<\/font>.
\n\t        startOfLastStatement
\n\t        \t        <\/font>ifNil:<\/font>
\n\t        \t        \t        [<\/font>encoder<\/font>
\n\t        \t        \t        \t        <\/font>noteSourceRange:<\/font> <\/font>(<\/font>start<\/font> <\/font>to:<\/font> <\/font>end<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>forNode:<\/font> <\/font>self<\/font>]
\n\t        \t        <\/font>ifNotNil:<\/font>
\n\t        \t        \t        [<\/font>encoder<\/font>
\n\t        \t        \t        \t        <\/font>noteSourceRange:<\/font> <\/font>(<\/font>startOfLastStatement <\/font>to:<\/font> <\/font>end<\/font> <\/font>–<\/font> <\/font>1<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>forNode:<\/font> <\/font>self<\/font>]<\/p>\n

OK, so now our method looks like<\/font><\/p>\n

TempScopeEditor methods for source editing<\/i><\/font>
\nfixBlockLocalTempsInSingleBlockMethods<\/b>
\n\t        <\/font>"Move temporaries declared at method level but used only at block
\n\t         level to block level scope in methods that only contan one block
\n\t         (because this should be easier than dealing with multiple block cases)."<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>candidateMethodsSuchThat:<\/font>
\n\t        \t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>numBlocks<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        \t        \t        <\/font>(<\/font>InstructionStream <\/font>on:<\/font> <\/font>m<\/font>)<\/font> <\/font>scanFor:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>b<\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>b<\/font> <\/font>=<\/font> <\/font>143<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>numBlocks<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>numBlocks<\/font> <\/font>><\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>=<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>do:<\/font>\t  [:<\/font>source<\/font> :<\/font>class<\/font> :<\/font>methodNode<\/font> :<\/font>remoteTemps<\/font>|<\/font> <\/font>
\n        \t        \t        |<\/font> <\/font>tempsToMove<\/font> <\/font>blockNode<\/font> <\/font>startOfBlock |<\/font>
\n\t        \t        \t        <\/font>tempsToMove<\/font> <\/font>:=<\/b> <\/font>remoteTemps<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>.
\n\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font>
\n\t‡       \t        \t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>last<\/font>.
\n\t‡       \t        \t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>)<\/font> <\/font>first<\/font>.
\n\t        \t        \t        <\/font>self<\/font> <\/font>halt<\/font>]<\/font>]<\/p>\n

Let’s evaluate it and root around the halt to see that my next error is a misplaced closing bracket. There are two following the halt and there should be three at the end of the block computing tempsToMove:<\/font><\/p>\n

TempScopeEditor methods for source editing<\/i><\/font>
\nfixBlockLocalTempsInSingleBlockMethods<\/b>
\n\t        <\/font>"Move temporaries declared at method level but used only at block
\n\t         level to block level scope in methods that only contan one block
\n\t         (because this should be easier than dealing with multiple block cases)."<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>candidateMethodsSuchThat:<\/font>
\n\t        \t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>numBlocks<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        \t        \t        <\/font>(<\/font>InstructionStream <\/font>on:<\/font> <\/font>m<\/font>)<\/font> <\/font>scanFor:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>b<\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>b<\/font> <\/font>=<\/font> <\/font>143<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>numBlocks<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>numBlocks<\/font> <\/font>><\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>=<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>do:<\/font>\t        [:<\/font>source<\/font> :<\/font>class<\/font> :<\/font>methodNode<\/font> :<\/font>remoteTemps<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToMove<\/font> <\/font>blockNode<\/font> <\/font>startOfBlock |<\/font>
\n\t        \t        \t        <\/font>tempsToMove<\/font> <\/font>:=<\/b> <\/font>remoteTemps<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]]<\/font>]<\/font>.
\n\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>last<\/font>.
\n\t        \t        \t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>)<\/font> <\/font>first<\/font>.
\n\t        \t        \t        <\/font>self<\/font> <\/font>halt<\/font>]<\/font> ‡<\/p>\n

Now <\/font>source copyFrom: 1 to: startOfBlock is<\/p>\n

\t        ‘copySameFrom: otherObject
\n\t        \t        "Copy to myself all instance variables named the same in otherObject.
\n\t        \t        This ignores otherObject”s control over its own inst vars."
\n\t        
\n\t        \t        | myInstVars otherInstVars match |
\n\t        \t        myInstVars := self class allInstVarNames.
\n\t        \t        otherInstVars := otherObject class allInstVarNames.
\n\t        \t        myInstVars doWithIndex: [‘<\/p>\n

\"halt and tempsToMove is an OrderedCollection({match}). Now we can start editing, a process complex enough to need its own method. It will likely need the blockNode itself, and we’ll want to eyeball the results:<\/p>\n

fixBlockLocalTempsInSingleBlockMethods<\/b>
\n\t        <\/font>"Move temporaries declared at method level but used only at block
\n\t         level to block level scope in methods that only contan one block
\n\t         (because this should be easier than dealing with multiple block cases)."<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>candidateMethodsSuchThat:<\/font>
\n\t        \t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>numBlocks<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        \t        \t        <\/font>(<\/font>InstructionStream <\/font>on:<\/font> <\/font>m<\/font>)<\/font> <\/font>scanFor:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>b<\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>b<\/font> <\/font>=<\/font> <\/font>143<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>numBlocks<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>numBlocks<\/font> <\/font>><\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>=<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>do:<\/font>\t        [:<\/font>source<\/font> :<\/font>class<\/font> :<\/font>methodNode<\/font> :<\/font>remoteTemps<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToMove<\/font> <\/font>blockNode<\/font> <\/font>newSource<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>tempsToMove<\/font> <\/font>:=<\/b> <\/font>remoteTemps<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/font>.
\n\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>last<\/font>.
\n\t‡       \t        \t        <\/font>newSource<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>moveTemps:<\/font> <\/font>tempsToMove<\/font> <\/font>to:<\/font> <\/font>blockNode<\/font> <\/font>in:<\/font> <\/font>source<\/font> <\/font>encoder:<\/font> <\/font>methodNode<\/font> <\/font>encoder<\/font>.
\n\t        \t        \t        <\/font>(<\/font>StringHolder <\/font>new<\/font> <\/font>textContents:<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>TextDiffBuilder <\/font>buildDisplayPatchFrom:<\/font> <\/font>source<\/font> <\/font>to:<\/font> <\/font>newSource<\/font>)<\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>openLabel:<\/font> <\/font>‘temps edit’<\/font><\/font>.
\n\t        \t        \t        <\/font>self<\/font> <\/font>halt<\/font>]<\/font><\/p>\n

but since we need the methodNode to know which temporaries to keep my first cut ends up being<\/p>\n

<\/font>moveTemps:<\/b> <\/font>tempsToMove<\/font> <\/font>in:<\/b> <\/font>methodNode<\/font> <\/font>to:<\/b> <\/font>blockNode<\/font> <\/font>in:<\/b> <\/font>sourceString<\/font> <\/font>encoder:<\/b> <\/font>encoder<\/font>
\n\t        <\/font>^<\/font>String <\/font>streamContents:<\/font>
\n\t        \t        [:<\/font>s<\/font>|<\/font>\t        <\/font>|<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>startOfBlock<\/font> <\/font>endOfArgs<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>methodNode<\/font> <\/font>block<\/font> <\/font>temporaries<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>t<\/font>]<\/font>.
\n\t        \t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>)<\/font> <\/font>first<\/font>.
\n\t        \t        <\/font>endOfArgs<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>startOfBlock<\/font>.
\n\t        \t        <\/font>tempsStart<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font>.
\n\t        \t        <\/font>tempsEnd<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        <\/font>tempsToKeep<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font>
\n\t        \t        \t        \t        \t        <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font>;
\n\t        \t        \t        \t        \t        <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t        \t         <\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>next:<\/font> <\/font>(<\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font>)<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font>.
\n\t        \t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>; <\/font>space<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>s<\/font> <\/font>space<\/font>; <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/font>]<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>; <\/font>space<\/font>]<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>next:<\/font> <\/font>sourceString<\/font> <\/font>size<\/font> <\/font>–<\/font> <\/font>endOfArgs<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>
\n<\/font>
\nwhich produces the following diff:<\/p>\n

\t        <\/font>copySameFrom: otherObject
\n\t        <\/font>\t        "Copy to myself all instance variables named the same in otherObject.
\n\t        <\/font>\t        This ignores otherObject’s control over its own inst vars."
\n\t        
\n<\/font>\t        <\/font>\t        | myInstVars otherInstVars i iLimiT <0-6> |<\/font>
\n<\/font>\t        <\/font>\t        | myInstVars otherInstVars match |<\/font>
\n\t        <\/font>\t        myInstVars := self class allInstVarNames.
\n\t        <\/font>\t        otherInstVars := otherObject class allInstVarNames.
\n<\/font>\t        <\/font>\t        myInstVars doWithIndex: [:each :index | | match |<\/font>
\n<\/font>\t        <\/font>\t        myInstVars doWithIndex: [:each :index |<\/font>
\n\t        <\/font>\t        \t        (match := otherInstVars indexOf: each) > 0 ifTrue:
\n\t        <\/font>\t        \t        \t        [self instVarAt: index put: (otherObject instVarAt: match)]].
\n\t        <\/font>\t        1 to: (self basicSize min: otherObject basicSize) do: [:i |
\n\t        <\/font>\t        \t        self basicAt: i put: (otherObject basicAt: i)].<\/p>\n

Progress. We need to filter-out the implicit temporary "<0-6>" used to hold indirect temps:
\n
\n\t        \t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>instVarNamed:<\/font> <\/font>‘temporaries’<\/font><\/font>)<\/font> <\/font>reject:<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>t<\/font> <\/font>isIndirectTempVector<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>t<\/font>]<\/font>]<\/font>.<\/p>\n

and filter-out implicit temporaries the compiler introduces when optimizing to:do: blocks (<\/font>i<\/font> & <\/font>iLimiT<\/font>):<\/p>\n

\t        \t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode<\/font> <\/font>instVarNamed:<\/font> <\/font>‘temporaries’<\/font><\/font>)<\/font> <\/font>reject:<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>t<\/font> <\/font>isIndirectTempVector<\/font> <\/font>or:<\/font> <\/font>[<\/font>t<\/font> <\/font>scope<\/font> <\/font><<\/font> <\/font>0<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>t<\/font>]<\/font>]<\/font>]<\/font>.<\/p>\n

and <\/font>include the indirect temps the implicit temp vector holds:
\n
\n\t        \t        <\/font>tempsToKeep<\/font> <\/font>:= OrderedCollection <\/font>new<\/font>.
\n\t        \t        <\/font>methodNode<\/font> <\/font>block<\/font> <\/font>temporaries<\/font> <\/font>do:<\/font>
\n\t        \t        \t        <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        <\/font>t<\/font> <\/font>isIndirectTempVector<\/font>
\n\t        \t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>t<\/font> <\/font>remoteTemps<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>remoteTemp<\/font>|<\/font>
\n\t        \t        \t        \t        \t        \t         <\/font>(<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>remoteTemp<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>remoteTemp<\/font>]<\/font>]<\/font>]<\/font>
\n\t        \t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>(<\/font>t<\/font> <\/font>scope<\/font> <\/font><<\/font> <\/font>0<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>t<\/font>]<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>t<\/font>]<\/font>]<\/font>]<\/font>.
\n<\/font>
\nAnd there are extra spaces following each temporary:
\n
\n\t        \t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         …
\n\t        \t        <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.<\/p>\n

Alright, a correct edit!<\/p>\n

\t        <\/font>copySameFrom: otherObject
\n\t        <\/font>\t        "Copy to myself all instance variables named the same in otherObject.
\n\t        <\/font>\t        This ignores otherObject’s control over its own inst vars."
\n\t        
\n<\/font>\t        <\/font>\t        | myInstVars otherInstVars |<\/font>
\n<\/font>\t        <\/font>\t        | myInstVars otherInstVars match |<\/font>
\n\t        <\/font>\t        myInstVars := self class allInstVarNames.
\n\t        <\/font>\t        otherInstVars := otherObject class allInstVarNames.
\n\t        <\/font>\t        myInstVars doWithIndex: [:each :index |
\n\t        <\/font>\t        \t        (match := otherInstVars indexOf: each) > 0 ifTrue:
\n\t        <\/font>\t        \t        \t        [self instVarAt: index put: (otherObject instVarAt: match)]].
\n<\/font>\t        <\/font>\t        1 to: (self basicSize min: otherObject basicSize) do: [:i | | match |<\/font>
\n<\/font>\t        <\/font>\t        1 to: (self basicSize min: otherObject basicSize) do: [:i |<\/font>
\n\t        <\/font>\t        \t        self basicAt: i put: (otherObject basicAt: i)].<\/p>\n

So now instead of eyeballing let’s parse the result to test what we’ve got so far. We’ll print to the transcript to track progress:<\/p>\n

fixBlockLocalTempsInSingleBlockMethods<\/b>
\n\t        <\/font>"Move temporaries declared at method level but used only at block
\n\t         level to block level scope in methods that only contan one block
\n\t         (because this should be easier than dealing with multiple block cases)."<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>candidateMethodsSuchThat:<\/font>
\n\t        \t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>numBlocks<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        \t        \t        <\/font>(<\/font>InstructionStream <\/font>on:<\/font> <\/font>m<\/font>)<\/font> <\/font>scanFor:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>b<\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>b<\/font> <\/font>=<\/font> <\/font>143<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>numBlocks<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>numBlocks<\/font> <\/font>><\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>=<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>do:<\/font>\t        [:<\/font>source<\/font> :<\/font>class<\/font> :<\/font>methodNode<\/font> :<\/font>remoteTemps<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToMove<\/font> <\/font>blockNode<\/font> <\/font>newSource<\/font> <\/font>|<\/font>
\n\t        \t        \t        Transcript <\/font>cr<\/font>; <\/font>print:<\/font> <\/font>class<\/font>; <\/font>nextPutAll:<\/font> <\/font>‘>>’<\/font><\/font>; <\/font>print:<\/font> <\/font>methodNode<\/font> <\/font>selector<\/font>; <\/font>flush<\/font>.
\n\t        \t        \t        <\/font>tempsToMove<\/font> <\/font>:=<\/b> <\/font>remoteTemps<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/font>.
\n\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>last<\/font>.
\n\t        \t        \t        <\/font>newSource<\/font> <\/font>:=<\/b> <\/font>self<\/font>\t        <\/font>moveTemps:<\/font> <\/font>tempsToMove<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>in:<\/font> <\/font>methodNode<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>to:<\/font> <\/font>blockNode<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>in:<\/font> <\/font>source<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>encoder:<\/font> <\/font>methodNode<\/font> <\/font>encoder<\/font>.
\n\t        \t        \t        <\/font>class<\/font> <\/font>parserClass<\/font> <\/font>new<\/font> <\/font>parse:<\/font> <\/font>newSource<\/font> <\/font>class:<\/font> <\/font>class<\/font>.
\n\t        \t        \t        <\/font>"(StringHolder new textContents:
\n\t        \t        \t        \t        (TextDiffBuilder buildDisplayPatchFrom: source to: newSource))
\n\t        \t        \t        \t        \t        openLabel: ‘temps edit’."<\/font>]<\/p>\n

Bang! <\/font>Error: subscript is out of bounds: -6<\/b>. Huh? This while trying to edit<\/p>\n

Object methods for *Tweak-Core-Object<\/i>
\nget:<\/b> <\/font>fieldName<\/font>
\n\t        <\/font>|<\/font> <\/font>var<\/font> <\/font>|<\/font>
\n\t        <\/font>^self<\/font> <\/font>instVarAt:<\/font> (<\/font>self<\/font> <\/font>class<\/font> <\/font>allInstVarNames<\/font> <\/font>indexOf:<\/font> <\/font>fieldName<\/font> <\/font>ifAbsent:<\/font>[<\/font>
\n\t        \t        <\/font>^<\/font>(<\/font>var<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>class<\/font> <\/font>bindingOf:<\/font> <\/font>fieldName<\/font>)<\/font> <\/font>ifNotNil:<\/font>[<\/font>var<\/font> <\/font>value<\/font>]<\/font>.
\n\t        <\/font>]<\/font>)<\/p>\n

And of course there are no arguments or temporaries in<\/p>\n

\t        <\/font>[<\/font>^<\/font>(<\/font>var<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>class<\/font> <\/font>bindingOf:<\/font> <\/font>fieldName<\/font>)<\/font> <\/font>ifNotNil:<\/font>[<\/font>var<\/font> <\/font>value<\/font>]<\/font>.<\/font>]<\/p>\n

<\/font>so endOfArgs is zero and the<\/p>\n

\t        <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/p>\n

<\/font>expression ends up moving the stream backwards 6 characters! So:<\/p>\n

\t        \t        <\/font>endOfArgs<\/font> <\/font>:=<\/b> <\/font>blockNode<\/font> <\/font>arguments<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>startOfBlock<\/font>]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifFalse:<\/font> <\/font>[<\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>startOfBlock<\/font>]<\/font>.<\/p>\n

And now the parser chugs along parsing about 90 edited methods until a SyntaxError:<\/p>\n

\t        <\/font>readsTweakField:<\/b> <\/font>field<\/font>
\n\t        \t        <\/font>"Answer whether the receiver reads the given field"<\/font>
\n\t        \t        <\/font>|<\/font> <\/font>toGet<\/font> <\/font>scanner<\/font> <\/font>byte<\/font> <\/font>type<\/font> <\/font>max<\/font> <\/font>offset<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>toGet<\/font> <\/font>:=<\/b> <\/font>field<\/font> <\/font>toGet<\/font> <\/font>ifNil:<\/font>[<\/font>^false<\/font>].
\n\t        \t        (<\/font>self<\/font> <\/font>hasLiteral:<\/font> <\/font>toGet<\/font>) <\/font>ifFalse:<\/font>[<\/font>^false<\/font>].
\n\t        \t        <\/font>max<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>numLiterals<\/font>.
\n\t        \t        <\/font>"We scan the first sixteen accurately"<\/font>
\n\t        \t        <\/font>max<\/font> <\/font>><\/font> <\/font>16<\/font> <\/font>ifTrue:<\/font>[<\/font>max<\/font> <\/font>:=<\/b> <\/font>16<\/font>].
\n\t        \t        <\/font>1<\/font> <\/font>to:<\/font> <\/font>self<\/font> <\/font>numLiterals<\/font> <\/font>do:<\/font>[:<\/font>Name is already defined -><\/b><\/font>i<\/font>|<\/font>
\n\t        \t        \t        <\/font>(<\/font>self<\/font> <\/font>literalAt:<\/font> <\/font>i<\/font>)<\/font> <\/font>==<\/font> <\/font>toGet<\/font> <\/font>ifTrue:<\/font>[<\/font>
\n\t        \t        \t        \t        <\/font>"scan for push: self; send: toGet"<\/font>
\n\t        \t        \t        \t        <\/font>scanner<\/font> <\/font>:=<\/b> InstructionStream <\/font>on:<\/font> <\/font>self<\/font>.
\n\t        \t        \t        \t        <\/font>scanner<\/font> <\/font>scanFor:<\/font> <\/font>[<\/font>:<\/font>insn<\/font>|<\/font>
\n\t        \t        \t        \t        \t        <\/font>(<\/font>insn<\/font> <\/font>=<\/font> <\/font>16r70<\/font> <\/font>"push self"<\/font>)<\/font> <\/font>ifTrue:<\/font>[<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>byte<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>at:<\/font> <\/font>scanner<\/font> <\/font>pc+<\/font>1<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>type<\/font> <\/font>:=<\/b> <\/font>byte<\/font> <\/font>\/\/<\/font> <\/font>16<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>offset<\/font> <\/font>:=<\/b> <\/font>byte<\/font> <\/font>\\\\<\/font> <\/font>16<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>type<\/font> <\/font>><\/font> <\/font>12<\/font> <\/font>ifTrue:<\/font>[<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>(<\/font>offset<\/font>+<\/font>1<\/font> <\/font>=<\/font> <\/font>i<\/font>)<\/font> <\/font>ifTrue:<\/font>[<\/font>^true<\/font>]<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>]<\/font>.
\n\t        \t        \t        \t        \t        <\/font>]<\/font>.
\n\t        \t        \t        \t        \t        <\/font>false<\/font>
\n\t        \t        \t        \t        <\/font>]<\/font>.
\n\t        \t        \t        <\/font>]<\/font>.
\n\t        \t        ].
\n\t        \t        <\/font>17<\/font> <\/font>to:<\/font> <\/font>self<\/font> <\/font>numLiterals<\/font> <\/font>do:<\/font>[:<\/font>i<\/font>|<\/font>
\n\t        \t        \t        <\/font>(<\/font>self<\/font> <\/font>literalAt:<\/font> <\/font>i<\/font>)<\/font> <\/font>==<\/font> <\/font>toGet<\/font> <\/font>ifTrue:<\/font>[<\/font>^true<\/font>]<\/font>.
\n\t        \t        ].
\n\t        \t        <\/font>^false<\/font><\/p>\n

Here i is remote because it is implicitly assigned to at the end of the 1 to: self numLiterals do:[:i|…] loop and read by the<\/p>\n

\t        \t        \t        \t        [<\/font>:<\/font>insn<\/font>|<\/font>
\n\t        \t        \t        \t        \t        <\/font>(<\/font>insn<\/font> <\/font>=<\/font> <\/font>16r70<\/font> <\/font>"push self"<\/font>)<\/font> <\/font>ifTrue:<\/font>[<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>byte<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>at:<\/font> <\/font>scanner<\/font> <\/font>pc+<\/font>1<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>type<\/font> <\/font>:=<\/b> <\/font>byte<\/font> <\/font>\/\/<\/font> <\/font>16<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>offset<\/font> <\/font>:=<\/b> <\/font>byte<\/font> <\/font>\\\\<\/font> <\/font>16<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>type<\/font> <\/font>><\/font> <\/font>12<\/font> <\/font>ifTrue:<\/font>[<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>(<\/font>offset<\/font>+<\/font>1<\/font> <\/font>=<\/font> <\/font>i<\/font>)<\/font> <\/font>ifTrue:<\/font>[<\/font>^true<\/font>]<\/font>.
\n\t        \t        \t        \t        \t        \t        <\/font>]<\/font>.
\n\t        \t        \t        \t        \t        <\/font>]<\/font>.
\n\t        \t        \t        \t        \t        <\/font>false<\/font>
\n\t        \t        \t        \t        <\/font>]<\/font><\/p>\n

block. So we need to filter-out implicitly declared temps from tempsToMove in both arms:
\n
\n\t        \t        <\/font>methodNode<\/font> <\/font>block<\/font> <\/font>temporaries<\/font> <\/font>do:<\/font>
\n\t        \t        \t        <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        <\/font>t<\/font> <\/font>isIndirectTempVector<\/font>
\n\t        \t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>t<\/font> <\/font>remoteTemps<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>rt<\/font>|<\/font>
\n\t        \t        \t        \t        \t        \t         <\/font>(<\/font>rt<\/font> <\/font>scope<\/font> <\/font><<\/font> <\/font>0<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>rt<\/font>]<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>rt<\/font>]<\/font>]<\/font>]<\/font>
\n\t        \t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>(<\/font>t<\/font> <\/font>scope<\/font> <\/font><<\/font> <\/font>0<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>t<\/font>]<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>t<\/font>]<\/font>]<\/font>]<\/font>.<\/p>\n

Which results in 135 successful parses until Color class>><\/font>colorTest:extent:colorMapper:. One reason to keep the StringHolder diff expression in a comment in the fixBlockLocalTempsInSingleBlockMethods method is so I can easily evaluate it in the debugger and see the diff:<\/p>\n

\t        colorTest: depth extent: chartExtent colorMapper: colorMapper
\n\t        \t        "Create a palette of colors sorted horizontally by hue and vertically by lightness. Useful for eyeballing the color gamut of the display, or for choosing a color interactively."
\n\t        \t        "Note: It is slow to build this palette, so it should be cached for quick access."
\n\t        \t        "(Color colorTest: 32 extent: 570@180 colorMapper: [:c | startHue palette transHt vSteps transCaption grayWidth hSteps x y c | Color<\/font>
\n\t        \t        "(Color colorTest: 32 extent: 570@180 colorMapper: [:c | c]) display"
\n\t        \t        "(Color colorTest: 32 extent: 570@180 colorMapper:
\n\t        \t        \t        [:c | Color
\n<\/font>\t        \t        \t        \t        r: (c red * 7) asInteger \/ 7
\n\t        \t        \t        \t        g: (c green * 7) asInteger \/ 7
\n\t        \t        \t        \t        b: (c blue * 3) asInteger \/ 3]) display"
\n\t        \t        "(Color colorTest: 32 extent: 570@180 colorMapper:
\n\t        \t        \t        [:c | Color
\n\t        \t        \t        \t        r: (c red * 5) asInteger \/ 5
\n\t        \t        \t        \t        g: (c green * 5) asInteger \/ 5
\n\t        \t        \t        \t        b: (c blue * 5) asInteger \/ 5]) display"
\n\t        \t        "(Color colorTest: 32 extent: 570@180 colorMapper:
\n\t        \t        \t        [:c | Color
\n\t        \t        \t        \t        r: (c red * 15) asInteger \/ 15
\n\t        \t        \t        \t        g: (c green * 15) asInteger \/ 15
\n\t        \t        \t        \t        b: (c blue * 15) asInteger \/ 15]) display"
\n\t        \t        "(Color colorTest: 32 extent: 570@180 colorMapper:
\n\t        \t        \t        [:c | Color
\n\t        \t        \t        \t        r: (c red * 31) asInteger \/ 31
\n\t        \t        \t        \t        g: (c green * 31) asInteger \/ 31
\n\t        \t        \t        \t        b: (c blue * 31) asInteger \/ 31]) display"
\n\t        
\n\t        \t        | basicHue x y c startHue palette transHt vSteps transCaption grayWidth hSteps |
\n\t        \t        …<\/p>\n

Oops! My editor is naive enough to be fooled by code inside comments. We need to search for the method-level temps from the start of the temporaries proper. Luckily the parser has a pair of methods that can be used to do just this. Parser>>parseMethodComment:setPattern: parses the selector and arguments and any initial comments, leaving the parser at the first non-comment token of the method body. Parser>>startOfNextToken answers this token’s source position. So let’s add this as an argument to the editing method. Lets also factor out the tempsToKeep computation for readability.
\n\t        
\nfixBlockLocalTempsInSingleBlockMethods<\/b>
\n\t        <\/font>"Move temporaries declared at method level but used only at block
\n\t         level to block level scope in methods that only contan one block
\n\t         (because this should be easier than dealing with multiple block cases)."<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>candidateMethodsSuchThat:<\/font>
\n\t        \t        \t        [:<\/font>m<\/font>|<\/font> <\/font>|<\/font> <\/font>numBlocks<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>0<\/font>.
\n\t        \t        \t        <\/font>(<\/font>InstructionStream <\/font>on:<\/font> <\/font>m<\/font>)<\/font> <\/font>scanFor:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>b<\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>b<\/font> <\/font>=<\/font> <\/font>143<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font>numBlocks<\/font> <\/font>:=<\/b> <\/font>numBlocks<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>numBlocks<\/font> <\/font>><\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        \t        <\/font>numBlocks<\/font> <\/font>=<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>do:<\/font>\t        [:<\/font>source<\/font> :<\/font>class<\/font> :<\/font>methodNode<\/font> :<\/font>remoteTemps<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToMove<\/font> <\/font>blockNode<\/font> <\/font>methodBodyStart<\/font> <\/font>newSource<\/font> <\/font>|<\/font>
\n\t        \t        \t        Transcript <\/font>cr<\/font>; <\/font>print:<\/font> <\/font>class<\/font>; <\/font>nextPutAll:<\/font> <\/font>‘>>’<\/font><\/font>; <\/font>print:<\/font> <\/font>methodNode<\/font> <\/font>selector<\/font>; <\/font>flush<\/font>.
\n\t        \t        \t        <\/font>tempsToMove<\/font> <\/font>:=<\/b> <\/font>remoteTemps<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>rs<\/font>|<\/font> <\/font>(<\/font>rs<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>and:<\/font> <\/font>[<\/font>(<\/font>t<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNil:<\/font> <\/font>[true]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifNotNil:<\/font> <\/font>[<\/font>:<\/font>ws<\/font>|<\/font> <\/font>(<\/font>ws<\/font> <\/font>includesKey:<\/font> <\/font>methodNode<\/font> <\/font>block<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/font>.
\n\t        \t        \t        <\/font>blockNode<\/font> <\/font>:=<\/b> <\/font>(<\/font>(<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> <\/font>[<\/font>:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>last<\/font>.
\n\t‡       \t        \t        <\/font>methodBodyStart<\/font> <\/font>:=<\/b> <\/font>class<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>parseMethodComment:<\/font> <\/font>source<\/font> <\/font>setPattern:<\/font> <\/font>[<\/font>:<\/font>ignored<\/font>|<\/font>]<\/font>;
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>startOfNextToken<\/font>.
\n\t        \t        \t        <\/font>newSource<\/font> <\/font>:=<\/b> <\/font>self<\/font>\t        <\/font>moveTemps:<\/font> <\/font>tempsToMove<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>in:<\/font> <\/font>methodNode<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>to:<\/font> <\/font>blockNode<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>source:<\/font> <\/font>source<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>methodBodyStart:<\/font> <\/font>methodBodyStart<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        <\/font>encoder:<\/font> <\/font>methodNode<\/font> <\/font>encoder<\/font>.
\n\t        \t        \t        <\/font>class<\/font> <\/font>parserClass<\/font> <\/font>new<\/font> <\/font>parse:<\/font> <\/font>newSource<\/font> <\/font>class:<\/font> <\/font>class<\/font>.
\n\t        \t        \t        <\/font>"(StringHolder new textContents:
\n\t        \t        \t        \t        (TextDiffBuilder buildDisplayPatchFrom: source to: newSource))
\n\t        \t        \t        \t        \t        openLabel: ‘temps edit’."<\/font>]<\/p>\n

<\/font>moveTemps:<\/b> <\/font>tempsToMove<\/font> <\/font>in:<\/b> <\/font>methodNode<\/font> <\/font>to:<\/b> <\/font>blockNode<\/font> <\/font>source:<\/b> <\/font>sourceString<\/font> <\/font>methodBodyStart:<\/b> <\/font>methodBodyStart<\/font> <\/font>encoder:<\/b> <\/font>encoder<\/font>
\n\t        <\/font>^<\/font>String <\/font>streamContents:<\/font>
\n\t        \t        [:<\/font>s<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>startOfBlock<\/font> <\/font>endOfArgs<\/font> <\/font>|<\/font>
\n\t‡       \t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>tempsToKeepAtMethodLevelOf:<\/font> <\/font>methodNode<\/font> <\/font>tempsToMove:<\/font> <\/font>tempsToMove<\/font>.
\n\t        \t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>)<\/font> <\/font>first<\/font>.
\n\t        \t        <\/font>endOfArgs<\/font> <\/font>:=<\/b> <\/font>blockNode<\/font> <\/font>arguments<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>startOfBlock<\/font>]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifFalse:<\/font> <\/font>[<\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>startOfBlock<\/font>]<\/font>.
\n\t        \t        <\/font>tempsStart<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>methodBodyStart<\/font>.
\n\t        \t        <\/font>tempsEnd<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t‡       \t        <\/font>tempsToKeep<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font>;
\n\t        \t        \t        \t         <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font>.
\n\t        \t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>s<\/font> <\/font>space<\/font>; <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/font>]<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>next:<\/font> <\/font>sourceString<\/font> <\/font>size<\/font> <\/font>–<\/font> <\/font>endOfArgs<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/p>\n

<\/font>tempsToKeepAtMethodLevelOf:<\/b> <\/font>methodNode<\/font> <\/font>tempsToMove:<\/b> <\/font>tempsToMove<\/font>
\n\t        <\/font>|<\/font> <\/font>tempsToKeep<\/font> <\/font>|<\/font>
\n\t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> OrderedCollection <\/font>new<\/font>.
\n\t        <\/font>methodNode<\/font> <\/font>block<\/font> <\/font>temporaries<\/font> <\/font>do:<\/font>
\n\t        \t        [:<\/font>t<\/font>|<\/font>
\n\t        \t        <\/font>t<\/font> <\/font>isIndirectTempVector<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>t<\/font> <\/font>remoteTemps<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>rt<\/font>|<\/font>
\n\t        \t        \t        \t        \t         <\/font>(<\/font>rt<\/font> <\/font>scope<\/font> <\/font><<\/font> <\/font>0<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>rt<\/font>]<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>rt<\/font>]<\/font>]<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>(<\/font>t<\/font> <\/font>scope<\/font> <\/font><<\/font> <\/font>0<\/font> <\/font>or:<\/font> <\/font>[<\/font>tempsToMove<\/font> <\/font>includes:<\/font> <\/font>t<\/font>]<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>t<\/font>]<\/font>]<\/font>].
\n\t        <\/font>^<\/font>tempsToKeep<\/font><\/p>\n

Good. Now we can edit and parse 228 methods before an obvious case I should have thought of, that of moving temps into a block that already has block-local temps:<\/p>\n

\t        <\/font>privateAddAllMorphs: aCollection atIndex: index
\n\t        <\/font>\t        "Private. Add aCollection of morphs to the receiver"
\n<\/font>\t        <\/font>\t        | myWorld otherSubmorphs |<\/font>
\n<\/font>\t        <\/font>\t        | myWorld itsWorld otherSubmorphs |<\/font>
\n\t        <\/font>\t        myWorld := self world.
\n\t        <\/font>\t        otherSubmorphs := submorphs copyWithoutAll: aCollection.
\n\t        <\/font>\t        (index between: 0 and: otherSubmorphs size)
\n\t        <\/font>\t        \t        ifFalse: [^ self error: ‘index out of range’].
\n\t        <\/font>\t        index = 0
\n\t        <\/font>\t        \t        ifTrue:[\t        submorphs := aCollection asArray, otherSubmorphs]
\n\t        <\/font>\t        \t        ifFalse:[\t        index = otherSubmorphs size
\n\t        <\/font>\t        \t        \t        ifTrue:[\t        submorphs := otherSubmorphs, aCollection]
\n\t        <\/font>\t        \t        \t        ifFalse:[\t        submorphs := otherSubmorphs copyReplaceFrom: index + 1 to: index with: aCollection ]].
\n<\/font>\t        <\/font>\t        aCollection do: [:m | | itsWorld | | itsOwner |<\/font>
\n<\/font>\t        <\/font>\t        aCollection do: [:m | | itsOwner |<\/font>
\n\t        <\/font>\t        \t        itsOwner := m owner.
\n\t        <\/font>\t        \t        itsOwner ifNotNil: [
\n\t        <\/font>\t        \t        \t        itsWorld := m world.
\n\t        <\/font>\t        \t        \t        (itsWorld == myWorld) ifFalse: [
\n\t        <\/font>\t        \t        \t        \t        itsWorld ifNotNil: [self privateInvalidateMorph: m].
\n\t        <\/font>\t        \t        \t        \t        m outOfWorld: itsWorld].
\n\t        <\/font>\t        \t        \t        (itsOwner ~~ self) ifTrue: [
\n\t        <\/font>\t        \t        \t        \t        m owner privateRemove: m.
\n\t        <\/font>\t        \t        \t        \t        m owner removedMorph: m ]].
\n\t        <\/font>\t        \t        m privateOwner: self.
\n\t        <\/font>\t        \t        myWorld ifNotNil: [self privateInvalidateMorph: m].
\n\t        <\/font>\t        \t        (myWorld == itsWorld) ifFalse: [m intoWorld: myWorld].
\n\t        <\/font>\t        \t        itsOwner == self ifFalse: [
\n\t        <\/font>\t        \t        \t        self addedMorph: m.
\n\t        <\/font>\t        \t        \t        m noteNewOwner: self ].
\n\t        <\/font>\t        ].
\n\t        <\/font>\t        self layoutChanged.
\n\t        
\nObviously we’ve potentially got the same problem finding the start of the block-level temps as finding the start of the method-level temps; a comment could come before the block-level temps and include a | character. But this is really unlikely style so let’s just do the simple thing:<\/p>\n

<\/font>moveTemps:<\/b> <\/font>tempsToMove<\/font> <\/font>in:<\/b> <\/font>methodNode<\/font> <\/font>to:<\/b> <\/font>blockNode<\/font> <\/font>source:<\/b> <\/font>sourceString<\/font> <\/font>methodBodyStart:<\/b> <\/font>methodBodyStart<\/font> <\/font>encoder:<\/b> <\/font>encoder<\/font>
\n\t        <\/font>^<\/font>String <\/font>streamContents:<\/font>
\n\t        \t        [:<\/font>s<\/font>|<\/font> <\/font>|<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>startOfBlock<\/font> <\/font>endOfArgs<\/font> <\/font>restStart<\/font> <\/font>blockTempsStart<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>tempsToKeepAtMethodLevelOf:<\/font> <\/font>methodNode<\/font> <\/font>tempsToMove:<\/font> <\/font>tempsToMove<\/font>.
\n\t        \t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>)<\/font> <\/font>first<\/font>.
\n\t        \t        <\/font>endOfArgs<\/font> <\/font>:=<\/b> <\/font>blockNode<\/font> <\/font>arguments<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>startOfBlock<\/font>]<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>ifFalse:<\/font> <\/font>[<\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>startOfBlock<\/font>]<\/font>.
\n\t        \t        <\/font>tempsStart<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>methodBodyStart<\/font>.
\n\t        \t        <\/font>tempsEnd<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        <\/font>tempsToKeep<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font>;
\n\t        \t        \t        \t         <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font>.
\n\t        \t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>s<\/font> <\/font>space<\/font>; <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>tempsEnd<\/font>]<\/font>.
\n\t‡       \t        <\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t        \t        <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        \t        <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t        \t        <\/font>restStart<\/font> <\/font>:=<\/b> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>blockTempsStart<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t        \t         <\/font>s<\/font> <\/font>next:<\/font> <\/font>blockTempsStart<\/font> <\/font>–<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t        \t         <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>restStart<\/font> <\/font>:=<\/b> <\/font>blockTempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>next:<\/font> <\/font>sourceString<\/font> <\/font>size<\/font> <\/font>–<\/font> <\/font>restStart<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>restStart<\/font>]<\/p>\n

Boom! It now blows up much earlier. Here’s a diff:<\/p>\n

\t        <\/font>nextOrNilSuchThat: aBlock
\n\t        <\/font>\t        "Answer the next object that satisfies aBlock, skipping any intermediate objects.
\n\t        <\/font>\t        If no object has been sent, answer <nil> and leave me intact.
\n\t        <\/font>\t        NOTA BENE: aBlock MUST NOT contain a non-local return (^)."
\n\t        
\n<\/font>\t        <\/font>\t        | value |<\/font>
\n<\/font>\t        <\/font>\t        accessProtect critical: [[<\/font>
\n\t        <\/font>\t        | value readPos |<\/font>
\n\t        <\/font>\t        accessProtect critical: [<\/font>
\n\t        <\/font>\t        \t        value := nil.
\n\t        <\/font>\t        \t        readPos := readPosition.
\n\t        <\/font>\t        \t        [readPos < writePosition and: [value isNil]] whileTrue: [
\n\t        <\/font>\t        \t        \t        value := contentsArray at: readPos.
\n\t        <\/font>\t        \t        \t        readPos := readPos + 1.
\n\t        <\/font>\t        \t        \t        (aBlock value: value) ifTrue: [
\n<\/font>\t        <\/font>\t        \t        \t        \t        readPosition to: readPos – 1 do: [ :j | readPos<\/font>
\n<\/font>\t        <\/font>\t        \t        \t        \t        readPosition to: readPos – 1 do: [ :j |<\/font>
\n\t        <\/font>\t        \t        \t        \t        \t        contentsArray at: j put: nil.
\n\t        <\/font>\t        \t        \t        \t        ].
\n\t        <\/font>\t        \t        \t        \t        readPosition := readPos.
\n\t        <\/font>\t        \t        \t        ] ifFalse: [
\n\t        <\/font>\t        \t        \t        \t        value := nil.
\n\t        <\/font>\t        \t        \t        ].
\n\t        <\/font>\t        \t        ].
\n\t        <\/font>\t        \t        readPosition >= writePosition ifTrue: [readSynch initSignals].
\n\t        <\/font>\t        ].
\n\t        <\/font>\t        ^value<\/p>\n

because the critical: block only has implicit temporaries (the i in <\/font>readPosition to: readPos – 1 do: [ :j |…]). We could try filtering-out implicit temps as in
\n<\/font>
\n\t        \t        <\/font>(<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>select:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>t<\/font> <\/font>scope<\/font> <\/font>>=<\/font> <\/font>0<\/font>]<\/font>)<\/font> <\/font>isEmpty<\/font><\/p>\n

but this won’t work for a couple of reasons. One, the compiler marks block temporaries as out-of-scope once a block has been parsed so all block temps are going to have their scopes less than zero in any parse tree. Two, there could be a temporary declaration in an optimized block nested within it and the start of those nested temporaries won’t be anywhere near the right place. We need to search for the block’s temporaries immediately following the arguments:<\/font>
\n
\n\t        \t        <\/font>maybeBlockTempsStart<\/font> <\/font>:=<\/b> <\/font>sourceString<\/font> <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>ifAbsent:<\/font> <\/font>sourceString<\/font> <\/font>size<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        <\/font>(<\/font>(<\/font>sourceString<\/font> <\/font>copyFrom:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>to:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font>)<\/font> <\/font>allSatisfy:<\/font>
\n\t        \t        \t        <\/font>[<\/font>:<\/font>c<\/font>|<\/font> <\/font>c<\/font> <\/font>isSeparator<\/font>]<\/font>)<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>next:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>–<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t        \t         <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>restStart<\/font> <\/font>:=<\/b> <\/font>maybeBlockTempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t        \t        <\/font>tempsToMove<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        \t        <\/font>s<\/font> <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t        \t        <\/font>restStart<\/font> <\/font>:=<\/b> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>.
\n\t        \t        <\/font>s<\/font> <\/font>next:<\/font> <\/font>sourceString<\/font> <\/font>size<\/font> <\/font>–<\/font> <\/font>restStart<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> <\/font>sourceString<\/font> <\/font>startingAt:<\/font> <\/font>restStart<\/font>]<\/p>\n

and lo and behold all 645 odd methods are edited and parse correctly. Arguably we’ve got by on a wing and a prayer, but that’s at least part of the XP way. Do the simplest thing that could possibly work. I haven’t been able to demonstrate how quickly this can be done because I’ve been writing this as I’ve gone along, and have had a couple of unexpected stumbles along the way, so it has been far from fleet. But I hope I’ve demonstrated how exploratory programming has got us to a solution quite easily, the debugger being a great tool to use to understand a problem. Had we started off with BUFD I might have been lazy or intimidated and given up; after all the code still works without the editing. But using the environment we’ve come up with a solution and it’s been fun (at least for me). But we’re not done yet. We need to generalize to methods with more than one block and we need to check for read-before-written temps before this is a tool we an use to eliminate unnecessary remote temps and proceed to take some meaningful measurements.<\/p>\n

A More Comprehensive Implementation<\/H2>
\nNow we understand the problem enough that we can do some up-front design. To generalise we need to move temporaries to their correct block scope. Handling multiple blocks doesn’t change the fact that we’re moving temps so the start of the process, deleting the temps from method-level scope, is unchanged. Further, since block declarations follow each other, if we iterate through blocks in source order our cheap and cheerful editing process can be extended. We need to maintain the position in the source we’ve got to so far and output from this point up through the current block’s temporaries.<\/p>\n

Let’s make the editor deal with a single method at a time. We can get all of the information we need to compile from the method. We need an output stream on which to write the modifed source, a map of some sort indicating which temps to move to which block and we need the block nodes in the method. In extending the source editing approach we’ll need to know where we are in the input source so far. We also need the encoder since source ranges for blocks are obtained from it.<\/p>\n

Object <\/font>subclass:<\/font> <\/font>#TempScopeEditor<\/font>
\n\t        <\/font>instanceVariableNames:<\/font> <\/font>‘method out tempMap blockNodes sourceString soFar encoder’<\/font><\/font>
\n\t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>category:<\/font> <\/font>‘Cog-Scripts’<\/p>\n

<\/font><\/font>TempScopeEditor methods for initialize-release<\/i><\/font>
\n<\/font><\/font>forMethod:<\/b> <\/font>aCompiledMethod<\/font>
\n\t        <\/font>|<\/font> <\/font>methodNode<\/font> <\/font>|<\/font>
\n\t        method <\/font>:=<\/b> <\/font>aCompiledMethod<\/font>.
\n\t        sourceString <\/font>:=<\/b> <\/font>aCompiledMethod<\/font> <\/font>getSourceFromFile<\/font> <\/font>asString<\/font>.
\n\t        <\/font>methodNode<\/font> <\/font>:=<\/b> method <\/font>methodClass<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>parse:<\/font> sourceString
\n\t        \t        \t        \t        \t        \t        <\/font>class:<\/font> method <\/font>methodClass<\/font>.
\n\t        <\/font>methodNode<\/font> <\/font>ensureClosureAnalysisDone<\/font>.
\n\t        encoder <\/font>:=<\/b> <\/font>methodNode<\/font> <\/font>encoder<\/font>.
\n\t        blockNodes <\/font>:=<\/b> (<\/font>methodNode<\/font> <\/font>accept:<\/font> BlockNodeCollectingVisitor <\/font>new<\/font>)
\n\t        \t        \t        \t        \t        \t        <\/font>blockNodes<\/font> <\/font>reject:<\/font> [:<\/font>bn<\/font>|<\/font> <\/font>bn<\/font> <\/font>optimized<\/font>].
\n\t        out <\/font>:=<\/b> (String <\/font>new:<\/font> sourceString <\/font>size<\/font>) <\/font>writeStream<\/font>.
\n\t        tempMap <\/font>:=<\/b> IdentityDictionary <\/font>new<\/font><\/p>\n

We need to edit the method. That’ll be the main entry point. We need to construct the temp map and then edit it. I’ll end with a halt for now, to check the output.<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>edit<\/b>
\n\t        <\/font>self<\/font> <\/font>buildTempMap<\/font>.
\n\t        <\/font>self<\/font> <\/font>copyMethodMovingTemps<\/font>.
\n\t        <\/font>self<\/font> <\/font>halt<\/font><\/p>\n

Since visitors visit the parse nodes in order of evaluation we can find the smallest enclosing scope by choosing the last block that fully encloses a temp.<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>buildTempMap<\/b>
\n\t        <\/font>"Build the map for moving remote temps. Each remote temp
\n\t         that should be moved is entered into the map referencing its
\n\t         smallest enclosing scope. This may seem backwards but it
\n\t         means that the map is one-to-one, not one-to-many."<\/font>
\n\t        blockNodes <\/font>do:<\/font>
\n\t        \t        [:<\/font>blockNode<\/font>|<\/font>
\n\t        \t        <\/font>(<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>notEmpty<\/font>
\n\t        \t         <\/font>and:<\/font> <\/font>[<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>last<\/font> <\/font>isIndirectTempVector<\/font>]<\/font>)<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        <\/font>[<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>last<\/font> <\/font>remoteTemps<\/font> <\/font>do:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>remoteTemp<\/font>|<\/font> <\/font>|<\/font> <\/font>enclosingScopes<\/font> <\/font>smallestEnclosingBlockScope<\/font> <\/font>|<\/font>
\n\t        \t        \t        \t         <\/font>enclosingScopes<\/font> <\/font>:=<\/b> blockNodes <\/font>select:<\/font> <\/font>[<\/font>:<\/font>blockScope<\/font>|<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>self<\/font> <\/font>blockNode:<\/font> <\/font>blockScope<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>isEnclosingScopeFor:<\/font> <\/font>remoteTemp<\/font>]<\/font>.
\n\t        \t        \t        \t         <\/font>enclosingScopes<\/font> <\/font>notEmpty<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>smallestEnclosingBlockScope<\/font> <\/font>:=<\/b> <\/font>enclosingScopes<\/font> <\/font>last<\/font>.
\n\t        \t        \t        \t        \t         <\/font>smallestEnclosingBlockScope<\/font> <\/font>~~<\/font> <\/font>blockNode<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t        \t         <\/font>[<\/font>tempMap <\/font>at:<\/font> <\/font>remoteTemp<\/font> <\/font>put:<\/font> <\/font>smallestEnclosingBlockScope<\/font>]<\/font>]<\/font>]<\/font>]<\/font>]<\/p>\n

Finding out whether a block is a valid scope is easy if we find the converse. Reject temps that have any references outside of a given block.<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>blockNode:<\/b> <\/font>aBlockNode<\/font> <\/font>isEnclosingScopeFor:<\/b> <\/font>aTempVariableNode<\/font>
\n\t        <\/font>^<\/font>(<\/font>(<\/font>self<\/font>
\n\t        \t        <\/font>anyScopes:<\/font> <\/font>(<\/font>aTempVariableNode<\/font> <\/font>instVarNamed:<\/font> <\/font>‘readingScopes’<\/font><\/font>)<\/font>
\n\t        \t        <\/font>outsideExtent:<\/font> <\/font>aBlockNode<\/font> <\/font>blockExtent<\/font>)<\/font>
\n\t        <\/font>or:<\/font> <\/font>[<\/font>self<\/font>
\n\t        \t        <\/font>anyScopes:<\/font> <\/font>(<\/font>aTempVariableNode<\/font> <\/font>instVarNamed:<\/font> <\/font>‘writingScopes’<\/font><\/font>)<\/font>
\n\t        \t        <\/font>outsideExtent:<\/font> <\/font>aBlockNode<\/font> <\/font>blockExtent<\/font>]<\/font>) <\/font>not<\/p>\n

<\/font>anyScopes:<\/b> <\/font>referenceScopeDict<\/font> <\/font>outsideExtent:<\/b> <\/font>blockExtent<\/font>
\n\t        <\/font>^<\/font>referenceScopeDict<\/font> <\/font>notEmpty<\/font>
\n\t         <\/font>and:<\/font> [<\/font>referenceScopeDict<\/font> <\/font>anySatisfy:<\/font>
\n\t        \t        \t        <\/font>[<\/font>:<\/font>set<\/font>|<\/font>
\n\t        \t        \t        <\/font>set<\/font> <\/font>anySatisfy:<\/font> <\/font>[<\/font>:<\/font>location<\/font>|<\/font> <\/font>(<\/font>blockExtent<\/font> <\/font>rangeIncludes:<\/font> <\/font>location<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/p>\n

If we don’t find any temps to move the method doesn’t need editing. So our edit method should be more like<\/p>\n

<\/font>TempScopeEditor methods for editing
\n<\/i><\/font>edit<\/b>
\n\t        <\/font>self<\/font> <\/font>buildTempMap<\/font>.
\n\t        tempMap <\/font>isEmpty<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        [<\/font>self<\/font> <\/font>copyMethodMovingTemps<\/font>.
\n\t        \t         <\/font>self<\/font> <\/font>halt<\/font>.
\n\t        \t         method <\/font>methodClass<\/font> <\/font>compile:<\/font> out <\/font>contents<\/font> <\/font>classified:<\/font> method <\/font>methodReference<\/font> <\/font>category<\/font>]<\/p>\n

Now we can adapt the old code. We need to enumerate over all blocks except the first one, which is actually a block node for the entire method.<\/p>\n

<\/font>TempScopeEditor methods for editing
\n<\/i><\/font>copyMethodMovingTemps<\/b>
\n\t        <\/font>|<\/font> <\/font>methodBodyStart<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>|<\/font>
\n\t        <\/font>methodBodyStart<\/font> <\/font>:=<\/b> method <\/font>methodClass<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>parseMethodComment:<\/font> sourceString <\/font>setPattern:<\/font> [:<\/font>ignored<\/font>|<\/font>];
\n\t        \t        \t        \t        \t        \t        \t        <\/font>startOfNextToken<\/font>.
\n\t        (<\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>tempsToKeepAtMethodLevel<\/font>) <\/font>isEmpty<\/font>
\n\t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        [soFar <\/font>:=<\/b> <\/font>1<\/font>]
\n\t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        [<\/font>tempsStart<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>methodBodyStart<\/font>.
\n\t        \t        \t         <\/font>tempsEnd<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t         out <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>putAll:<\/font> sourceString.
\n\t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> <\/font>tempsEnd<\/font>].
\n\t        blockNodes <\/font>allButFirst<\/font> <\/font>do:<\/font>
\n\t        \t        [:<\/font>blockNode<\/font>|<\/font>
\n\t        \t        <\/font>self<\/font> <\/font>processBlockNode:<\/font> <\/font>blockNode<\/font>].
\n\t        out <\/font>next:<\/font> sourceString <\/font>size<\/font> <\/font>–<\/font> soFar <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> soFar<\/p>\n

Since the first block is that of the entire method we can compute the set of method-level temps to keep from it. But we need to compute temps to keep for each block node so we can factor-out. But there’s a wrinkle. The compiler marks all block temps as out of scope on leaving the block but not all method-level scopes. Hence<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>tempsToKeepAtMethodLevel<\/b>
\n\t        <\/font>^<\/font>(<\/font>self<\/font> <\/font>tempsToKeepFor:<\/font> blockNodes <\/font>first<\/font>) <\/font>select:<\/font>
\n\t        \t        [:<\/font>t<\/font>|<\/font>t<\/font> <\/font>scope<\/font> <\/font>>=<\/font> <\/font>0<\/font>]<\/p>\n

<\/font>tempsToKeepFor:<\/b> <\/font>blockNode<\/font>
\n\t        <\/font>|<\/font> <\/font>tempsToKeep<\/font> <\/font>|<\/font>
\n\t        <\/font>tempsToKeep<\/font> <\/font>:=<\/b> OrderedCollection <\/font>new<\/font>.
\n\t        <\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>do:<\/font>
\n\t        \t        [:<\/font>t<\/font>|<\/font>
\n\t        \t        <\/font>t<\/font> <\/font>isIndirectTempVector<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>t<\/font> <\/font>remoteTemps<\/font> <\/font>do:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>:<\/font>rt<\/font>|<\/font>
\n\t        \t        \t        \t        \t        <\/font>(<\/font>tempMap <\/font>includesKey:<\/font> <\/font>rt<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>rt<\/font>]<\/font>]<\/font>]<\/font>
\n\t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>tempsToKeep<\/font> <\/font>addLast:<\/font> <\/font>t<\/font>]<\/font>].
\n\t        <\/font>^<\/font>tempsToKeep<\/font><\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>processBlockNode:<\/b> <\/font>blockNode<\/font>
\n\t        <\/font>|<\/font> <\/font>tempsToMoveHere<\/font> <\/font>startOfBlock<\/font> <\/font>endOfArgs<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>blockTempsInSource<\/font> <\/font>|<\/font>
\n\t        <\/font>tempsToMoveHere<\/font> <\/font>:=<\/b> (tempMap <\/font>select:<\/font> <\/font>[<\/font>:<\/font>aBlockNode<\/font>|<\/font> <\/font>aBlockNode<\/font> <\/font>==<\/font> <\/font>blockNode<\/font>]<\/font>) <\/font>keys<\/font>.
\n\t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> (encoder <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>) <\/font>first<\/font>.
\n\t        <\/font>endOfArgs<\/font> <\/font>:=<\/b> <\/font>blockNode<\/font> <\/font>arguments<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifTrue:<\/font> [<\/font>startOfBlock<\/font>]
\n\t        \t        \t        \t        \t        <\/font>ifFalse:<\/font> [sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>startOfBlock<\/font>].
\n\t        out <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> soFar <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> soFar.
\n\t        <\/font>maybeBlockTempsStart<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>ifAbsent:<\/font> sourceString <\/font>size<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        <\/font>blockTempsInSource<\/font> <\/font>:=<\/b> (sourceString <\/font>copyFrom:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>to:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font>) <\/font>allSatisfy:<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        [:<\/font>c<\/font>|<\/font> <\/font>c<\/font> <\/font>isSeparator<\/font>].
\n\t        <\/font>blockTempsInSource<\/font>
\n\t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        [out <\/font>next:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>–<\/font> <\/font>endOfArgs<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t         <\/font>(<\/font>self<\/font> <\/font>tempsToKeepFor:<\/font> <\/font>blockNode<\/font>)<\/font> <\/font>do:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>tempNode<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>tempNode<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         <\/font>tempsToMoveHere<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t         <\/font>(<\/font>sourceString <\/font>at:<\/font> soFar <\/font>–<\/font> <\/font>1<\/font>)<\/font> <\/font>isSeparator<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>soFar <\/font>:=<\/b> soFar <\/font>–<\/font> <\/font>1<\/font>]<\/font>]
\n\t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        [out <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t        <\/font>tempsToMoveHere<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t        out <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t        soFar <\/font>:=<\/b> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/p>\n

Finding an interesting method to test, one that has an indirect temp in a block that needs moving let’s test it:<\/p>\n

\t        <\/font>"Find a block scope with a temp that needs moving."
\n\t        <\/font>| scanner |
\n\t        <\/font>scanner := InstructionStream new.
\n\t        <\/font>SystemNavigation default browseAllSelect:
\n\t        <\/font>\t        [:m| | seenBlock |
\n\t        <\/font>\t        seenBlock := false.
\n\t        <\/font>\t        m isQuick not
\n\t        <\/font>\t        and: [scanner method: m pc: m initialPC.
\n\t        <\/font>\t        \t        scanner scanFor:
\n\t        <\/font>\t        \t        \t        [:b|
\n\t        <\/font>\t        \t        \t        b = 143 ifTrue: [seenBlock := true].
\n\t        <\/font>\t        \t        \t        seenBlock and: [b = 138 and: [scanner followingByte <= 127]]]]]<\/p>\n

which throws up SARInstaller class>>ensurePackageWithId:<\/p>\n

ensurePackageWithId:<\/b> <\/font>anIdString<\/font><\/p>\n

\t        <\/font>self<\/font> <\/font>squeakMapDo:<\/font> [ :<\/font>sm<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>card<\/font> <\/font>newCS<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>self<\/font> <\/font>withCurrentChangeSetNamed:<\/font> <\/font>‘updates’<\/font><\/font> <\/font>do:<\/font> <\/font>[<\/font> :<\/font>cs<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>newCS<\/font> <\/font>:=<\/b> <\/font>cs<\/font>.
\n\t        \t        \t        <\/font>card<\/font> <\/font>:=<\/b> <\/font>sm<\/font> <\/font>cardWithId:<\/font> <\/font>anIdString<\/font>.
\n\t        \t        \t        <\/font>(<\/font>card<\/font> <\/font>isNil<\/font> <\/font>or:<\/font> <\/font>[<\/font> <\/font>card<\/font> <\/font>isInstalled<\/font> <\/font>not<\/font> <\/font>or:<\/font> <\/font>[<\/font> <\/font>card<\/font> <\/font>isOld<\/font> <\/font>]<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font> <\/font>sm<\/font> <\/font>installPackageWithId:<\/font> <\/font>anIdString<\/font> <\/font>]<\/font>
\n\t        \t        <\/font>]<\/font>.
\n\t        \t        <\/font>newCS<\/font> <\/font>isEmpty<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font> ChangeSet <\/font>removeChangeSet:<\/font> <\/font>newCS<\/font> <\/font>]<\/font>
\n\t        ].<\/p>\n

Editing it works nicely:
\n\t        <\/font>ensurePackageWithId: anIdString
\n\t        
\n\t        <\/font>\t        self squeakMapDo: [ :sm | | card newCS |<\/font>
\n\t        <\/font>\t        \t        self withCurrentChangeSetNamed: ‘updates’ do: [ :cs |<\/font>
\n\t        <\/font>\t        self squeakMapDo: [ :sm | | newCS |<\/font>
\n\t        <\/font>\t        \t        self withCurrentChangeSetNamed: ‘updates’ do: [ :cs | | card |<\/font>
\n\t        <\/font>\t        \t        \t        newCS := cs.
\n\t        <\/font>\t        \t        \t        card := sm cardWithId: anIdString.
\n\t        <\/font>\t        \t        \t        (card isNil or: [ card isInstalled not or: [ card isOld ]])
\n\t        <\/font>\t        \t        \t        \t        ifTrue: [ sm installPackageWithId: anIdString ]
\n\t        <\/font>\t        \t        ].
\n\t        <\/font>\t        \t        newCS isEmpty ifTrue: [ ChangeSet removeChangeSet: newCS ]
\n\t        <\/font>\t        ].<\/p>\n

To this we need to add a read-before-written checker and we should be able to fix all methods in the system (rather than all conceivable methods). We can implement another simple visitor:<\/p>\n

<\/font>ParseNodeVisitor <\/font>subclass:<\/font> <\/font>#ReadBeforeWrittenVisitor<\/font>
\n\t        <\/font>instanceVariableNames:<\/font> <\/font>‘readBeforeWritten written’<\/font><\/font>
\n\t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>category:<\/font> <\/font>‘Cog-Scripts’<\/font><\/font><\/p>\n

<\/font>ReadBeforeWrittenVisitor<\/i><\/font> methods for accessing<\/i><\/font>
\n<\/font>readBeforeWritten<\/b>
\n\t        <\/font>^<\/font>readBeforeWritten <\/font>ifNil:<\/font> [IdentitySet <\/font>new<\/font>]<\/p>\n

<\/font>ReadBeforeWrittenParseNodeVisitor<\/i><\/font> methods for visiting
\n<\/i><\/font>visitAssignmentNode:<\/b> <\/font>anAssignmentNode<\/font>
\n\t        <\/font>anAssignmentNode<\/font> <\/font>variable<\/font> <\/font>isTemp<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        [written <\/font>ifNil:<\/font> <\/font>[<\/font>written <\/font>:=<\/b> IdentitySet <\/font>new<\/font>]<\/font>.
\n\t        \t        written <\/font>add:<\/font> <\/font>anAssignmentNode<\/font> <\/font>variable<\/font>]<\/p>\n

<\/font>visitTempVariableNode:<\/b> <\/font>aTempVariableNode<\/font>
\n\t        (<\/font>aTempVariableNode<\/font> <\/font>isArg<\/font>
\n\t         <\/font>or:<\/font> <\/font>[<\/font>written <\/font>notNil<\/font>
\n\t        \t        <\/font>and:<\/font> <\/font>[<\/font>written <\/font>includes:<\/font> <\/font>aTempVariableNode<\/font>]<\/font>]<\/font>) <\/font>ifTrue:<\/font>
\n\t        \t        [<\/font>^self<\/font>].
\n\t        readBeforeWritten <\/font>ifNil:<\/font>
\n\t        \t        [readBeforeWritten <\/font>:=<\/b> IdentitySet <\/font>new<\/font>].
\n\t        readBeforeWritten <\/font>add:<\/font> <\/font>aTempVariableNode<\/font><\/p>\n

and let’s test it.
\n\t        <\/font>| mc |
\n\t        <\/font>mc := #none.
\n\t        <\/font>SystemNavigation default browseAllSelect:
\n\t        <\/font>\t        [:m|
\n\t        <\/font>\t        m methodClass ~~ mc ifTrue:
\n\t        <\/font>\t        \t        [Transcript cr; print: (mc := m methodClass); flush].
\n\t        <\/font>\t        m isQuick not
\n\t        <\/font>\t        and: [(m methodNode accept: ReadBeforeWrittenVisitor new) readBeforeWritten notEmpty]]<\/p>\n

Let’s see what the editor does before adding the read-before-written check:
\n
\n<\/font>TempScopeEditor methods for editing
\n<\/i><\/font>editNoCompile<\/b>
\n\t        <\/font>self<\/font> <\/font>buildTempMap<\/font>.
\n\t        <\/font>^<\/font>tempMap <\/font>isEmpty<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        [<\/font>self<\/font> <\/font>copyMethodMovingTemps<\/font>.
\n\t        \t         out <\/font>contents<\/font>]<\/p>\n

and test it on something that has a read-before-written temp:
\n<\/font>
\n\t        (TempScopeEditor new forMethod: Collection >> #detectMax:) editNoCompile<\/p>\n

produces<\/p>\n

\t        ‘detectMax: aBlock
\n\t        \t        "Evaluate aBlock with each of the receiver”s elements as the argument.
\n\t        \t        Answer the element for which aBlock evaluates to the highest magnitude.
\n\t        \t        If collection empty, return nil. This method might also be called elect:."
\n\t        
\n\t        \t        | maxElement|
\n\t        \t        self do: [:each | | maxValue val |
\n\t        \t        \t        maxValue == nil
\n\t        \t        \t        \t        ifFalse: [
\n\t        \t        \t        \t        \t        (val := aBlock value: each) > maxValue ifTrue: [
\n\t        \t        \t        \t        \t        \t        maxElement := each.
\n\t        \t        \t        \t        \t        \t        maxValue := val]]
\n\t        \t        \t        \t        ifTrue: ["first element"
\n\t        \t        \t        \t        \t        maxElement := each.
\n\t        \t        \t        \t        \t        maxValue := aBlock value: each].
\n\t        \t        \t        \t        \t        "Note that there is no way to get the first element that works
\n\t        \t        \t        \t        \t        for all kinds of Collections. Must test every one."].
\n\t        \t        ^ maxElement’<\/p>\n

Indeed, maxValue is incorrectly moved into the inner scope. But there’s also a minor bug with the missing space before the closing vertical bar. Let’s fix that.
\n
\n<\/font>TempScopeEditor methods for editing
\n<\/i><\/font>copyMethodMovingTemps<\/b>
\n\t        <\/font>|<\/font> <\/font>methodBodyStart<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>|<\/font>
\n\t        <\/font>methodBodyStart<\/font> <\/font>:=<\/b> method <\/font>methodClass<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>parseMethodComment:<\/font> sourceString <\/font>setPattern:<\/font> [:<\/font>ignored<\/font>|<\/font>];
\n\t        \t        \t        \t        \t        \t        \t        <\/font>startOfNextToken<\/font>.
\n\t        (<\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>tempsToKeepAtMethodLevel<\/font>) <\/font>isEmpty<\/font>
\n\t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        [soFar <\/font>:=<\/b> <\/font>1<\/font>]
\n\t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        [<\/font>tempsStart<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>methodBodyStart<\/font>.
\n\t        \t        \t         <\/font>tempsEnd<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t         out <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>putAll:<\/font> sourceString.
\n\t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> <\/font>tempsEnd<\/font>.
\n\t‡       \t        \t         <\/font>(<\/font>sourceString <\/font>at:<\/font> soFar <\/font>–<\/font> <\/font>1<\/font>)<\/font> <\/font>isSeparator<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>soFar <\/font>:=<\/b> soFar <\/font>–<\/font> <\/font>1<\/font>]<\/font>].
\n\t        blockNodes <\/font>allButFirst<\/font> <\/font>do:<\/font>
\n\t        \t        [:<\/font>blockNode<\/font>|<\/font>
\n\t        \t        <\/font>self<\/font> <\/font>processBlockNode:<\/font> <\/font>blockNode<\/font>].
\n\t        out <\/font>next:<\/font> sourceString <\/font>size<\/font> <\/font>–<\/font> soFar <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> soFar<\/p>\n

To add the read-before-written check I need to make sure the visitor visits the same parse tree we derive the block nodes from so that the visitor answers temp nodes in the block node’s tree. I need to make the method node an instance variable. I can derive the encoder from the method node, so…<\/p>\n

Object <\/font>subclass:<\/font> <\/font>#TempScopeEditor<\/font>
\n\t        <\/font>instanceVariableNames:<\/font> <\/font>‘method methodNode out tempMap blockNodes sourceString soFar’<\/font><\/font>
\n\t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        <\/font>category:<\/font> <\/font>‘Cog-Scripts’<\/font><\/font><\/p>\n

<\/font>TempScopeEditor methods for editing
\n<\/i><\/font>buildTempMap<\/b>
\n\t        <\/font>"Build the map for moving remote temps. Each remote temp
\n\t         that should be moved is entered into the map referencing its
\n\t         smallest enclosing scope. This may seem backwards but it
\n\t         means that the map is one-to-one, not one-to-many."<\/font>
\n\t        <\/font>|<\/font> <\/font>readBeforeWritten<\/font> <\/font>|<\/font>
\n\t‡       <\/font>readBeforeWritten<\/font> <\/font>:=<\/b> (methodNode <\/font>accept:<\/font> ReadBeforeWrittenVisitor <\/font>new<\/font>) <\/font>readBeforeWritten<\/font>.
\n\t        blockNodes <\/font>do:<\/font>
\n\t        \t        [:<\/font>blockNode<\/font>|<\/font>
\n\t        \t        <\/font>(<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>notEmpty<\/font>
\n\t        \t         <\/font>and:<\/font> <\/font>[<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>last<\/font> <\/font>isIndirectTempVector<\/font>]<\/font>)<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        <\/font>[<\/font>blockNode<\/font> <\/font>temporaries<\/font> <\/font>last<\/font> <\/font>remoteTemps<\/font> <\/font>do:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>remoteTemp<\/font>|<\/font> <\/font>|<\/font> <\/font>enclosingScopes<\/font> <\/font>smallestEnclosingBlockScope<\/font> <\/font>|<\/font>
\n\t‡       \t        \t        \t         <\/font>(<\/font>readBeforeWritten<\/font> <\/font>includes:<\/font> <\/font>remoteTemp<\/font>)<\/font> <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t         \t        <\/font>[<\/font>enclosingScopes<\/font> <\/font>:=<\/b> blockNodes <\/font>select:<\/font> <\/font>[<\/font>:<\/font>blockScope<\/font>|<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>self<\/font> <\/font>blockNode:<\/font> <\/font>blockScope<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        <\/font>isEnclosingScopeFor:<\/font> <\/font>remoteTemp<\/font>]<\/font>.
\n\t        \t        \t        \t        \t         <\/font>enclosingScopes<\/font> <\/font>notEmpty<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t        \t        <\/font>[<\/font>smallestEnclosingBlockScope<\/font> <\/font>:=<\/b> <\/font>enclosingScopes<\/font> <\/font>last<\/font>.
\n\t        \t        \t        \t        \t        \t         <\/font>smallestEnclosingBlockScope<\/font> <\/font>~~<\/font> <\/font>blockNode<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t        \t        \t         <\/font>[<\/font>tempMap <\/font>at:<\/font> <\/font>remoteTemp<\/font> <\/font>put:<\/font> <\/font>smallestEnclosingBlockScope<\/font>]<\/font>]<\/font>]<\/font>]<\/font>]<\/font>]<\/p>\n

and now
\n<\/font>
\n\t        (TempScopeEditor new forMethod: Collection >> #detectMax:) editNoCompile<\/p>\n

produces<\/p>\n

\t        ‘detectMax: aBlock
\n\t        \t        "Evaluate aBlock with each of the receiver”s elements as the argument.
\n\t        \t        Answer the element for which aBlock evaluates to the highest magnitude.
\n\t        \t        If collection empty, return nil. This method might also be called elect:."
\n\t        
\n\t        \t        | maxElement maxValue |
\n\t        \t        self do: [:each | | val |
\n\t        \t        \t        maxValue == nil
\n\t        \t        \t        \t        ifFalse: [
\n\t        \t        \t        \t        \t        (val := aBlock value: each) > maxValue ifTrue: [
\n\t        \t        \t        \t        \t        \t        maxElement := each.
\n\t        \t        \t        \t        \t        \t        maxValue := val]]
\n\t        \t        \t        \t        ifTrue: ["first element"
\n\t        \t        \t        \t        \t        maxElement := each.
\n\t        \t        \t        \t        \t        maxValue := aBlock value: each].
\n\t        \t        \t        \t        \t        "Note that there is no way to get the first element that works
\n\t        \t        \t        \t        \t        for all kinds of Collections. Must test every one."].
\n\t        \t        ^ maxElement’<\/p>\n

Measurements<\/H2><\/p>\n

Good. I hope we’re done. Let’s count and see the change:<\/p>\n

\t        | numMethods numMethodsWithClosure numMethodsWithIndirectTemps
\n\t         numClosures numClosuresWithCopiedValues numCopiedValues
\n\t         numRemoteTemps numScopesWithRemoteTemps |
\n\t        numMethods := numMethodsWithClosure := numMethodsWithIndirectTemps :=
\n\t        numClosures := numClosuresWithCopiedValues := numCopiedValues :=
\n\t        numRemoteTemps := numScopesWithRemoteTemps := 0.
\n\t        SystemNavigation default allSelect:
\n\t        \t        [:m| | s hasBlock hasIndirectTemps |
\n\t        \t        hasBlock := hasIndirectTemps := false.
\n\t        \t        s := InstructionStream on: m.
\n\t        \t        s scanFor:
\n\t        \t        \t        [:b|
\n\t        \t        \t        b = 143 "closure creation" ifTrue:
\n\t        \t        \t        \t        [hasBlock := true.
\n\t        \t        \t        \t        numClosures := numClosures + 1.
\n\t        \t        \t        \t        s followingByte >= 16 ifTrue:
\n\t        \t        \t        \t        \t        [numClosuresWithCopiedValues := numClosuresWithCopiedValues + 1.
\n\t        \t        \t        \t        \t         numCopiedValues := numCopiedValues + (s followingByte >> 4)]].
\n\t        \t        \t        (b = 138 "indirect temp vector creation"
\n\t        \t        \t         and: [s followingByte <= 127]) ifTrue:
\n\t        \t        \t        \t        [hasIndirectTemps := true.
\n\t        \t        \t        \t         numScopesWithRemoteTemps := numScopesWithRemoteTemps + 1.
\n\t        \t        \t        \t         numRemoteTemps := numRemoteTemps + s followingByte].
\n\t        \t        \t        false].
\n\t        \t        numMethods := numMethods + 1.
\n\t        \t        hasBlock ifTrue: [numMethodsWithClosure := numMethodsWithClosure + 1].
\n\t        \t        hasIndirectTemps ifTrue: [numMethodsWithIndirectTemps := numMethodsWithIndirectTemps + 1].
\n\t        \t        false].
\n\t        { numMethods. numMethodsWithClosure. numMethodsWithIndirectTemps.
\n\t         numClosures. numCopiedValues. numClosuresWithCopiedValues.
\n\t         numRemoteTemps. numScopesWithRemoteTemps }
\ngives<\/p>\n

\t         #(67701 10966 2430 18380 14843 9249 4828 2436)<\/p>\n

So 10966 \/ 677.01 = 16.2% of methods contain a closure. Prior to editing 2430 \/ 109.66 = 22.2% of these methods need indirect temps.<\/p>\n

Now let’s edit. We only need look at methods which create a closure:<\/p>\n

\t        SystemNavigation default allSelect:
\n\t        \t        [:m| | s |
\n\t        \t        s := InstructionStream on: m.
\n\t        \t        (s scanFor:
\n\t        \t        \t        \t        [:b|
\n\t        \t        \t        \t         b = 143 "closure creation"
\n\t        \t        \t        \t         and: [s followingByte >= 16]]) ifTrue:
\n\t        \t        \t        [(TempScopeEditor new forMethod: m) edit].
\n\t        \t        false]<\/p>\n

Boom! SyntaxError for Object>>get:<\/p>\n

\t        get:<\/b> <\/font>fieldName<\/font>
\n\t        \t        <\/font>|<\/font> <\/font>var<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>^self<\/font> <\/font>instVarAt:<\/font> (<\/font>self<\/font> <\/font>class<\/font> <\/font>allInstVarNames<\/font> <\/font>indexOf:<\/font> <\/font>fieldName<\/font> <\/font>ifAbsent:<\/font>[<\/font> <\/font>|<\/font> <\/font>Name<\/font> <\/font>is<\/font> <\/font>already<\/font> <\/font>defined<\/font> ->var |<\/font>
\n\t        \t        \t        ^(var := self class bindingOf: fieldName) ifNotNil:[var value].<\/font>
\n\t        \t        ])<\/font><\/p>\n

I’m forgetting to remove temporaries if all method-level temps are moved. So<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>copyMethodMovingTemps<\/b>
\n\t        <\/font>|<\/font> <\/font>methodBodyStart<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>|<\/font>
\n\t        <\/font>methodBodyStart<\/font> <\/font>:=<\/b> method <\/font>methodClass<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>parseMethodComment:<\/font> sourceString <\/font>setPattern:<\/font> [:<\/font>ignored<\/font>|<\/font>];
\n\t        \t        \t        \t        \t        \t        \t        <\/font>startOfNextToken<\/font>.
\n\t‡       <\/font>tempsStart<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>methodBodyStart<\/font>.
\n\t‡       <\/font>tempsEnd<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        (<\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>tempsToKeepAtMethodLevel<\/font>) <\/font>isEmpty<\/font>
\n\t        \t        <\/font>ifTrue:<\/font>
\n\t‡       \t        \t        [out <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString.
\n\t        \t        \t         soFar <\/font>:=<\/b> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font>]
\n\t        \t        <\/font>ifFalse:<\/font>
\n\t‡       \t        \t        [out <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>putAll:<\/font> sourceString.
\n\t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> <\/font>tempsEnd<\/font>.
\n\t        \t        \t         <\/font>(<\/font>sourceString <\/font>at:<\/font> soFar <\/font>–<\/font> <\/font>1<\/font>)<\/font> <\/font>isSeparator<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>soFar <\/font>:=<\/b> soFar <\/font>–<\/font> <\/font>1<\/font>]<\/font>].
\n\t        blockNodes <\/font>allButFirst<\/font> <\/font>do:<\/font>
\n\t        \t        [:<\/font>blockNode<\/font>|<\/font>
\n\t        \t        <\/font>self<\/font> <\/font>processBlockNode:<\/font> <\/font>blockNode<\/font>].
\n\t        out <\/font>next:<\/font> sourceString <\/font>size<\/font> <\/font>–<\/font> soFar <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> soFar<\/font><\/p>\n

Boom! SyntaxError for EventSensor >> #eventTickler<\/p>\n

\t        eventTickler<\/b>
\n\t        \t        <\/font>"Poll infrequently to make sure that the UI process is not been stuck. <\/font>
\n\t        \t        <\/font>If it has been stuck, then spin the event loop so that I can detect the <\/font>
\n\t        \t        <\/font>interrupt key."<\/font>
\n\t        \t        <\/font>|<\/font> <\/font>delay<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>delay<\/font> <\/font>:=<\/b> Delay <\/font>forMilliseconds:<\/font> <\/font>self<\/font> <\/font>class<\/font> <\/font>eventPollPeriod<\/font>.
\n\t        \t        <\/font>self<\/font> <\/font>lastEventPoll<\/font>.\t        <\/font>"ensure not nil."<\/font>
\n\t        \t        [<\/font>|<\/font> <\/font>delta<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>[<\/font> <\/font>delay<\/font> <\/font>wait<\/font>.
\n\t        \t        <\/font>delta<\/font> <\/font>:=<\/b> Time <\/font>millisecondClockValue<\/font> <\/font>–<\/font> lastEventPoll.
\n\t        \t        <\/font>(<\/font>delta<\/font> <\/font><<\/font> <\/font>0<\/font>
\n\t        \t        \t        \t        <\/font>or:<\/font> <\/font>[<\/font>delta<\/font> <\/font>><\/font> <\/font>self<\/font> <\/font>class<\/font> <\/font>eventPollPeriod<\/font>]<\/font>)<\/font>
\n\t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font>"force check on rollover"<\/font>
\n\t        \t        \t        \t        <\/font>self<\/font> <\/font>fetchMoreEvents<\/font>]<\/font>]<\/font> <\/font>on:<\/font> Error <\/font>do:<\/font> <\/font>[<\/font>:<\/font>ex<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>]<\/font>.
\n\t        \t        <\/font>true<\/font> ] <\/font>whileTrue<\/font>.<\/p>\n

This has a method-level temp declared inside an optimized block, a case I was hoping not to encounter. Let me see if I can just ignore these. Hopefully there shouldn’t be very many of them.<\/p>\n

\t        <\/font>SystemNavigation default browseAllSelect:
\n\t        <\/font>\t        [:m| | s browseDueToSyntaxError |
\n\t        <\/font>\t        browseDueToSyntaxError := false.
\n\t        <\/font>\t        s := InstructionStream on: m.
\n\t        <\/font>\t        (s scanFor:
\n\t        <\/font>\t        \t        \t        [:b|
\n\t        <\/font>\t        \t        \t         b = 143 "closure creation"
\n\t        <\/font>\t        \t        \t         and: [s followingByte >= 16]]) ifTrue:
\n\t        \t        \t        [[(TempScopeEditor new forMethod: m) edit]
\n\t        \t        \t        \t        on: SyntaxErrorNotification
\n\t        \t        \t        \t        do: [:ex|
\n\t        \t        \t        \t        \t        browseDueToSyntaxError := true.
\n\t        \t        \t        \t        \t        ex return]].
\n\t        <\/font>\t        \t        false].
\n\t        <\/font>\t        browseDueToSyntaxError]<\/p>\n

But I also notice there is an empty temp declaration in the <\/font>on:<\/font> Error <\/font>do:<\/font> <\/font>[<\/font>:<\/font>ex<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>]<\/font> error handler block. So…<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>processBlockNode:<\/b> <\/font>blockNode<\/font>
\n\t        <\/font>|<\/font> <\/font>tempsToMoveHere<\/font> <\/font>startOfBlock<\/font> <\/font>endOfArgs<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>blockTempsInSource<\/font> <\/font>|<\/font>
\n\t        <\/font>tempsToMoveHere<\/font> <\/font>:=<\/b> (tempMap <\/font>select:<\/font> <\/font>[<\/font>:<\/font>aBlockNode<\/font>|<\/font> <\/font>aBlockNode<\/font> <\/font>==<\/font> <\/font>blockNode<\/font>]<\/font>) <\/font>keys<\/font>.
\n\t‡       <\/font>tempsToMoveHere<\/font> <\/font>isEmpty<\/font> <\/font>ifTrue:<\/font> [<\/font>^self<\/font>].
\n\t        <\/font>startOfBlock<\/font> <\/font>:=<\/b> (methodNode <\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> <\/font>blockNode<\/font> <\/font>closureCreationNode<\/font>) <\/font>first<\/font>.
\n\t        <\/font>endOfArgs<\/font> <\/font>:=<\/b> <\/font>blockNode<\/font> <\/font>arguments<\/font> <\/font>isEmpty<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifTrue:<\/font> [<\/font>startOfBlock<\/font>]
\n\t        \t        \t        \t        \t        <\/font>ifFalse:<\/font> [sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>startOfBlock<\/font>].
\n\t        out <\/font>next:<\/font> <\/font>endOfArgs<\/font> <\/font>–<\/font> soFar <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> soFar.
\n\t        <\/font>maybeBlockTempsStart<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>ifAbsent:<\/font> sourceString <\/font>size<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        <\/font>blockTempsInSource<\/font> <\/font>:=<\/b> (sourceString <\/font>copyFrom:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font> <\/font>to:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font>) <\/font>allSatisfy:<\/font>
\n\t        \t        \t        \t        \t        \t        \t        \t        [:<\/font>c<\/font>|<\/font> <\/font>c<\/font> <\/font>isSeparator<\/font>].
\n\t        <\/font>blockTempsInSource<\/font>
\n\t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        [out <\/font>next:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>–<\/font> <\/font>endOfArgs<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t         <\/font>(<\/font>self<\/font> <\/font>tempsToKeepFor:<\/font> <\/font>blockNode<\/font>)<\/font> <\/font>do:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>tempNode<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>tempNode<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         <\/font>tempsToMoveHere<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>maybeBlockTempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        \t        \t         <\/font>(<\/font>sourceString <\/font>at:<\/font> soFar <\/font>–<\/font> <\/font>1<\/font>)<\/font> <\/font>isSeparator<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>soFar <\/font>:=<\/b> soFar <\/font>–<\/font> <\/font>1<\/font>]<\/font>]
\n\t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        [out <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t         <\/font>tempsToMoveHere<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         out <\/font>space<\/font>; <\/font>nextPut:<\/font> <\/font>$|<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> <\/font>endOfArgs<\/font> <\/font>+<\/font> <\/font>1<\/font>]
\n<\/font>
\nLot’s of editing going on but then MessageNotUnderstood: UndefinedObject>>notEmpty
\n\t        anyScopes:<\/b> <\/font>referenceScopeDict<\/font> <\/font>outsideExtent:<\/b> <\/font>blockExtent<\/font>
\n\t        \t        <\/font>^<\/font>referenceScopeDict<\/font> <\/font>notEmpty<\/b><\/font>
\n\t        \t         <\/font>and:<\/font> [<\/font>referenceScopeDict<\/font> <\/font>anySatisfy:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>:<\/font>set<\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>set<\/font> <\/font>anySatisfy:<\/font> <\/font>[<\/font>:<\/font>location<\/font>|<\/font> <\/font>(<\/font>blockExtent<\/font> <\/font>rangeIncludes:<\/font> <\/font>location<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/p>\n

Oops. A TempVariableNode’s reading (or writing) scope’s dictionary will be nil if there are no reads (or writes) to it. So this needs to be<\/p>\n

<\/font>TempScopeEditor methods for editing<\/i><\/font>
\n<\/font>anyScopes:<\/b> <\/font>referenceScopeDict<\/font> <\/font>outsideExtent:<\/b> <\/font>blockExtent<\/font>
\n\t        <\/font>^<\/font>referenceScopeDict<\/font> <\/font>notNil<\/font>
\n\t         <\/font>and:<\/font> [<\/font>referenceScopeDict<\/font> <\/font>notEmpty<\/font>
\n\t         <\/font>and:<\/font> <\/font>[<\/font>referenceScopeDict<\/font> <\/font>anySatisfy:<\/font>
\n\t        \t        \t        <\/font>[<\/font>:<\/font>set<\/font>|<\/font>
\n\t        \t        \t        <\/font>set<\/font> <\/font>anySatisfy:<\/font> <\/font>[<\/font>:<\/font>location<\/font>|<\/font> <\/font>(<\/font>blockExtent<\/font> <\/font>rangeIncludes:<\/font> <\/font>location<\/font>)<\/font> <\/font>not<\/font>]<\/font>]<\/font>]<\/font>]<\/p>\n

<\/font>Great. 13 syntax errors. So lots of methods edited and the system is still running \ud83d\ude42 But there is a syntax error reported for SARInstaller class>>ensurePackageWithId: and it has been misedited, missing a vertical bar before newCS:<\/p>\n

\t        ensurePackageWithId:<\/b> <\/font>anIdString<\/font>
\n\t        
\n\t        \t        <\/font>self<\/font> <\/font>squeakMapDo:<\/font> [ :<\/font>sm<\/font> <\/font>|<\/font> <\/font>newCS<\/font> <\/font>|<\/font>
\n\t        \t        \t        <\/font>self<\/font> <\/font>withCurrentChangeSetNamed:<\/font> <\/font>‘updates’<\/font><\/font> <\/font>do:<\/font> <\/font>[<\/font> :<\/font>cs<\/font> <\/font>|<\/font> <\/font>|<\/font> <\/font>card<\/font> <\/font>|<\/font>
\n\t        \t        \t        \t        <\/font>newCS<\/font> <\/font>:=<\/b> <\/font>cs<\/font>.<\/font>
\n\t        \t        \t        \t        <\/font>card<\/font> <\/font>:=<\/b> <\/font>sm<\/font> <\/font>cardWithId:<\/font> <\/font>anIdString<\/font>.<\/font>
\n\t        \t        \t        \t        <\/font>(<\/font>card<\/font> <\/font>isNil<\/font> <\/font>or:<\/font> <\/font>[<\/font> <\/font>card<\/font> <\/font>isInstalled<\/font> <\/font>not<\/font> <\/font>or:<\/font> <\/font>[<\/font> <\/font>card<\/font> <\/font>isOld<\/font> <\/font>]<\/font>]<\/font>)<\/font>
\n\t        \t        \t        \t        \t        <\/font>ifTrue:<\/font> <\/font>[<\/font> <\/font>sm<\/font> <\/font>installPackageWithId:<\/font> <\/font>anIdString<\/font> <\/font>]<\/font>
\n\t        \t        \t        <\/font>]<\/font>.<\/font>
\n\t        \t        \t        <\/font>newCS<\/font> <\/font>isEmpty<\/font> <\/font>ifTrue:<\/font> <\/font>[<\/font> ChangeSet <\/font>removeChangeSet:<\/font> <\/font>newCS<\/font> <\/font>]<\/font>
\n\t        \t        ].<\/font><\/p>\n

Ah, here there are no method-level temps so the copyMethodMovingTemps method confuses the temp declaration in the block for a method-level temp declaration. The tempsStart is for method-level only if tempsStart is before the start of the first proper block. So…
\n
\n<\/font>TempScopeEditor methods for editing<\/i><\/font>
\ncopyMethodMovingTemps<\/b>
\n\t        <\/font>|<\/font> <\/font>methodBodyStart<\/font> <\/font>tempsToKeep<\/font> <\/font>tempsStart<\/font> <\/font>tempsEnd<\/font> <\/font>|<\/font>
\n\t        <\/font>methodBodyStart<\/font> <\/font>:=<\/b> method <\/font>methodClass<\/font> <\/font>parserClass<\/font> <\/font>new<\/font>
\n\t        \t        \t        \t        \t        \t        \t        <\/font>parseMethodComment:<\/font> sourceString <\/font>setPattern:<\/font> [:<\/font>ignored<\/font>|<\/font>];
\n\t        \t        \t        \t        \t        \t        \t        <\/font>startOfNextToken<\/font>.
\n\t        <\/font>tempsStart<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>methodBodyStart<\/font>.
\n\t        <\/font>tempsEnd<\/font> <\/font>:=<\/b> sourceString <\/font>indexOf:<\/font> <\/font>$|<\/font> <\/font>startingAt:<\/font> <\/font>tempsStart<\/font> <\/font>+<\/font> <\/font>1<\/font>.
\n\t        (<\/font>tempsToKeep<\/font> <\/font>:=<\/b> <\/font>self<\/font> <\/font>tempsToKeepAtMethodLevel<\/font>) <\/font>isEmpty<\/font>
\n\t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        [<\/font>|<\/font> <\/font>startOfFirstBlock<\/font> <\/font>|<\/font>
\n\t‡       \t        \t         <\/font>startOfFirstBlock<\/font> <\/font>:=<\/b> <\/font>(<\/font>methodNode <\/font>encoder<\/font> <\/font>sourceRangeFor:<\/font> blockNodes <\/font>second<\/font> <\/font>closureCreationNode<\/font>)<\/font> <\/font>first<\/font>.
\n\t        \t        \t         <\/font>tempsStart<\/font> <\/font><<\/font> <\/font>startOfFirstBlock<\/font>
\n\t        \t        \t        \t        <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        \t         <\/font>[<\/font>out <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>–<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString.
\n\t        \t        \t        \t        \t         soFar <\/font>:=<\/b> <\/font>tempsEnd<\/font> <\/font>+<\/font> <\/font>1<\/font>]<\/font>
\n\t        \t        \t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        \t        \t        <\/font>[<\/font>soFar <\/font>:=<\/b> <\/font>1<\/font>]<\/font>]
\n\t        \t        <\/font>ifFalse:<\/font>
\n\t        \t        \t        [out <\/font>next:<\/font> <\/font>tempsStart<\/font> <\/font>putAll:<\/font> sourceString.
\n\t        \t        \t         <\/font>tempsToKeep<\/font> <\/font>do:<\/font> <\/font>[<\/font>:<\/font>t<\/font>|<\/font> out <\/font>space<\/font>; <\/font>nextPutAll:<\/font> <\/font>t<\/font> <\/font>name<\/font>]<\/font>.
\n\t        \t        \t         soFar <\/font>:=<\/b> <\/font>tempsEnd<\/font>.
\n\t        \t        \t         <\/font>(<\/font>sourceString <\/font>at:<\/font> soFar <\/font>–<\/font> <\/font>1<\/font>)<\/font> <\/font>isSeparator<\/font> <\/font>ifTrue:<\/font>
\n\t        \t        \t        \t        <\/font>[<\/font>soFar <\/font>:=<\/b> soFar <\/font>–<\/font> <\/font>1<\/font>]<\/font>].
\n\t        blockNodes <\/font>allButFirst<\/font> <\/font>do:<\/font>
\n\t        \t        [:<\/font>blockNode<\/font>|<\/font>
\n\t        \t        <\/font>self<\/font> <\/font>processBlockNode:<\/font> <\/font>blockNode<\/font>].
\n\t        out <\/font>next:<\/font> sourceString <\/font>size<\/font> <\/font>–<\/font> soFar <\/font>+<\/font> <\/font>1<\/font> <\/font>putAll:<\/font> sourceString <\/font>startingAt:<\/font> soFar<\/p>\n

Great, 1426 methods edited successfully and 13 syntax errors that I have to fix-up by hand. I’m happy. Of course this would have been much easier with Lisp (and arguably using the Refactoring Editor, but my current understanding is that the publicly available version doesn’t preserve formatting). But writing 8 methods to edit 1426 automatically feels like a win to me.<\/p>\n

What were these syntax errors? There are two cases where a temp that needs to be moved is declared within an optimized control structure, so we were right not to bother handling this case. The others are stores into block arguments, something the old compiler allows but the new compiler only allows if one sets a preference (allowBlockArgumentAssignment). Of these about half are short-cuts and half are cases where the block temp is being nilled because if compiled with the old compiler the temp would be at method level and could cause the system to hold onto the block temp’s value. See e.g. WorldState>>alarmSortBlock:<\/p>\n

<\/font>WorldState methods for alarms<\/i><\/font>
\n<\/font>alarmSortBlock<\/b><\/p>\n

\t        <\/font>|<\/font> <\/font>answer<\/font> <\/font>|<\/font><\/p>\n

\t        <\/font>"Please pardon the hackery below. Since the block provided by this method is retained elsewhere, it is possible that the block argument variables would retain references to objects that were no longer really needed. In one case, this feature resulted in doubling the size of a published project."<\/font><\/p>\n

\t        <\/font>^<\/font>[ :<\/font>alarm1<\/font> :<\/font>alarm2<\/font> <\/font>|<\/font>
\n\t        \t        <\/font>answer<\/font> <\/font>:=<\/b> <\/font>alarm1<\/font> <\/font>scheduledTime<\/font> <\/font><<\/font> <\/font>alarm2<\/font> <\/font>scheduledTime<\/font>.
\n\t        \t        <\/font>alarm1<\/font> <\/font>:=<\/b> <\/font>alarm2<\/font> <\/font>:=<\/b> <\/font>nil<\/font>.
\n\t        \t        <\/font>answer<\/font>
\n\t        ]<\/p>\n

Of course with closures none of this is necessary since the closure doesn’t hold onto its local variables, only the activation of the closure does, and the activation doesn’t persist beyond its invocation. So the above becomes simply<\/p>\n

<\/font>WorldState methods for alarms<\/i><\/font>
\n<\/font>alarmSortBlock<\/b>
\n\t        <\/font>^<\/font>[ :<\/font>alarm1<\/font> :<\/font>alarm2<\/font> <\/font>|<\/font> <\/font>alarm1<\/font> <\/font>scheduledTime<\/font> <\/font><<\/font> <\/font>alarm2<\/font> <\/font>scheduledTime<\/font>]<\/p>\n

But these are changes we’re going to have to make manually. At least the syntax errors allow us to spot them.<\/p>\n

So evaluating the counter above now answers<\/p>\n

\t        <\/font>#(67702 10966 1481 18380 13763 8910 2241 1518)<\/p>\n

which is a healthy change from<\/p>\n

\t         #(67701 10966 2430 18380 14843 9249 4828 2436)<\/p>\n

Let me list this properly so I don’t go insane:<\/p>\n

\t        Methods:\t        \t        \t        \t        \t        \t        <\/font>67702
\n\t        <\/font>MethodsWithClosure:\t        \t        \t        10966
\n\t        <\/font>MethodsWithIndirectTemps:\t        \t         1481
\n\t        <\/font>Closures:\t        \t        \t        \t        \t        \t        18380
\n\t        <\/font>CopiedValues:\t        \t        \t        \t        \t        13763
\n\t        <\/font>ClosuresWithCopiedValues:\t        \t         8910
\n\t        <\/font>RemoteTemps:\t        \t        \t        \t        \t         2241
\n\t        <\/font>ScopesWithRemoteTemps:\t        \t         1518<\/p>\n

So out of 67702 methods 10966 (16.2%) create a closure, but only 1481 (2.2%) require remote temps. This corresponds somewhat to the 1.7% of methods in VisualWorks 5i that used remote temps, but perhaps indicates a stylistic difference between the two systems because of closures.<\/p>\n

Of 18380 closures 8910 (48.5%) require copied values, and they average 13763 \/ 8910 = 1.5 copied values per copying closure, or 13763 \/ 18380 = 0.75 copied values per generic closure. Of 13763 copied values 2241 (16.3%) are for remote temps, so 83.7% of copied values are read-only copies of arguments and temporaries. I can’t easily measure how many closures require remote temps by scanning the bytecode. I’ll leave that as an exercise for the reader \ud83d\ude09 But it should be approximately 1481 \/ 10966 * 18380 \/ 100 or 25% of all closures.<\/p>\n

I’ve responded to the figure of essentially half of all closures needing copied values by modifying the closure design to store copied values in indexable fields of the closure (something Paolo Bonzini asked about). You’ll find the latest version of the bootstrap uses this representation for BlockClosure:
\n\t        Object <\/font>variableSubclass:<\/font> <\/font>#BlockClosure<\/font>
\n\t        \t        <\/font>instanceVariableNames:<\/font> <\/font>‘outerContext startpc numArgs’<\/font><\/font>
\n\t        \t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        \t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        \t        <\/font>category:<\/font> <\/font>‘Kernel-Methods’<\/font><\/font>
\nWhile this arrangement currently precludes adding instance variables to BlockClosure, or using subclasses of BlockClosure, in any new object representation in new versions of Cog, I will try and make determining the number of named instance variables cheap enough that the closure value primitives will be able to accept subclasses of BlockClosure with additional instance variables.<\/p>\n

Cool, Clean Closures<\/a><\/H2><\/p>\n

There are a few other things we should measure, namely how often a block does an up-arrow return and how often a block references self or self’s instance variables, and how often bocks make no access to their enclosing environment at all. To do either of these things the closure has to reference its enclosing context object. [This isn’t strictly correct; one could access self and its instance variables via a copied value, as does VisualWorks, but this is quite complex, and definitely against the spirit of DTSTTCPW.<\/em>] Since the VM will evolve into one that doesn’t create contexts unless absolutely necessary, if a block doesn’t need to reference its enclosing context it wll be faster to create, since the enclosing context will not have to be created as well. In other words, in a VM that optimizes contexts away the current closure design involves at least two allocations, one for the closure and one for the context.<\/p>\n

Blocks that don’t access their outer environment at all, blocks like alarmSortBlock above, which don’t need any copied values, don’t access self or its instance variables and don’t do up-arrow returns, so called "clean blocks" clearly don’t need to have a context instantiated, but my current design requires a context because the closure accesses the method through the context. VisualWorks calls these self-contained blocks "clean" blocks, and, because they make no reference to the outside environment the VisualWorks compiler can create them at compile time. Were we to modify the closure value primitives suitably we could do the same thing. One implementation would be to make the outerContext instance variable do double duty as either an outer context or a method, e.g.
\n\t        Object <\/font>variableSubclass:<\/font> <\/font>#BlockClosure<\/font>
\n\t        \t        <\/font>instanceVariableNames:<\/font> <\/font>‘methodOrOuterContext startpc numArgs’<\/font><\/font>
\n\t        \t        <\/font>classVariableNames:<\/font> <\/font>”<\/font><\/font>
\n\t        \t        <\/font>poolDictionaries:<\/font> <\/font>”<\/font><\/font>
\n\t        \t        <\/font>category:<\/font> <\/font>‘Kernel-Methods’<\/font><\/font>
\nand modify the primitiveClosureValue methods to distinguish between a context or a compiled method (easy, sicne both contexts and methdos have a special unique field value in their object header) and substitute nil as the receiver in the block’s activation when activating a block whose methodOrOuterContext instance variable holds a method.<\/p>\n

All this is easy to do and is probably worth-while for performance but it does have the down side that clean blocks don’t identify themselves properly in the debugger since they don’t have an outer context to refer to. In the VisualWorks debugger when you’re in a clean block you can’t see the values of any variables in the outer context, because the clean block isn’t created within any specific method activation. You can see only in which method the block is defined. So for the moment I’ve deferred implementing this optimization. In any case the VM modifications are small. Most of the work is in the bytecode compiler and this could easily be a system preference so that the compiler would only produce clean blocks if so desired by the more power-hungry programmer.<\/p>\n

OK, let’s measure those clean vs non-clean statisics. Because this is Cinema Verite and not a documentary I’m going to allow myself a short-cut. I’m writing this last section a long time after the earlier measurements. In the interim I’ve made some changes to the compiler (mostly fixes to the decompiler) and have changed the closure representation of copied values as described. So these measurements won’t match exactly with those above but you’ll see they’re really close. Hey, if a work of genius like The Wrong Trousers can survive a few continuity glitches<\/a> then so can a ropey old blog like this! [um, The Wrong trowsers can survive continuity glitches precisely because it is a work of genius. Your ropey old blog, on the other hand, could do well to pull its socks up and show a little humility.<\/em> ed.]<\/p>\n

Here’s the workspace script that also collect the additional measurements.<\/p>\n

\t        | numMethods numMethodsWithClosure numMethodsWithIndirectTemps
\n\t         numClosures numClosuresWithCopiedValues numCopiedValues
\n\t         numRemoteTemps numScopesWithRemoteTemps
\n\t         upArrowReturns usesSelfs upArrowReturnAndUsesSelfs numClean |<\/p>\n

\t        numMethods := numMethodsWithClosure := numMethodsWithIndirectTemps :=
\n\t        numClosures := numClosuresWithCopiedValues := numCopiedValues :=
\n\t        numRemoteTemps := numScopesWithRemoteTemps :=
\n\t        upArrowReturns := usesSelfs := upArrowReturnAndUsesSelfs := numClean := 0.
\n\t        SystemNavigation default allSelect:
\n\t        \t        [:m| | s hasBlock hasIndirectTemps blkPc blkSz doesUAR usesSelf hasCopied |
\n\t        \t        hasBlock := hasIndirectTemps := false.
\n\t        \t        s := InstructionStream on: m.
\n\t        \t        s scanFor:
\n\t        \t        \t        [:b|
\n\t        \t        \t        b = 143 "closure creation" ifTrue:
\n\t        \t        \t        \t        [hasBlock := true.
\n\t        \t        \t        \t        numClosures := numClosures + 1.
\n\t        \t        \t        \t        s followingByte >= 16 ifTrue:
\n\t        \t        \t        \t        \t        [numClosuresWithCopiedValues := numClosuresWithCopiedValues + 1.
\n\t        \t        \t        \t        \t         numCopiedValues := numCopiedValues + (s followingByte >> 4)]].
\n\t        \t        \t        (b = 138 "indirect temp vector creation"
\n\t        \t        \t         and: [s followingByte <= 127]) ifTrue:
\n\t        \t        \t        \t        [hasIndirectTemps := true.
\n\t        \t        \t        \t         numScopesWithRemoteTemps := numScopesWithRemoteTemps + 1.
\n\t        \t        \t        \t         numRemoteTemps := numRemoteTemps + s followingByte].
\n\t        \t        \t        false].
\n\t        \t        numMethods := numMethods + 1.
\n\t        \t        hasBlock ifTrue:
\n\t        \t        \t        [numMethodsWithClosure := numMethodsWithClosure + 1.
\n\t        \t        \t         s pc: m initialPC; scanFor: [:b| b = 143].
\n\t        \t        \t         blkSz := s interpretNextInstructionFor: BlockStartLocator new.
\n\t        \t        \t         blkPc := s pc.
\n\t        \t        \t         doesUAR := usesSelf := false.
\n\t        \t        \t         hasCopied := s followingByte >= 16.
\n\t        \t        \t         s scanFor:
\n\t        \t        \t        \t        [:b|
\n\t        \t        \t        \t        s pc >= (blkPc + blkSz)
\n\t        \t        \t        \t        \t        ifTrue: [true]
\n\t        \t        \t        \t        \t        ifFalse:
\n\t        \t        \t        \t        \t        \t        [doesUAR := doesUAR or: [s willReturn and: [s willBlockReturn not]].
\n\t        \t        \t        \t        \t        \t         usesSelf := usesSelf or: [b = 112 "pushSelf"
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        or: [b < 16 "pushInstVar"
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        or: [(b = 128 and: [s followingByte <= 63]) "pushInstVar"
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        or: [(b between: 96 and: 96 + 7) "storePopInstVar"
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        or: [(b = 130 and: [s followingByte <= 63]) "storePopInstVar"
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        or: [(b = 129 and: [s followingByte <= 63]) "storeInstVar"
\n\t        \t        \t        \t        \t        \t        \t        \t        \t        \t        \t        or: [b = 132 and: [s followingByte = 160]]]]]]]].
\n\t        \t        \t        \t        \t        \t        false]].
\n\t        \t        \t         doesUAR ifTrue:
\n\t        \t        \t        \t        [upArrowReturns := upArrowReturns + 1].
\n\t        \t        \t         usesSelf ifTrue:
\n\t        \t        \t        \t        [usesSelfs := usesSelfs + 1].
\n\t        \t        \t         (doesUAR and: [usesSelf]) ifTrue:
\n\t        \t        \t        \t        [upArrowReturnAndUsesSelfs := upArrowReturnAndUsesSelfs + 1].
\n\t        \t        \t         (doesUAR or: [usesSelf or: [hasCopied]]) ifFalse:
\n\t        \t        \t        \t        [numClean := numClean + 1]].
\n\t        \t        hasIndirectTemps ifTrue: [numMethodsWithIndirectTemps := numMethodsWithIndirectTemps + 1].
\n\t        \t        false].
\n\t        { {‘Methods’. numMethods}. {‘MethodsWithClosure’. numMethodsWithClosure}. {‘MethodsWithIndirectTemps’. numMethodsWithIndirectTemps}.
\n\t         {‘Closures’. numClosures}. {‘CopiedValues’. numCopiedValues}. {‘ClosuresWithCopiedValues’. numClosuresWithCopiedValues}.
\n\t         {‘RemoteTemps’. numRemoteTemps}. {‘ScopesWithRemoteTemps’. numScopesWithRemoteTemps}.
\n\t         {‘UpArrowReturns’. upArrowReturns}. {‘ReferencesSelf’. usesSelfs}. {‘Both’. upArrowReturnAndUsesSelfs}.
\n\t         {‘Clean’. numClean} }<\/p>\n

which produces<\/p>\n

\t        Methods:\t        \t        \t        \t        \t        67805
\n\t        MethodsWithClosure:\t        \t        11022
\n\t        MethodsWithIndirectTemps:\t         1545
\n\t        Closures:\t        \t        \t        \t        \t        18437
\n\t        CopiedValues:\t        \t        \t        \t        13860
\n\t        ClosuresWithCopiedValues:\t         8933
\n\t        RemoteTemps:\t        \t        \t        \t         2366
\n\t        ScopesWithRemoteTemps:\t         1578
\n\t        UpArrowReturns:\t        \t        \t         1025
\n\t        ReferencesSelf:\t        \t        \t        \t         4049
\n\t        Both:\t        \t        \t        \t        \t        \t         346
\n\t        Clean:\t        \t        \t        \t        \t        \t         188<\/p>\n

Clearly clean blocks are too rare to bother with; only 1% are clean. 20% access self. 5.6% of closures use ^-return. 1.9% do both. So it could be worth-while considering an implementation where closures can hold onto their receiver independently of the outerContext, because a whopping 94.4% of closures don’t need their outer context. I’m a bit peeved because in my earliest sketches I had an explicit receiver slot in BlockClosure and I took it out thinking that it was simpler to access receiver and method though the outer context. The greast thing about software is that nothing is set in stone so there will be opportunity to revisit this later. <\/p>\n

After far too long a break the next posts will describe the stack interpreter which, inside the VM, invisibly maps contexts to stack frames.<\/p>\n

\t
\t\t \t\t\"\"<\/a> \t\t  <\/span> \t\tSend article as PDF<\/span> \t\t  <\/span> \t\t \t\t \t<\/form> <\/div>","protected":false},"excerpt":{"rendered":"

You read the last post and I can tell you’re unhappy with the size of the remote temp bytecodes. I can hear you grumbling that it has got to hurt performance. "But these bytecodes are really rare in practice" I claim. "Prove it", you say. "I took a look and there are loads of the […]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":[],"categories":[3],"tags":[],"_links":{"self":[{"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/posts\/14"}],"collection":[{"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/comments?post=14"}],"version-history":[{"count":0,"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/posts\/14\/revisions"}],"wp:attachment":[{"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/media?parent=14"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/categories?post=14"},{"taxonomy":"post_tag","embeddable":true,"href":"http:\/\/www.mirandabanda.org\/cogblog\/wp-json\/wp\/v2\/tags?post=14"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}

So the Big Kahuna<\/a> is<\/p>\n