I was attempting to test some code that happened to have Smalltalk := SmalltalkEnvironment newFrom: Smalltalk. in there. It failed. Not, I must point out because of the rather annoying readonlybinding stuff that seems to have crept into the system. No, this is a fullscale failure of the Compiler. Sound the alarms! Call the Fire Brigade!
After a lot of cussing and scratching I feel reasonably sure it is caused by the (ab)use of LiteralVariableNodes. When translating the sourcecode 'Smalltalk := Smalltalk' (the simplest expression that illustrates the problem) we build a LiteralVariableNode and make it the variable side of an AssignemtnNode. Then we find 'Smalltalk' again and lookup the global to find a binding and set the value side of the AssignmentNode to be the very same LiteralVariableNode. This is how we share literals within a method.
Unfortunately, during the code sizing part of the compilation we send #sizeForValue: to said LiteralVariableNode, which cheerfully sideeffects the crucial instvar 'code'. Later, we send #sizeForStorePop: which also frigs with it. The next phase of compiling is to actually emit the opcodes into the CompiledMethod. Sadly because the LiteralVariableNode is now messed up the code output is ((Compiler new compile:'junkTest3 Smalltalk _ Smalltalk' in: Object notifying: nil ifFail:[]) generate: #(0 0 0 0)) symbolic ' 9 <40> pushLit: Smalltalk 10 <40> pushLit: Smalltalk 11 <CA> send: value: 12 <87> pop 13 <78> returnSelf ' .. which as any fool can see is f'ing wrong. You end up sending #value: to the object Smalltalk instead of the global literal association which has the String 'Smalltalk' as its key and the object Smalltalk as its value. Kaboom, say goodnight Gracie.
What we _should_ get is 13 <21> pushConstant: ROB(#Smalltalk) 14 <40> pushLit: Smalltalk 15 <CA> send: value: 16 <87> pop 17 <78> returnSelf
Sigh.
Changing Encode>global:name: to return a fresh LiteralVariableNode each time fixes the code problem but results in CompiledMethods with multiple literals for each global variable referred. Not the best result we could hope for.
I'm too tired right now to do any more with this but I'd say there are several options available to improve this:-
i) change Encoder>global:name: as above and fix up the handling of literals elsewhere in the code
ii) leave the variable node sharing and change the sizeForStorePop: etc code to not screw things up.
iii) Y'know we really don't need to do this presizing crap in MethodNode>generate: It would be entirely plausible to emit into a stream on a ByteArray and the copy those bytes into a CompiledMethod which size we would work out trivially from the contents of the byte array. And it gets even simpler in VI4 since the CompiledMethod isn't this funky screwed up format anyway. Heck, it might even make compiling faster.
If you want to play with this, try a few expressions like
((Compiler new compile:'junkTest3 Smalltalk _ Smalltalk' in: Object notifying: nil ifFail:[]) generate: #(0 0 0 0)) symbolic
and put a halt before the #generate: . Look at the difference between Smalltalk := Smalltalk and Smalltalk := 1 and so on.
tired tim