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

Reply via email to