I got annoyed since a few times I thought I'd solved everything just the issue looking at the manual examples of one issue, only to find I'd caused the other case to fail. Also keeping track of all cases as I threw away images with partial solutions was awkward. So I turned the examples from all these Cases into tests and uploaded AdditionalTests-14606-13260-15174-BenComan.1 - so solutions can consistently test all cases. These can be integrated once we have a solution for all of them. Can you think of anymore to add?
On Sun, Apr 5, 2015 at 8:27 AM, Nicolai Hess <nicolaih...@web.de> wrote: > > > 2015-04-01 18:56 GMT+02:00 Ben Coman <b...@openinworld.com>: > >> Issue 14606 [1] is tagged "Really Important" where evaluating.... >> >> | t1 t2 t3 t4 t5 t6 | >> t1 := 1. >> t2 := 2. >> t3 := 3. >> t4 := 4. >> t5 := 5 . >> t6 := 6. >> [ t5 := 50. >> t6 := 60. >> t3 + t4. >> 1 halt. >> ] value >> >> => can't see the value of t4, t5, t6. >> >> I've just submitted a possible solution for this, but its a bit late >> before release for such a change - so I'm asking for "all hands in" to help >> stress test it. You don't need to review code, just please merge latest >> slice 14606 into your normal working image and report your experience. >> >> cheers -ben >> >> [1] https://pharo.fogbugz.com/default.asp?14606#124466 >> >> > > > Origins of issue "14606 Can't see some variables values while debugging > closure" > It starts with issue: > 14079 Inconsistent information in debugger (pharo 4) > (fixed in 40258) > The thisContext inspector in the debugger (sometimes) shows the wrong > values for the thisContext temp vars. > This was first seen in Method FreeTypeFontProvider>>#updateFromFile: > I extracted a minimal example to reproduce this behavior: > [ :x | > | d c b a | > a := 1. > b := 2. > c := 3. > d := 4. > [ c:=3. c=0 ] on:Error do:[:ex|]. > d halt. > [ c:=3. c=0 ] whileTrue:[]] value:23 > If you execute this method, it stops at "d halt". Look at the "all temp > vars" and the > individual temp var entries in the context inspector. It shows the temp > vars with the correct > values. Now do a step over, int the whileTrue block the temp var name list > is unchanged but > the values changed their order, now the list of individual temp vars shows > the wrong values! > Why did this happen? > There are two problems, > 1. The order of temp vars changes between the method context and the block > closure > 2. The EyeDebuggerContextInspector compares ContextTempEyeElements > without their inst vars (tempName AND tempIndex). > In EyeInspector>>#updateList: > We step into a new block and the debugger calls updateList, here > we check the current list (self list getItems) with the new list (elements) > and check if they are equal, they aren't, because the lists contain > ContextTempEyeElements with > the same tempName but different tempIndex, but the #= selector in > AbstractEyeElement doesn't check this. > EyeInspector>>#updateList > "update the list of elements displayed according to the new object" > > | elements | > " self haltOnce." > elements := self generateElements. > self list getItems = elements ifTrue: [ ^ self ]. "<<<--- returns > event if the elements aren't equal" > .... > > Possible solutions: > - Don't reorder tempvars in inner scopes. This needs some deeper knowlegde > of opals AST Translator. > - Fix the #= for AbstractEyeElement resp. ContextTempEyeElement > - in ContextTempEyeElement, access the temp values not by index (they > changed) but by name > I choosed the latter, because I could not get a working fix for the other, > this raised some > new issues. Not really new, they exists already, but weren't visible > because they were only > used in special cases: > > 13260 inspecting variables in the debugger fails in some cases > (fixed in 40354) > Minimal example: > x := [ :k || b a | b:=[ a:=1. a:=k+1.a]] value:7. > x value > > *Execute* the first line, then debug the second. > In the debugger, in the thisContext inspector, you can select the var > "k", it shows 7, you can select the var "b" it shows nil (because it *is* > nil), > you can not show the value for "a", although it should show "1". > Why did this happens? > With the fix for issue 14079, we now use #tempNamed:aName instead of > #namedTempAt: anIndex > but in the end, the context only stores the temp vars by index, so we (the > debugger) > needs to find the index for the named temp. This is done through > DebuggerMethodMapOpal > namedTempAt:in: resp. tempNamed:in: > It looks up the temp index by the variable binding for the *Method* scope > (and finally the method context). > This is wrong, because in this case, the method context is already dead. > I fixed this by changing OCVectorTempVariable>>#readFromContext:scope: > to start from the inner scope instead of the scope that defined the method > (because it can > be already dead). (This wasn't straight forward, I need to change some > more because the > way the IR class for constructing the closures bytecodes, only searched in > its "block sequence" > but the copyied tempvector is defined in its "sequence" (I don't really > remember what is what). > Even if I had choosed a different fix for issue 14079, this still had to > be fixed, because > indirect variable access through namedTempAt: is used, if we select the > temp var in the *code view* > and do a inspectIt. > Even with this fix, the issue is not fully solved: > > Unsolved issues: > 14606 Can't see some variables values while debugging closure > Minimal example: > |a b c| > a := [ Transcript show: 'test' ]. > [ b :=4. > c := 5. > 1 halt] on: Error do: a. > select the b or c value in the thisContext inspector shows "nil" although > they actually have a value. > And > > 15174 error when inspecting/printing variables in the debugger > > this is a minimal example, reproducing this case, the assignment on "a" is > alreaday > done, after that accessing the value with DebuggerMethodMapOpal should > work, but > it raises an error > | a b | > [ ] value. > a := 1. > b := [ > a := 4. > (DebuggerMethodMapOpal forMethod: thisContext method) tempNamed: 'a' > in: thisContext ] value. > ^ a + b > > -> MessageNotUnderstood: receiver of "indexForVarNamed:" is nil > in OCCopyingTempVariable>>indexInTempVectorFromIR: > > Both issues has to be solved and they have actually two causes: > 1. There is a guarding clause in Context>>tempNamed: anIndex > If the context stackpointer is below temp index, it returns nil. This is > for the case > the block closure vars aren't yet fully initialized (context tempvars are > pushed > on the stack with pushNil, if the stack pointer is below the index, it > looks like we > access a temp var, for which there isn't yet a place on the stack). > But this is wrong! > If the closure uses a closure copied or closure indirect tempvars, then > multiple > vars may be in *one* array that only takes one place on the stack. > 2. (This is the reason for the MNU). Reading the temp vector index for > "copied" temp > vars fails. My proposed solution in "15174 error when inspecting/printing > variables in the debugger" > Looks like fixing this bug at the wrong place. I think it has something to > do with 12887 Fix pc->AST maping. > > There was another issue, something with optimized blocks and reading and > or writing closure temp > vars, but I don't remember that, one. Don't know if it had something to do > with this issues. > > Hope this was understandable and it helps (but I doubt we can solve any of > this for this release. > I was working on them the last 6 month, and issues 12887 is there since > one year, still exists). > > > nicolai > > >
