On 27 janv. 2014, at 14:32, Nicolai Hess <[email protected]> wrote:
> 2014-01-27 Camille Teruel <[email protected]> > > On 27 janv. 2014, at 10:44, Camille Teruel <[email protected]> wrote: > >> >> On 27 janv. 2014, at 09:28, Clément Bera <[email protected]> wrote: >> >>> Hello, >>> >>> I don't think this line was there by mistake. In some cases, the activePC >>> with this code is lower than the startpc and then the debugger shows an >>> error 'no ast node found at this pc' and cannot properly highlight the >>> current executed code because the pc is outside the method bytecode. >> >> Ok, thanks Clément. >> So shouldn't it be: activePC <= self home startpc ifTrue: [ activePC := pc ] >> since self may be the context of a block? > > We looked at the issue with Clément and we agreed on adding #home. > Fix in inbox. > Nicolai what do you think? > >> >>> I remember adding this line as a hack to fix a bug report.... But I don't >>> remember any more... >>> >>> Here's an extract from my blog post, at that time the method had not this >>> line, and we added it due to a bug report: >>> >>> in a debugger, the context on the top of the stack does not highlight the >>> same way other contexts does. In fact, the top context highlights the >>> instruction in source code that *will* be executed in the next step. On the >>> contrary, all the other contexts highlights the instruction in source code >>> that *has been* executed previously. That means that you need to take care >>> about that in the mapping to highlight the correct range. >>> >>> Right now we had to keep the old mapping API so there is a method named >>> rangeForPC:contextIsActiveContext: that has a boolean as second argument. >>> With this boolean you know if you need to look for the range of the current >>> pc (first argument) of the previous pc of the first argument. On the old >>> mapping, they needed the exact pc for their pc to abstract pc mapping. So >>> they needed a method previousPCFor: to handle the case of multiple byte >>> code instructions. But we don’t care, as the pc will be used in the scan of >>> instructionForPC:, so we just need to do pc – 1. >>> >>> The result is : >>> >>> DebuggerMethodMapOpal>>#rangeForPC: aPC >>> contextIsActiveContext:contextIsActive >>> "return the debug highlight for aPC" >>> | pc | >>> pc := contextIsActive ifTrue: [aPC] ifFalse: [aPC - 1]. >>> ^ (methodNode sourceNodeForPC: pc) debugHighlightRange >>> RBMethodNode>>#sourceNodeForPC: anInteger >>> ^ (self ir instructionForPC: anInteger) sourceNode >>> >>> IRMethod>>#instructionForPC: aPC >>> 0 to: -3 by: -1 do: [ : off | >>> (self firstInstructionMatching: [:ir | ir bytecodeOffset = (aPC - off) ]) >>> ifNotNil: [:it |^it]] >>> >>> >>> >>> 2014/1/27 Camille Teruel <[email protected]> >>> >>> On 27 janv. 2014, at 08:44, Marcus Denker <[email protected]> wrote: >>> >>>> >>>> On 26 Jan 2014, at 10:08, Camille Teruel <[email protected]> wrote: >>>> >>>>> [ [ ] ] >>>>> prints: >>>>> [ ] >>>>> instead of: >>>>> [ [ ] ] >>>>> >>>>> and >>>>> [ :arg | [ arg ] ] >>>>> prints: >>>>> DoIt >>>>> ^ [ :arg | [ arg ] ] yourself >>>>> instead of: >>>>> [ :arg | [ arg ] ] >>>>> >>>>> >>>>> Apparently it's the print that is not correct because of problem in >>>>> #sourceNode. >>>>> Any clue? Marcus? Clément? >>>> >>>> Other than “the devil is in the details” (this is all far too complex for >>>> my taste, implementation wise…) >>>> >>>> Can you add an issue? My problem is that i have no time right now to even >>>> think about it… >>> >>> My problem is that I found a fix with try-and-fail without really >>> understanding the code. >>> I just found that problematic examples were passing through that condition >>> whereas non problematic examples were not. >>> I looked at the versions and though that you may have left this line by >>> error. >>> I removed it and so far, it *seems* to work. So the fix is there but I have >>> no clue if it's correct or not. >>> Here it the case: https://pharo.fogbugz.com/f/cases/12732/sourceNode-broken >>> >>>> >>>> Marcus >>> >>> >> > > > I am lost :) > > It looks right, I did some tests and it works. > > But I fail to see, > in what situation would this be true: > activePC <= self home startpc So do I :)
