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? > 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 > >
