On 16 Oct 2014 at 22:07:03, Nicolai Hess ([email protected]) wrote: 2014-10-16 17:40 GMT+02:00 Marcus Denker <[email protected]>: >> >> While in the write we need to mark: >> >> lookupVariableForWrite: aVariableNode >> >> | var | >> >> var := scope lookupVar: aVariableNode name. >> >> var ifNil: [^var]. >> var isSpecialVariable ifTrue: [ self storeIntoSpecialVariable: >>aVariableNode ]. >> var isWritable ifFalse: [ self storeIntoReadOnlyVariable: aVariableNode >>]. >> >> var isTemp ifTrue: [ >> "when in a loop, all writes escape" >> scope outerScope isInsideOptimizedLoop ifTrue: [ var >>markEscapingWrite ]. >> "else only escaping when they will end up in different closures" >> (var scope outerNotOptimizedScope ~= scope >>outerNotOptimizedScope) >> ifTrue: [ var markEscapingWrite]]. >> ^var >> >> >> I checked that all Opal tests that are failing are actually testing wrong, >> and I double checked >> that all those methods are now compiled like with the old old compiler… >> Now recompilng the image. > > Hmm… nope…. something wrong. >
the “outerScope” was wrong. So without that, I can recompile the image and your example is compiled without temp vectors… Marcus good! I'll open an issue on fogbugz and create a slice with your changes. about reordering of tempvars: It would be nice if the order preserves, but I think we can live with it. Yes, we should order them. But keep in mind that this does not mean that you can access them via “at: index”. The index is “virtual”, so var3 can be actually var2, because the *real* var2 lives together with var1 in a tempVector (which is now var1). Knowledge has to come from somewhere… e.g. the DebuggerMethodMap builds up a description, while we (for now) just delegate to the AST (+scopes + sematic vars). this means that #tempAt: sees always the real world. e.g. a method with 3 vars where they are all in the tempVector, tempAt:2 will raise an error. namedTempAt: does what the old one did. This now can get confused in the Opal case with the changing order, which we should fix. (and yes, we should check how much slower the AST bases scheme is for accessing temps by offset... we could cache a representation similar to what the old DebuggerMethodMap does). Although there is an issue with the debugger: 14058 Inconsistent information in debugger I solved that one by using tempvar names instead if the index, but this has another issue (sometimes the last (or most nested) tempvar is always nil). The real error is, EyeDebuggerContextInspector does recognizes if the index changes (somewhere in EyeDebuggerContextInspector>>#updateList it compares the new elements (with new indices) with the old elements list and skips the update) Ok… that could be fixed with ordered temp names? Marcus
