[ [ ] ] 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?
So with apparently removing the fifth line of MethodContext>>#sourceNode solves the problem, i.e by replacing: sourceNode | activePC | activePC := pc ifNil: [ self startpc + 1] ifNotNil: [pc - 1]. activePC <= self startpc ifTrue: [ activePC := pc ]. ^(method sourceNodeForPC: activePC) methodOrBlockNode with: sourceNode | activePC | activePC := pc ifNil: [ self startpc + 1] ifNotNil: [pc - 1]. ^(method sourceNodeForPC: activePC) methodOrBlockNode But I don't know of its correct, need an expert review :) On 26 janv. 2014, at 10:07, Camille Teruel <camille.teruel@gmail.com> 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?
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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⦠Marcus
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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
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. 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) debugHighlightRangeRBMethodNode>>#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 <camille.teruel@gmail.com>
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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
On 27 janv. 2014, at 09:28, Clément Bera <bera.clement@gmail.com> 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 <camille.teruel@gmail.com>
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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
On 27 janv. 2014, at 10:44, Camille Teruel <camille.teruel@gmail.com> wrote:
On 27 janv. 2014, at 09:28, Clément Bera <bera.clement@gmail.com> 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 <camille.teruel@gmail.com>
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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
2014-01-27 Camille Teruel <camille.teruel@gmail.com>
On 27 janv. 2014, at 10:44, Camille Teruel <camille.teruel@gmail.com> wrote:
On 27 janv. 2014, at 09:28, Clément Bera <bera.clement@gmail.com> 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) debugHighlightRangeRBMethodNode>>#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 <camille.teruel@gmail.com>
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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
On 27 janv. 2014, at 14:32, Nicolai Hess <nicolaihess@web.de> wrote:
2014-01-27 Camille Teruel <camille.teruel@gmail.com>
On 27 janv. 2014, at 10:44, Camille Teruel <camille.teruel@gmail.com> wrote:
On 27 janv. 2014, at 09:28, Clément Bera <bera.clement@gmail.com> 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 <camille.teruel@gmail.com>
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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 :)
I remembered this situation was true in a bug report. Seemingly it is when you do context sourceNode on a dead context. 2014/1/27 Nicolai Hess <nicolaihess@web.de>
2014-01-27 Camille Teruel <camille.teruel@gmail.com>
On 27 janv. 2014, at 10:44, Camille Teruel <camille.teruel@gmail.com> wrote:
On 27 janv. 2014, at 09:28, Clément Bera <bera.clement@gmail.com> 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) debugHighlightRangeRBMethodNode>>#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 <camille.teruel@gmail.com>
On 27 janv. 2014, at 08:44, Marcus Denker <marcus.denker@inria.fr> wrote:
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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
2014-01-27 Marcus Denker <marcus.denker@inria.fr>
On 26 Jan 2014, at 10:08, Camille Teruel <camille.teruel@gmail.com> 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â¦
Marcus
Yes, please add an issue. I 'll take a look at it. One note. Blocks without inner blocks are working: [:x | x] prints [: | x] (in both versions. As there is a difference how this line activePC <= self startpc ifTrue: [ activePC := pc ]. change the sourceNode output. I would take a look at what its purpose may be). nicolai
participants (4)
-
Camille Teruel -
Clément Bera -
Marcus Denker -
Nicolai Hess