Issue 12970 - 16 steps to get through #halt - PROPOSED FIX
I've found a fix for something that has annoyed me for a long time - but I can't determine what it might break. 1. In Workspace I evaluate the following Halt enableHaltOnce. a := 1. self halt. b := 2. self haltOnce. c := 3. Transcript show: a + b + c. 2. In the debugger after it stops at the #halt, it takes a SINGLE <Step Over> to move to d:=2 - no problem. 3. Now <Restart> and then <Step Over> down to c:=3 and observe: * the #halt now takes EIGHT (used to be SIXTEEN) steps to traverse (ouch!) * the #haltOnce takes THREE steps to traverse (good) Tracing through for both #halt and #haltOnce it seems that the specific conditional reference to #halt in Halt>>signalerContext is the difference! Removing that condition results in the #halt behaving like #haltOnce, taking only THREE <Step Over>s to traverse a restarted #halt. Halt>>signalerContext ^ signalContext findContextSuchThat: [ :context | (context receiver == self or: [ (context receiver == self class) or: [ context method selector = #halt ]]) not ] Now #haltOnce seems to have been operating without a problem for a long timer, so presumably its okay to make #halt behave the same way. But this is deep stuff I don't fully understand. Anyone see a problem with removing that last condition? cheers -ben
Ben Coman wrote
Anyone see a problem with removing that last condition?
Off the top of my head... Yes that doesn't seem like a good idea. You don't want to find yourself inside Halt. You want to be in your code. Although the change makes this weird exceptional-sounding case better, it looks like a hack that will make halts more annoying in general. The question is why you end up deeply buried in the internals of Halt and how to prevent that in this odd case. ----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
2015-01-25 0:22 GMT+01:00 Sean P. DeNigris <sean@clipperadams.com>:
Ben Coman wrote
Anyone see a problem with removing that last condition?
Off the top of my head... Yes that doesn't seem like a good idea. You don't want to find yourself inside Halt. You want to be in your code.
But isn't it already like that in case of #haltOnce ?
Although the change makes this weird exceptional-sounding case better, it looks like a hack that will make halts more annoying in general. The question is why you end up deeply buried in the internals of Halt and how to prevent that in this odd case.
----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
@Sean, In what way will it make halts more annoying? As far as I can tell, it eliminates FIVE mouse-clicks from #halt, and #haltOnce behaviour is unchanged. Now ideally the debugger should not step into the halt at all when you <Step Over> - but this change eliminates FIVE mouse-clicks from #halt, making #halt behave the same as #haltOnce. I can live with that. Lets do the simple thing first (the ticket has been open 11 months), and leave the potentially harder thing to make it right later on - unless of course this creates other problems. On Mon, Jan 26, 2015 at 5:25 PM, Nicolai Hess <nicolaihess@web.de> wrote:
2015-01-25 0:22 GMT+01:00 Sean P. DeNigris <sean@clipperadams.com>:
Ben Coman wrote
Anyone see a problem with removing that last condition?
Off the top of my head... Yes that doesn't seem like a good idea. You don't want to find yourself inside Halt. You want to be in your code.
But isn't it already like that in case of #haltOnce ?
Although the change makes this weird exceptional-sounding case better, it looks like a hack that will make halts more annoying in general. The question is why you end up deeply buried in the internals of Halt and how to prevent that in this odd case.
----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
Just notice, I have another solution under development. On Tue, Jan 27, 2015 at 9:55 PM, Ben Coman <btc@openinworld.com> wrote:
@Sean, In what way will it make halts more annoying? As far as I can tell, it eliminates FIVE mouse-clicks from #halt, and #haltOnce behaviour is unchanged.
Now ideally the debugger should not step into the halt at all when you <Step Over> - but this change eliminates FIVE mouse-clicks from #halt, making #halt behave the same as #haltOnce. I can live with that. Lets do the simple thing first (the ticket has been open 11 months), and leave the potentially harder thing to make it right later on - unless of course this creates other problems.
On Mon, Jan 26, 2015 at 5:25 PM, Nicolai Hess <nicolaihess@web.de> wrote:
2015-01-25 0:22 GMT+01:00 Sean P. DeNigris <sean@clipperadams.com>:
Ben Coman wrote
Anyone see a problem with removing that last condition?
Off the top of my head... Yes that doesn't seem like a good idea. You don't want to find yourself inside Halt. You want to be in your code.
But isn't it already like that in case of #haltOnce ?
Although the change makes this weird exceptional-sounding case better, it looks like a hack that will make halts more annoying in general. The question is why you end up deeply buried in the internals of Halt and how to prevent that in this odd case.
----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
I poked around and found another possible solution uploaded as SLICE-Issue-12970-16-steps-to-get-through-halt-BenComan.2. However this based on my microscope view of tracing in from a #halt in the <Step Over> button. This is the first time I've looked at this and I don't grasp the the high level architecture. Can someone help polish it? The essential modification is the added line marked "--->" below, plus there is a scattering of <debuggerCompleteToSender> pragmas spread through methods of Halt and Object. I chose to use a pragma since I wanted to cover a broader range of methods than just #halt (e.g. like Halt>>signalerContext) and there seemed no common convention between methods in Halt and Object. Hopefully the pragma provides flexibility for later expansion - but I'm not sure about the pragma name I chose. Process>>complete: aContext "Run self until aContext is popped or an unhandled error is raised. Return self's new top context, unless an unhandled error was raised then return the signaler context (rather than open a debugger)." | ctxt pair error | ctxt := suspendedContext. suspendedContext := nil. "disable this process while running its stack in active process below" pair := Processor activeProcess evaluate: [ctxt runUntilErrorOrReturnFrom: aContext] onBehalfOf: self. suspendedContext := pair first. error := pair second. "--->" error class = Halt ifTrue: [ aContext methodNode pragmaNamed: #debuggerCompleteToSender ifPresent: [ ^ self completeTo: aContext sender ] ]. error ifNotNil: [^ error signalerContext]. ^ suspendedContext Here is my test script to compare before and after the slice. Debug it from Playground then <Step Over> each line. a := 1. Halt now. b := 2. Halt enableHaltOnce. Halt once. c := 3. self halt. d := 4. Halt enableHaltOnce. self haltOnce. self inform: (a + b + c + d) printString. Now if you execute the above script normally, and <Proceed> at each breakpoint, you will find the the #haltOnce leaves you in the wrong position, on level too deep. The can be fixed by adding "or: [ (context method selector = #haltOnce) ]" to Halt>>signalerContext, but then what about the other breakpoint statements like Object>>haltIf:. I wonder if Halt>>signalerContext might leverage the pragmas I added. The slice is tested to work with nested halts per the following example. Execute "BenPlay new outer" and it will stop at the first #halt, then the second #halt can be stepped over, but stepping over #inner breaks at the #halt inside #inner. BenPlay>>outer | a b c | a := 1. self halt. b := 2. self halt. self inner. c := 3. self inform: (a + b + c ) printString. BenPlay>>inner | a b | a := 10. self halt. b := 20. self inform: (a + b) printString. cheers -ben On Tue, Jan 27, 2015 at 11:14 PM, Ben Coman <btc@openinworld.com> wrote:
Just notice, I have another solution under development.
On Tue, Jan 27, 2015 at 9:55 PM, Ben Coman <btc@openinworld.com> wrote:
@Sean, In what way will it make halts more annoying? As far as I can tell, it eliminates FIVE mouse-clicks from #halt, and #haltOnce behaviour is unchanged.
Now ideally the debugger should not step into the halt at all when you <Step Over> - but this change eliminates FIVE mouse-clicks from #halt, making #halt behave the same as #haltOnce. I can live with that. Lets do the simple thing first (the ticket has been open 11 months), and leave the potentially harder thing to make it right later on - unless of course this creates other problems.
On Mon, Jan 26, 2015 at 5:25 PM, Nicolai Hess <nicolaihess@web.de> wrote:
2015-01-25 0:22 GMT+01:00 Sean P. DeNigris <sean@clipperadams.com>:
Ben Coman wrote
Anyone see a problem with removing that last condition?
Off the top of my head... Yes that doesn't seem like a good idea. You don't want to find yourself inside Halt. You want to be in your code.
But isn't it already like that in case of #haltOnce ?
Although the change makes this weird exceptional-sounding case better, it looks like a hack that will make halts more annoying in general. The question is why you end up deeply buried in the internals of Halt and how to prevent that in this odd case.
----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
Ben Coman wrote
@Sean, In what way will it make halts more annoying?
Clearly seems to make things better in your use case, but IIRC the purpose of that test is that you don't want to open the debugger within the halting system. And indeed, when you remove it, you end up in #halt. But where you want to be is in UndefinedObject>>DoIt. So, it seems like a hack that, while better in this case, may have implications in other cases. The solution we're looking for is: "debuggers-opened-due-to-halts always open on the first context totally outside the halt machinery" and we're not quite there yet. ----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
On Fri, Jan 30, 2015 at 2:54 PM, Sean P. DeNigris <sean@clipperadams.com> wrote:
Ben Coman wrote
@Sean, In what way will it make halts more annoying?
Clearly seems to make things better in your use case, but IIRC the purpose of that test is that you don't want to open the debugger within the halting system. And indeed, when you remove it, you end up in #halt. But where you want to be is in UndefinedObject>>DoIt. So, it seems like a hack that, while better in this case, may have implications in other cases. The solution we're looking for is: "debuggers-opened-due-to-halts always open on the first context totally outside the halt machinery" and we're not quite there yet.
Nobody responded to my suggestion of inlining the code for haltOnce so that the Halt signal happens in haltOnce itself, just as it does inside halt. This may make the system respond to haltOnce the same as it does to halt. i.e. the debugger can look to the sender of the signalling context. As it is now, the sender of the signalling context of a haltOnce is halt, not the sender of haltOnce as desired. Let me reiterate, I expect haltOnce "Halt unless we have already done it once." (Smalltalk at: #HaltOnce ifAbsent: [false]) ifTrue: [Smalltalk at: #HaltOnce put: false. Halt signal] will behave similarly to halt Halt signal and stop the debugger where desired. If the decomposition into haltOnceEnabled & clearHaltOnce is desired it can still be written haltOnce "Halt unless we have already done it once." self haltOnceEnabled ifTrue: [self clearHaltOnce. Halt signal] but keeping Halt signal in haltOnce instead of invoking through halt is key.
----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
-- best, Eliot
On Sat, Jan 31, 2015 at 8:44 AM, Eliot Miranda <eliot.miranda@gmail.com> wrote:
On Fri, Jan 30, 2015 at 2:54 PM, Sean P. DeNigris <sean@clipperadams.com> wrote:
Ben Coman wrote
@Sean, In what way will it make halts more annoying?
Clearly seems to make things better in your use case, but IIRC the purpose of that test is that you don't want to open the debugger within the halting system. And indeed, when you remove it, you end up in #halt. But where you want to be is in UndefinedObject>>DoIt. So, it seems like a hack that, while better in this case, may have implications in other cases. The solution we're looking for is: "debuggers-opened-due-to-halts always open on the first context totally outside the halt machinery" and we're not quite there yet.
Okay I agree. So I threw away that solution you commented on and came at it another way. In summary... * The following four statements now all operate identically * self halt. * self haltOnce. * Halt now. * Halt once. * The first time the debugger opens - the halt statement is highlighted in the method that contains it. (In addition, the several other conditional and counting halt methods in the Halt class protocol 'halting' and Object protocol '*Kernel-Exceptions-debugging' "should" also work identically - but I haven't tested them pending confirmation I'm on the right track) * Upon <Restart>, all the above statements are properly ignored by <Step Over> Consider the following now opens a debugger on line 2 and gets to line 5 in just three <Step Over>s. a := 1. self halt. b := 2. self halt. c := 3.
Nobody responded to my suggestion of inlining the code for haltOnce so that the Halt signal happens in haltOnce itself, just as it does inside halt. This may make the system respond to haltOnce the same as it does to halt. i.e. the debugger can look to the sender of the signalling context. As it is now, the sender of the signalling context of a haltOnce is halt, not the sender of haltOnce as desired.
Let me reiterate, I expect
haltOnce "Halt unless we have already done it once." (Smalltalk at: #HaltOnce ifAbsent: [false]) ifTrue: [Smalltalk at: #HaltOnce put: false. Halt signal]
will behave similarly to
halt Halt signal
I believe I have been successful in this. The following now opens a debugger on line 3 instead of inside the #haltOnce.... a := 1. Halt enableHaltOnce. self haltOnce. b := 2. This is all in SLICE-Issue-12970-16-steps-to-get-through-halt-BenComan.4. Now maybe its not the best implementation, but I believe it satisfies all functional requirements and please can it be reviewed for integration asap. For me this adds that much more joy to using our great debugger. cheers -ben
Ben Coman wrote
This is all in SLICE-Issue-12970-16-steps-to-get-through-halt-BenComan.4.
Great, I'll take another look. Did you consider Elliot's suggestion? ----- Cheers, Sean -- View this message in context: http://forum.world.st/Issue-12970-16-steps-to-get-through-halt-PROPOSED-FIX-... Sent from the Pharo Smalltalk Developers mailing list archive at Nabble.com.
On Sat, Jan 31, 2015 at 11:50 PM, Sean P. DeNigris <sean@clipperadams.com> wrote:
Ben Coman wrote
This is all in SLICE-Issue-12970-16-steps-to-get-through-halt-BenComan.4.
Great, I'll take another look. Did you consider Elliot's suggestion?
Sorry I didn't. At first glance I didn't fathom the "inlining" concept and I already had a sniff that this other solution having potential to solve all woes, so I didn't stop to dig into it. Reviewing it again now, I understand the "inlining", but trying it as given (in a fresh image) fails to bring up a debugger at all, for the following... a := 1. Halt enableHaltOnce. self haltOnce. b := 2. I think the following gives the same intent... haltOnce "Halt unless we have already done it once." Halt isHaltOnceEnabled ifTrue: [Halt disableHaltOnce. Halt signal] but the debugger opens on the "Halt signal". This is due to Halt>>signalerContext's special handling ONLY of #halt, which is the essence of the different behaviour between #halt and #haltOnce. Changing "Halt signal" to "self halt" has no effect, with the debugger opening on the same line. As well, I think it would only solve the behaviour for the initial opening of the debugger, but not subsequent encounters of a halt while stepping - which was the problem I was attacking. Indeed, solving that #haltOnce problem was an afterthought - the one line change from the .2 slice to the .3 slice Now for your critical review, in particular consider the pragma name "<debuggerCompleteToSender>". I'm not sure its the best. Also, did I miss tagging any halt-like methods ? cheers -ben
All, Please take a moment to comment on how you like the current behaviour of executing the following snippet then <Step Over> three times to get to the line c := 3. a := 1. self halt. b := 2. self halt. c := 3. cheers -ben
On 24 Feb 2015, at 15:21, Ben Coman <btc@openInWorld.com> wrote:
All,
Please take a moment to comment on how you like the current behaviour of executing the following snippet then <Step Over> three times to get to the line c := 3.
a := 1. self halt. b := 2. self halt. c := 3.
I like the new behavior⦠it seems to work fine. The solution is now very special to this case⦠I wonder if it can be differently (but I have no idea of any). So I vote for adding it⦠we can always improve later. Marcus
Hi Ben, On Sat, Jan 24, 2015 at 7:57 AM, Ben Coman <btc@openinworld.com> wrote:
I've found a fix for something that has annoyed me for a long time - but I can't determine what it might break.
1. In Workspace I evaluate the following
Halt enableHaltOnce. a := 1. self halt. b := 2. self haltOnce. c := 3. Transcript show: a + b + c.
2. In the debugger after it stops at the #halt, it takes a SINGLE <Step Over> to move to d:=2 - no problem.
3. Now <Restart> and then <Step Over> down to c:=3 and observe: * the #halt now takes EIGHT (used to be SIXTEEN) steps to traverse (ouch!) * the #haltOnce takes THREE steps to traverse (good)
Tracing through for both #halt and #haltOnce it seems that the specific conditional reference to #halt in Halt>>signalerContext is the difference! Removing that condition results in the #halt behaving like #haltOnce, taking only THREE <Step Over>s to traverse a restarted #halt.
Halt>>signalerContext ^ signalContext findContextSuchThat: [ :context | (context receiver == self or: [ (context receiver == self class) or: [ context method selector = #halt ]]) not ]
What happens if instead you inline haltOnce to read haltOnce "Halt unless we have already done it once." (Smalltalk at: #HaltOnce ifAbsent: [false]) ifTrue: [Smalltalk at: #HaltOnce put: false. Halt signal] ? Now #haltOnce seems to have been operating without a problem for a long
timer, so presumably its okay to make #halt behave the same way. But this is deep stuff I don't fully understand. Anyone see a problem with removing that last condition?
cheers -ben
-- best, Eliot
participants (5)
-
Ben Coman -
Eliot Miranda -
Marcus Denker -
Nicolai Hess -
Sean P. DeNigris