Is this a bug with Step "Over"?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
33 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Is this a bug with Step "Over"?

Chris Muller-3
3 timesRepeat: [Warning signal].
10 factorial

Select the above in a workspace, select "debug it" then, in the
debugger, click the "Over" button just once.

3 Warning pre-debuggers appear instantly and then the execution moves
to the "factorial".

Warnings have the opportunity to resume or cancel?  IOW, shouldn't it
work more like as if it were a halt?

3 timesRepeat: [ self halt ].
10 factorial

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

marcel.taeumel
I think that's what "Step Through" is for.

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Chris Muller-3
No, step Through does not work either, but that is a different question.

My question was about the proper behavior with Warnings when stepping Over.


On Fri, Jun 5, 2015 at 3:21 PM, marcel.taeumel <[hidden email]> wrote:

> I think that's what "Step Through" is for.
>
> Best,
> Marcel
>
>
>
> --
> View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4830738.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Ben Coman
In reply to this post by Chris Muller-3
On Sat, Jun 6, 2015 at 4:35 AM, Chris Muller <[hidden email]> wrote:

> 3 timesRepeat: [Warning signal].
> 10 factorial
>
> Select the above in a workspace, select "debug it" then, in the
> debugger, click the "Over" button just once.
>
> 3 Warning pre-debuggers appear instantly and then the execution moves
> to the "factorial".
>
> Warnings have the opportunity to resume or cancel?  IOW, shouldn't it
> work more like as if it were a halt?
>
> 3 timesRepeat: [ self halt ].
> 10 factorial
>

Pharo does the same and its an interesting question so I've cross posted.
If you "do it" then you get *one* pre-debug window at a time.
When you "debug it" and step "Over" you get *three* debug windows at
once.  Shouldn't it behave the same as the "do it" to get *one* at a
time ?

cheers -ben

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

marcel.taeumel
In reply to this post by Chris Muller-3
Warnings are Notifications, which resume silently if not handled. Only Errors (e.g. Halt) stop execution.

Now if you take a look at Warning >> #defaultAction, you can see why a debugger appears at all -- which is not common for Notifications.

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Chris Muller-3
Warnings do NOT resume silently if not handled.  See?

    Warning signal  "<---- do it"

    Notification signal  "<------- do it"

And nor should they!  Warnings are different from a Notification for
that very reason, as confirmed by the first sentence of Andreas'
comment in Warning>>#defaultAction.

This seems like a bad regression from 4.5, I'm afraid this is a
show-stopper for the 4.6 release...    :(



On Sat, Jun 6, 2015 at 3:19 AM, marcel.taeumel <[hidden email]> wrote:

> Warnings are Notifications, which resume silently if not handled. Only Errors
> (e.g. Halt) stop execution.
>
> Now if you take a look at Warning >> #defaultAction, you can see why a
> debugger appears at all -- which is not common for Notifications.
>
> Best,
> Marcel
>
>
>
> --
> View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4830776.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

marcel.taeumel
Why is Debugger >> #doStep in 4.6 older than in 4.5?

4.5 sd 11/20/2005 21:27
4.6 ajh 7/6/2003 21:06

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Tobias Pape

On 07.06.2015, at 14:09, marcel.taeumel <[hidden email]> wrote:

> Why is Debugger >> #doStep in 4.6 older than in 4.5?
>
> 4.5 sd 11/20/2005 21:27
> 4.6 ajh 7/6/2003 21:06
>

probably resurrecting method time stamps from _ conversion?

just guessing

best regards
        -tobias

> Best,
> Marcel
>
>
>
> --
> View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4830841.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>


Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

marcel.taeumel
Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Eliot Miranda-2
Hi Marcel,

On Mon, Jun 8, 2015 at 2:36 AM, marcel.taeumel <[hidden email]> wrote:
Might it be related to something from here?

http://forum.world.st/Bug-whith-contextOn-do-td4816402.html

http://forum.world.st/Process-specific-state-difficult-to-debug-td4802422.html
http://forum.world.st/The-Trunk-Kernel-eem-896-mcz-td4802546.html

I think it's more to do with a period of confusion I had with ContextPart>>runUntilErrorOrReturnFrom: which I was convinced was buggy qwhen in fact it was a VM issue to do with the awful practice of popping an empty stack, leaving stackp = -1, and then pushing back the receiver, something I wish the VM didn't have to support, but does.




Best,
Marcel



--
View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4830904.html
Sent from the Squeak - Dev mailing list archive at Nabble.com.




--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Eliot Miranda-2
In reply to this post by marcel.taeumel
Hi All,

    so the "bug" is that Warning's default action is to open a debugger:

Warning>>defaultAction
"The user should be notified of the occurrence of an exceptional occurrence and given an option of continuing or aborting the computation. The description of the occurrence should include any text specified as the argument of the #signal: message."
ToolSet
debugContext: thisContext
label: 'Warning'
contents: self messageText, '\\Select Proceed to continue, or close this window to cancel the operation.' withCRs.
self resume.

contrast that with Halt's defaultAction:

Halt>>defaultAction
"No one has handled this error, but now give them a chance to decide how to debug it.  If none handle this either then open debugger (see UnhandedError-defaultAction)"

UnhandledError signalForException: self

With Halt's approach, the debugger can catch it, and catch subsequent halts; indeed the debugger can catch any unhandled exception raised within debugging.  But it can't, and shouldn't, squash the opeining of a debugger if the debugged code chooses to do so.

One fix is to use Halt's defaultAction for Warning.  I think this is the right thing to do.  Another solution is to accept that Warning behaves bizarrely and leave it as is.  What do y'all think?



On Mon, Jun 8, 2015 at 2:36 AM, marcel.taeumel <[hidden email]> wrote:
Might it be related to something from here?

http://forum.world.st/Bug-whith-contextOn-do-td4816402.html

http://forum.world.st/Process-specific-state-difficult-to-debug-td4802422.html
http://forum.world.st/The-Trunk-Kernel-eem-896-mcz-td4802546.html


Best,
Marcel



--
View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4830904.html
Sent from the Squeak - Dev mailing list archive at Nabble.com.




--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Chris Muller-3
> Warning>>defaultAction
> "The user should be notified of the occurrence of an exceptional occurrence
> and given an option of continuing or aborting the computation. The
> description of the occurrence should include any text specified as the
> argument of the #signal: message."
> ToolSet
> debugContext: thisContext
> label: 'Warning'
> contents: self messageText, '\\Select Proceed to continue, or close this
> window to cancel the operation.' withCRs.
> self resume.
>
> contrast that with Halt's defaultAction:
>
> Halt>>defaultAction
> "No one has handled this error, but now give them a chance to decide how to
> debug it.  If none handle this either then open debugger (see
> UnhandedError-defaultAction)"
>
> UnhandledError signalForException: self
>
> With Halt's approach, the debugger can catch it, and catch subsequent halts;
> indeed the debugger can catch any unhandled exception raised within
> debugging.  But it can't, and shouldn't, squash the opeining of a debugger
> if the debugged code chooses to do so.
>
> One fix is to use Halt's defaultAction for Warning.  I think this is the
> right thing to do.  Another solution is to accept that Warning behaves
> bizarrely and leave it as is.  What do y'all think?

Hi Eliot, nobody's #defaultAction changed from 4.5 to 4.6 so why does
Warning work correctly in 4.5 but not 4.6?  I'm concerned the problem
may be deeper..

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

marcel.taeumel
See Debugger class >> #openContext:label:contents:, which is in the control flow of Warning >> defaultAction. There, you can read "Processor activeProcess suspend", which seems to be ignored. Now go to pragma of that method. It says "<primitive: 19>", which is a simulation guard and hence important for a debugger's step-over.

Can someone explain, how those simulation guards work or are supposed to work? There actually were some changes in the simulation code... but where...

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

marcel.taeumel
Hi, all.

I understood the changed behavior. Eliot is right, we need to update Warning >> #defaultAction so that it behaves like Halt >> #defaultAction. Let me explain why...

What is an Exception? An abstract base class for the exception handling system.
What is an Error? A non-resumable exception.
What is a Notification? A resumable exception that does not raise a debugger if unhandled.
What is a Halt? A resumable exception that does raise a debugger if unhandled.

---

So what is a Warning? Actually in 4.5, warnings behave just like Halt, that is, as resumable exceptions that do raise debuggers if unhandled. In 4.6, this still works for normally executed ("do-it") code but not in the debugger.

Why is that?

We just extended "Processor activeProcess" to return the effective process, which is different to the active one while debugging due to Process >> #evaluate:onBehalfOf:, to support debugging process-local state. This is fine.

What changed from 4.5 to 4.6 in the control flow of Warning >> #defaultAction?

Eventually, we get to MorphicProject >> #spawnNewProcessIfThisIsUI. But now in 4.6, the comparison is *false when debugging* and hence *we do not spawn a new UI process*. Until then in 4.5, this spawning of a new UI process cleaned up debugger stuff in the current stack (see Debugger >> #informExistingDebugger:label:) and thus also *dropped the current progress of step-over*!

What was the current progress of step-over? Well, our example is this:

3 timesRepeat: [Warning signal].
Transcript showln: 10 factorial.

We want to step-over the #timesRepeat: call. Now go see Warning >> #defaultAction:

defaultAction
        ToolSet
                debugContext: thisContext
                label: 'Warning'
                contents: self messageText, '\\Select Proceed to continue, or close this window to cancel the operation.' withCRs.
        self resume.

What we do not execute anymore if we spawn a new UI process -- due to the clean-up of the debugger stack -- is "self resume." That call just removes the Warning's calls from the stack. Thus, we keep going in the surrounding step-over progress and get all warnings at once.

In a nutshell, the current implementation of Warning >> #defaultAction exploits a very nasty side-effect in 4.5 (even a buggy one), which got finally fixed in 4.6 when we started evaluating code on behalf of another process.

---

Warning and Halt behave exactly the same. We should make Warning a subclass of Halt and extend the message text as in its current #default action. Warnings *are no* Notifications. They never were.

Any objections?

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Chris Muller-3
> I understood the changed behavior. Eliot is right, we need to update Warning
>>> #defaultAction so that it behaves like Halt >> #defaultAction. Let me
> explain why...
>
> What is an Exception? An abstract base class for the exception handling
> system.
> What is an Error? A non-resumable exception.
> What is a Notification? A resumable exception that does not raise a debugger
> if unhandled.
> What is a Halt? A resumable exception that does raise a debugger if
> unhandled.
>
> ---
>
> So what is a Warning? Actually in 4.5, warnings behave just like Halt, that
> is, as resumable exceptions that do raise debuggers if unhandled. In 4.6,
> this still works for normally executed ("do-it") code but not in the
> debugger.
>
> Why is that?
>
> We just extended "Processor activeProcess" to return the effective process,
> which is different to the active one while debugging due to Process >>
> #evaluate:onBehalfOf:, to support debugging process-local state. This is
> fine.
>
> What changed from 4.5 to 4.6 in the control flow of Warning >>
> #defaultAction?
>
> Eventually, we get to MorphicProject >> #spawnNewProcessIfThisIsUI. But now
> in 4.6, the comparison is *false when debugging* and hence *we do not spawn
> a new UI process*. Until then in 4.5, this spawning of a new UI process
> cleaned up debugger stuff in the current stack (see Debugger >>
> #informExistingDebugger:label:) and thus also *dropped the current progress
> of step-over*!
>
> What was the current progress of step-over? Well, our example is this:
>
> 3 timesRepeat: [Warning signal].
> Transcript showln: 10 factorial.
>
> We want to step-over the #timesRepeat: call. Now go see Warning >>
> #defaultAction:
>
> defaultAction
>         ToolSet
>                 debugContext: thisContext
>                 label: 'Warning'
>                 contents: self messageText, '\\Select Proceed to continue, or close this
> window to cancel the operation.' withCRs.
>         self resume.
>
> What we do not execute anymore if we spawn a new UI process -- due to the
> clean-up of the debugger stack -- is "self resume." That call just removes
> the Warning's calls from the stack. Thus, we keep going in the surrounding
> step-over progress and get all warnings at once.
>
> In a nutshell, the current implementation of Warning >> #defaultAction
> exploits a very nasty side-effect in 4.5 (even a buggy one), which got
> finally fixed in 4.6 when we started evaluating code on behalf of another
> process.
>
> ---

Thank you Marcel!!  I'm very glad to hear that this is the result of
fixing things instead of breaking things.

> Warning and Halt behave exactly the same. We should make Warning a subclass
> of Halt and extend the message text as in its current #default action.
> Warnings *are no* Notifications. They never were.
>
> Any objections?

None initially, but let's just do a quick thought-iteration on
potential impacts.  The only one I can think of is if someone ever
wanted to handle Halt to change or enhance debugging, as in:

  [ "...code to run..." ]
    on: Halt
    do: [ :halt | "invoke special debugger?" ]

that such code would be triggered by Warnings as well (which may be
fine, I don't know).

I see Warnings as an application-level thing whereas Halt as a
system-level thing.  Even though Warnings could inherit the behavior
we want, I think we should at least consider having Warning be its own
subclass of Exception.  I know this means it would duplicate the
one-line method in Halt but, at this level of the system, proper
separation of concerns supercedes the duplication of a small amount of
code.  So which is the proper separation of concerns?

We are back on track, thank you again!!!

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Eliot Miranda-2
In reply to this post by marcel.taeumel
Hi Marcel,

     AFAICT the idea if the simulation guards is to raise errors when doing something that "shouldn't happen during debugging".  But what should or shouldn't happen during debugging?  It's a value judgement.  Fir example, forking a new process.  IMO there no reason why a new process shouldn't be forked during debugging.  Indeed, before yield was a primitive, not bring able to fork would have prevented debugging code that contained a yield.

So unless doing something under debugging would really break the system I don't see that there should be simulation guards.  But that's my opinion.

Eliot (phone)

On Jun 21, 2015, at 12:56 AM, "marcel.taeumel" <[hidden email]> wrote:

> See Debugger class >> #openContext:label:contents:, which is in the control
> flow of Warning >> defaultAction. There, you can read "Processor
> activeProcess suspend", which seems to be ignored. Now go to pragma of that
> method. It says "<primitive: 19>", which is a simulation guard and hence
> important for a debugger's step-over.
>
> Can someone explain, how those simulation guards work or are supposed to
> work? There actually were some changes in the simulation code... but
> where...
>
> Best,
> Marcel
>
>
>
> --
> View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4833361.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Eliot Miranda-2
In reply to this post by marcel.taeumel
Hi Marcel,


On Jun 23, 2015, at 2:44 AM, "marcel.taeumel" <[hidden email]> wrote:

> Hi, all.
>
> I understood the changed behavior. Eliot is right, we need to update Warning
>>> #defaultAction so that it behaves like Halt >> #defaultAction. Let me
> explain why...
>
> What is an Exception? An abstract base class for the exception handling
> system.
> What is an Error? A non-resumable exception.
> What is a Notification? A resumable exception that does not raise a debugger
> if unhandled.
> What is a Halt? A resumable exception that does raise a debugger if
> unhandled.
>
> ---
>
> So what is a Warning? Actually in 4.5, warnings behave just like Halt, that
> is, as resumable exceptions that do raise debuggers if unhandled. In 4.6,
> this still works for normally executed ("do-it") code but not in the
> debugger.
>
> Why is that?
>
> We just extended "Processor activeProcess" to return the effective process,
> which is different to the active one while debugging due to Process >>
> #evaluate:onBehalfOf:, to support debugging process-local state. This is
> fine.
>
> What changed from 4.5 to 4.6 in the control flow of Warning >>
> #defaultAction?
>
> Eventually, we get to MorphicProject >> #spawnNewProcessIfThisIsUI. But now
> in 4.6, the comparison is *false when debugging* and hence *we do not spawn
> a new UI process*. Until then in 4.5, this spawning of a new UI process
> cleaned up debugger stuff in the current stack (see Debugger >>
> #informExistingDebugger:label:) and thus also *dropped the current progress
> of step-over*!
>
> What was the current progress of step-over? Well, our example is this:
>
> 3 timesRepeat: [Warning signal].
> Transcript showln: 10 factorial.
>
> We want to step-over the #timesRepeat: call. Now go see Warning >>
> #defaultAction:
>
> defaultAction
>    ToolSet
>        debugContext: thisContext
>        label: 'Warning'
>        contents: self messageText, '\\Select Proceed to continue, or close this
> window to cancel the operation.' withCRs.
>    self resume.
>
> What we do not execute anymore if we spawn a new UI process -- due to the
> clean-up of the debugger stack -- is "self resume." That call just removes
> the Warning's calls from the stack. Thus, we keep going in the surrounding
> step-over progress and get all warnings at once.
>
> In a nutshell, the current implementation of Warning >> #defaultAction
> exploits a very nasty side-effect in 4.5 (even a buggy one), which got
> finally fixed in 4.6 when we started evaluating code on behalf of another
> process.
>
> ---
>
> Warning and Halt behave exactly the same. We should make Warning a subclass
> of Halt and extend the message text as in its current #default action.
> Warnings *are no* Notifications. They never were.
>
> Any objections?

Quite the opposite.  Let us make it so.

>
> Best,
> Marcel
>
>
>
> --
> View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4833652.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>

Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Tobias Pape

On 23.06.2015, at 18:37, Eliot Miranda <[hidden email]> wrote:

> Hi Marcel,
>
>
> On Jun 23, 2015, at 2:44 AM, "marcel.taeumel" <[hidden email]> wrote:
>
>> Hi, all.
>>
>> I understood the changed behavior. Eliot is right, we need to update Warning
>>>> #defaultAction so that it behaves like Halt >> #defaultAction. Let me
>> explain why...
>>
>> What is an Exception? An abstract base class for the exception handling
>> system.
>> What is an Error? A non-resumable exception.
>> What is a Notification? A resumable exception that does not raise a debugger
>> if unhandled.
>> What is a Halt? A resumable exception that does raise a debugger if
>> unhandled.
>>
>> ---
>>
>> So what is a Warning? Actually in 4.5, warnings behave just like Halt, that
>> is, as resumable exceptions that do raise debuggers if unhandled. In 4.6,
>> this still works for normally executed ("do-it") code but not in the
>> debugger.
>>
>> Why is that?
>>
>> We just extended "Processor activeProcess" to return the effective process,
>> which is different to the active one while debugging due to Process >>
>> #evaluate:onBehalfOf:, to support debugging process-local state. This is
>> fine.
>>
>> What changed from 4.5 to 4.6 in the control flow of Warning >>
>> #defaultAction?
>>
>> Eventually, we get to MorphicProject >> #spawnNewProcessIfThisIsUI. But now
>> in 4.6, the comparison is *false when debugging* and hence *we do not spawn
>> a new UI process*. Until then in 4.5, this spawning of a new UI process
>> cleaned up debugger stuff in the current stack (see Debugger >>
>> #informExistingDebugger:label:) and thus also *dropped the current progress
>> of step-over*!
>>
>> What was the current progress of step-over? Well, our example is this:
>>
>> 3 timesRepeat: [Warning signal].
>> Transcript showln: 10 factorial.
>>
>> We want to step-over the #timesRepeat: call. Now go see Warning >>
>> #defaultAction:
>>
>> defaultAction
>>   ToolSet
>>       debugContext: thisContext
>>       label: 'Warning'
>>       contents: self messageText, '\\Select Proceed to continue, or close this
>> window to cancel the operation.' withCRs.
>>   self resume.
>>
>> What we do not execute anymore if we spawn a new UI process -- due to the
>> clean-up of the debugger stack -- is "self resume." That call just removes
>> the Warning's calls from the stack. Thus, we keep going in the surrounding
>> step-over progress and get all warnings at once.
>>
>> In a nutshell, the current implementation of Warning >> #defaultAction
>> exploits a very nasty side-effect in 4.5 (even a buggy one), which got
>> finally fixed in 4.6 when we started evaluating code on behalf of another
>> process.
>>
>> ---
>>
>> Warning and Halt behave exactly the same. We should make Warning a subclass
>> of Halt and extend the message text as in its current #default action.
>> Warnings *are no* Notifications. They never were.
>>
>> Any objections?
>
> Quite the opposite.  Let us make it so.


Why not vice versa? Halt being a Warning would be more natural to me than Warning being
a Halt.

Best
        -Tobias

>
>>
>> Best,
>> Marcel
>>
>>
>>
>> --
>> View this message in context: http://forum.world.st/Is-this-a-bug-with-Step-Over-tp4830736p4833652.html
>> Sent from the Squeak - Dev mailing list archive at Nabble.com.



Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Stéphane Rollandin

> Why not vice versa? Halt being a Warning would be more natural to me than Warning being
> a Halt.

+1

Stef


Reply | Threaded
Open this post in threaded view
|

Re: Is this a bug with Step "Over"?

Chris Muller-3
In reply to this post by Tobias Pape
> Why not vice versa? Halt being a Warning would be more natural to me than Warning being
> a Halt.

We really should structure the class hierarchy based on the exception
handling use-cases we want to support, not semantics.  If Halt
inherited from Warning, then applications would no longer be able to
halt separately from handling Warnings (except by handling Halt too,
no way!).

12