Hi Frank,

    I'd rather see

on: exception do: handlerAction
       "Evaluate the receiver in the scope of an exception handler.
       The following primitive is just a marker used to find the error
handling context.
       See MethodContext>>#isHandlerOrSignalingContext. "
       <primitive: 199>
       <handler>
       ^ self value!

isHandlerContext
       "is this context for #on:do:?"
       ^self isHandlerOrSignalingContext and: [ (self method pragmaAt:
#handler) notNil: ]

etc, rather than testing the selector.  I'd also like to see the code in
MethodCOntext (e.g. where method can be directly accessed) than in
ContextPart.  Remember that some time soon we can collapse ContextPart and
MethodContext onto Context as the Pharo folks have already done.

On Fri, Sep 12, 2014 at 1:40 PM, <commits at source.squeak.org> wrote:

> Frank Shearar uploaded a new version of Kernel to project The Inbox:
> http://source.squeak.org/inbox/Kernel-fbs.870.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-fbs.870
> Author: fbs
> Time: 12 September 2014, 9:40:45.406 pm
> UUID: 868e0d8a-5e9d-d34d-ba2d-688800331f5b
> Ancestors: Kernel-eem.869
>
> These are the changes Andres Valloud made to Cuis, to address a
> long-standing bug in Cuis, Squeak and Pharo, in ExceptionsTests >>
> #testHandlerFromAction.
>
> However, it causes a regression in #testHandlerReentrancy, so as it stands
> it is NOT READY FOR MERGING.
>
> Please take a look, and see if we can bend Andres' code slightly so that
> we get both the fix and don't break anything.
>
> =============== Diff against Kernel-eem.869 ===============
>
> Item was changed:
>   ----- Method: BlockClosure>>on:do: (in category 'exceptions') -----
>   on: exception do: handlerAction
> +       "Evaluate the receiver in the scope of an exception handler.
> +       The following primitive is just a marker used to find the error
> handling context.
> +       See MethodContext>>#isHandlerOrSignalingContext. "
> +       <primitive: 199>
> -       "Evaluate the receiver in the scope of an exception handler."
> -
> -       | handlerActive |
> -       <primitive: 199>  "just a marker, fail and execute the following"
> -       handlerActive := true.
>         ^ self value!
>
> Item was changed:
>   ----- Method: BlockContext>>on:do: (in category 'exceptions') -----
>   on: exception do: handlerAction
> +       "Evaluate the receiver in the scope of an exception handler.
> +       The following primitive is just a marker used to find the error
> handling context.
> +       See MethodContext>>#isHandlerOrSignalingContext. "
> +       <primitive: 199>
> +       ^ self value!
> -       "Evaluate the receiver in the scope of an exception handler."
> -       | handlerActive |
> -       <primitive: 199>
> -       handlerActive := true.
> -       ^self value!
>
> Item was changed:
>   ----- Method: ContextPart>>canHandleSignal: (in category
> 'private-exceptions') -----
>   canHandleSignal: exception
>         "Sent to handler (on:do:) contexts only.  If my exception class
> (first arg) handles exception then return true, otherwise forward this
> message to the next handler context.  If none left, return false (see
> nil>>canHandleSignal:)"
>
> +       ^ (self exceptionClass handles: exception)
> +               or: [ self nextHandlerContext canHandleSignal: exception
> ].!
> -       ^ (((self tempAt: 1) handles: exception) and: [self tempAt: 3])
> -               or: [self nextHandlerContext canHandleSignal: exception].
> - !
>
> Item was added:
> + ----- Method: ContextPart>>evaluateSignal: (in category
> 'private-exceptions') -----
> + evaluateSignal: exception
> +       "The following primitive is just a marker used to find the
> evaluation context.
> +       See MethodContext>>#isHandlerOrSignalingContext. "
> +
> +       <primitive: 199>
> +       | value |
> +       exception privHandlerContext: self contextTag.
> +       value := self exceptionHandlerBlock valueWithPossibleArgument:
> exception.
> +       "return from self if not otherwise directed in handle block"
> +       self return: value!
>
> Item was added:
> + ----- Method: ContextPart>>exceptionClass (in category
> 'private-exceptions') -----
> + exceptionClass
> +
> +       ^self tempAt: 1!
>
> Item was added:
> + ----- Method: ContextPart>>exceptionHandlerBlock (in category
> 'private-exceptions') -----
> + exceptionHandlerBlock
> +       "handler context only. access temporaries from
> BlockClosure>>#on:do:"
> +
> +       ^self tempAt: 2!
>
> Item was added:
> + ----- Method: ContextPart>>findNextHandlerContext (in category
> 'private-exceptions') -----
> + findNextHandlerContext
> +       "Return the next handler marked context, returning nil if there is
> none.  Search starts with self and proceeds up to nil."
> +
> +       | context |
> +       context := self findNextHandlerOrSignalingContext.
> +       context isNil
> +               ifTrue: [ ^ nil ].
> +       context isHandlerContext
> +               ifTrue: [ ^ context ].  "If it isn't a handler context, it
> must be a signaling context.
> +       When we reach a signaling context we must skip over any handlers
> +       that might be on the stack between the signaling context and the
> handler
> +       context for that signal."
> +       ^ context exceptionClass privHandlerContext nextHandlerContext!
>
> Item was removed:
> - ----- Method: ContextPart>>findNextHandlerContextStarting (in category
> 'private-exceptions') -----
> - findNextHandlerContextStarting
> -       "Return the next handler marked context, returning nil if there is
> none.  Search starts with self and proceeds up to nil."
> -
> -       | ctx |
> -       <primitive: 197>
> -       ctx := self.
> -               [ctx isHandlerContext ifTrue:[^ctx].
> -               (ctx := ctx sender) == nil ] whileFalse.
> -       ^nil!
>
> Item was added:
> + ----- Method: ContextPart>>findNextHandlerOrSignalingContext (in
> category 'private-exceptions') -----
> + findNextHandlerOrSignalingContext
> +       "Return the next handler/signaling marked context, answering nil
> if there is none.
> +       Search starts with self and proceeds up to nil."
> +
> +       <primitive: 197>
> +       | context |
> +       context := self.
> +       [
> +       context isHandlerOrSignalingContext
> +               ifTrue: [ ^ context ].
> +       (context := context sender) == nil ] whileFalse.
> +       ^ nil!
>
> Item was changed:
>   ----- Method: ContextPart>>handleSignal: (in category
> 'private-exceptions') -----
>   handleSignal: exception
>         "Sent to handler (on:do:) contexts only.  If my exception class
> (first arg) handles exception then execute my handle block (second arg),
> otherwise forward this message to the next handler context.  If none left,
> execute exception's defaultAction (see nil>>handleSignal:)."
>
> +       (self exceptionClass handles: exception)
> +               ifFalse: [ ^ self nextHandlerContext handleSignal:
> exception ].
> +       self evaluateSignal: exception!
> -       | val |
> -       (((self tempAt: 1) handles: exception) and: [self tempAt: 3])
> ifFalse: [
> -               ^ self nextHandlerContext handleSignal: exception].
> -
> -       exception privHandlerContext: self contextTag.
> -       self tempAt: 3 put: false.  "disable self while executing handle
> block"
> -       val := [(self tempAt: 2) cull: exception ]
> -               ensure: [self tempAt: 3 put: true].
> -       self return: val.  "return from self if not otherwise directed in
> handle block"
> - !
>
> Item was changed:
>   ----- Method: ContextPart>>isHandlerContext (in category
> 'private-exceptions') -----
>   isHandlerContext
> +       "is this context for #on:do:?"
> +       ^self isHandlerOrSignalingContext and: [ self selector == #on:do:
> ]!
> -       ^false!
>
> Item was added:
> + ----- Method: ContextPart>>isHandlerOrSignalingContext (in category
> 'private-exceptions') -----
> + isHandlerOrSignalingContext
> +       "Both BlockClosure>>on:do: (handler) and
> ContextPart>>evaluateSignal: (signaling)
> +       are marked with primitive 199."
> +
> +       ^false!
>
> Item was changed:
>   ----- Method: ContextPart>>nextHandlerContext (in category
> 'private-exceptions') -----
>   nextHandlerContext
>
> +       ^ self sender findNextHandlerContext!
> -       ^ self sender findNextHandlerContextStarting!
>
> Item was removed:
> - ----- Method: ContextPart>>rearmHandlerDuring: (in category
> 'private-exceptions') -----
> - rearmHandlerDuring: aBlock
> -       "Sent to handler (on:do:) contexts only. Makes me re-entrant for
> the duration of aBlock. Only works in a closure-enabled image"
> -
> -       ^ [self tempAt: 3 put: true. aBlock value]
> -               ensure: [self tempAt: 3 put: false]!
>
> Item was added:
> + ----- Method: Exception>>privHandlerContext (in category 'priv
> handling') -----
> + privHandlerContext
> +
> +       ^handlerContext!
>
> Item was removed:
> - ----- Method: Exception>>rearmHandlerDuring: (in category 'handling')
> -----
> - rearmHandlerDuring: aBlock
> - "Make the current error handler re-entrant while it is running aBlock.
> Only works in a closure-enabled image"
> -
> -       ^ handlerContext rearmHandlerDuring: aBlock!
>
> Item was removed:
> - ----- Method: MethodContext>>isHandlerContext (in category
> 'private-exceptions') -----
> - isHandlerContext
> - "is this context for  method that is marked?"
> -       ^method primitive = 199!
>
> Item was added:
> + ----- Method: MethodContext>>isHandlerOrSignalingContext (in category
> 'private-exceptions') -----
> + isHandlerOrSignalingContext
> +       "Both BlockClosure>>on:do: (handler) and
> ContextPart>>evaluateSignal: (signaling)
> +       are marked with primitive 199."
> +
> +       ^method primitive = 199!
>
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20140912/4d84f88d/attachment.htm