The Inbox: Kernel-nice.1391.mcz

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

The Inbox: Kernel-nice.1391.mcz

commits-2
Nicolas Cellier uploaded a new version of Kernel to project The Inbox:
http://source.squeak.org/inbox/Kernel-nice.1391.mcz

==================== Summary ====================

Name: Kernel-nice.1391
Author: nice
Time: 23 April 2021, 10:58:48.925778 am
UUID: bde1d6d3-e3ba-48ad-b432-3ac5a1531880
Ancestors: Kernel-nice.1390

Remove the questionable ifCurtailed: block that did reactivate the handler in handleSignal:, and make the handling symetric, whether the handlerAction explicitily use exception return or not.

Document the mysterious tempAt:/tempAt:put: intention by using proper method names as requested by Marcel.
The slowdown shall be marginal, and system understanding should be improved.

Provide messages to selectively reactivate some handler contexts. For gurus: handle with care!

=============== Diff against Kernel-nice.1390 ===============

Item was changed:
  ----- Method: Context>>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 willHandleSignal: exception)
- ^ (((self tempAt: 1) handles: exception) and: [self tempAt: 3])
  or: [self nextHandlerContext canHandleSignal: exception].
  !

Item was added:
+ ----- Method: Context>>deactivateHandler (in category 'private-exceptions') -----
+ deactivateHandler
+ "Private - sent to exception handler context only (on:do:)"
+
+ stackp >= 3 ifTrue: [self tempAt: 3 put: false] "this is temporary handlerActive in #on:do:"!

Item was added:
+ ----- Method: Context>>fireHandlerActionWith: (in category 'private-exceptions') -----
+ fireHandlerActionWith: exception
+ "Sent to handler (on:do:) contexts only.
+ Perform the second argument, which is the handler action"
+
+ ^(self tempAt: 2) cull: exception!

Item was changed:
  ----- Method: Context>>handleSignal: (in category 'private-exceptions') -----
  handleSignal: exception
  "Sent to handler (on:do:) contexts only.  If my exception class (first arg) handles exception
  and the handler is active 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:)."
 
+ | val |
+ (self willHandleSignal: exception) ifFalse:
+ [self deactivateHandler.
- | handlerActive val |
- "If the context has been returned from the handlerActive temp var may not be accessible."
- handlerActive := stackp >= 3 and: [(self tempAt: 3) == true].
- (((self tempAt: 1) handles: exception) and: [handlerActive]) ifFalse:
- [stackp >= 3 ifTrue: [self tempAt: 3 put: false].
  ^self nextHandlerContext handleSignal: exception].
 
  exception privHandlerContext: self contextTag.
+ self deactivateHandler.  "disable self while executing handle block"
+ val := self fireHandlerActionWith: exception.
- self tempAt: 3 put: false.  "disable self while executing handle block"
- val := [(self tempAt: 2) cull: exception]
- ifCurtailed: [self tempAt: 3 put: true].
  self return: val  "return from self if not otherwise directed in handle block"
  !

Item was added:
+ ----- Method: Context>>isHandlerActive (in category 'private-exceptions') -----
+ isHandlerActive
+ "Private - sent to exception handler context only (on:do:)"
+
+ ^stackp >= 3 and: [(self tempAt: 3) == true] "this is temporary handlerActive in #on:do:"!

Item was added:
+ ----- Method: Context>>reactivateHandler (in category 'private-exceptions') -----
+ reactivateHandler
+ "Private - sent to exception handler context only (on:do:)"
+
+ stackp >= 3 ifTrue: [self tempAt: 3 put: true] "this is temporary handlerActive in #on:do:"!

Item was changed:
  ----- Method: Context>>reactivateHandlers (in category 'private-exceptions') -----
  reactivateHandlers
  "Private - sent to exception handler context only (on:do:).
  Reactivate all the handlers into the chain"
 
+ self reactivateHandler.
- self tempAt: 3 put: true. "this is temporary handlerActive in #on:do:"
  self nextHandlerContext reactivateHandlers!

Item was added:
+ ----- Method: Context>>reactivateHandlersUpTo: (in category 'private-exceptions') -----
+ reactivateHandlersUpTo: aHandlerContext
+ "Private - sent to exception handler context only (on:do:).
+ Reactivate the inner handlers into the chain, up to, but not including, aHandlerContext"
+
+ self == aHandlerContext ifTrue: [^self].
+ self reactivateHandler.
+ self nextHandlerContext reactivateHandlersUpTo: aHandlerContext!

Item was added:
+ ----- Method: Context>>reactivateHandlersWhich:upTo: (in category 'private-exceptions') -----
+ reactivateHandlersWhich: selectBlock upTo: aHandlerContext
+ "Private - sent to exception handler context only (on:do:).
+ Reactivate the inner handlers into the chain, up to, but not including, aHandlerContext, that satisfy the selectBlock predicate"
+
+ self == aHandlerContext ifTrue: [^self].
+ (selectBlock value: self) ifTrue: [self reactivateHandler].
+ self nextHandlerContext reactivateHandlersWhich: selectBlock upTo: aHandlerContext!

Item was changed:
  ----- Method: Context>>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 reactivateHandler. aBlock value]
+ ensure: [self deactivateHandler]!
- ^ [self tempAt: 3 put: true. aBlock value]
- ensure: [self tempAt: 3 put: false]!

Item was added:
+ ----- Method: Context>>willHandleSignal: (in category 'private-exceptions') -----
+ willHandleSignal: exception
+ "Sent to handler (on:do:) contexts only."
+
+ ^self isHandlerActive and: [(self tempAt: 1) handles: exception]
+ !

Item was added:
+ ----- Method: Exception>>reactivateHandlersUpTo: (in category 'priv handling') -----
+ reactivateHandlersUpTo: aHandlerContext
+ "reactivate all the exception handlers in the context chain"
+ self canSearchForSignalerContext
+ ifTrue: [signalContext nextHandlerContext reactivateHandlersUpTo: aHandlerContext]!

Item was added:
+ ----- Method: UndefinedObject>>reactivateHandlersUpTo: (in category 'bottom context') -----
+ reactivateHandlersUpTo: aHandlerContext
+
+ ^ self!

Item was added:
+ ----- Method: UndefinedObject>>reactivateHandlersWhich:upTo: (in category 'bottom context') -----
+ reactivateHandlersWhich: selectBlock upTo: aHandlerContext
+
+ ^ self!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-nice.1391.mcz

Jaromir Matas
Hi Nicolas,

> Remove the questionable ifCurtailed: block that did reactivate the handler
> in handleSignal:, and
> make the handling symetric, whether the handlerAction explicitily use
> exception return or not.

I'd like to suggest the following change to really make handling `return`
the same whether it's called explicitly or not: The last line should send
`return: val` to exception instead of self - the `exception return` calls
`handlerContext return` and at the moment it's true the `self` is the
handlerContext. But in theory someone can e.g. change `return` definition
and then the two returns won't be the same... Plus I think this is more
consistent and readable. Thanks,

handleSignal: exception
        "Sent to handler (on:do:) contexts only.  If my exception class (first arg)
handles exception
         and the handler is active 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:)."

        | val |
        (self willHandleSignal: exception) ifFalse:
                [self deactivateHandler.
                ^self nextHandlerContext handleSignal: exception].

        exception privHandlerContext: self contextTag.
        self deactivateHandler.  "disable self while executing handle block"
        val := self fireHandlerActionWith: exception.
+ exception return: val  "return as default action if not otherwise directed
in handle block"
- self return: val  "return from self if not otherwise directed in handle
block"




-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-nice.1391.mcz

Jakob Reschke
In reply to this post by commits-2
Hi Nicolas,

Please find Tests-jr.456 in the Inbox. It contains a test case that
reveals that reactivateHandlers is too eager at the moment. I tested
it with Kernel-nice.1391 loaded.

Kind regards,
Jakob

Am Fr., 23. Apr. 2021 um 10:58 Uhr schrieb <[hidden email]>:

>
> Nicolas Cellier uploaded a new version of Kernel to project The Inbox:
> http://source.squeak.org/inbox/Kernel-nice.1391.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-nice.1391
> Author: nice
> Time: 23 April 2021, 10:58:48.925778 am
> UUID: bde1d6d3-e3ba-48ad-b432-3ac5a1531880
> Ancestors: Kernel-nice.1390
>
> Remove the questionable ifCurtailed: block that did reactivate the handler in handleSignal:, and make the handling symetric, whether the handlerAction explicitily use exception return or not.
>
> Document the mysterious tempAt:/tempAt:put: intention by using proper method names as requested by Marcel.
> The slowdown shall be marginal, and system understanding should be improved.
>
> Provide messages to selectively reactivate some handler contexts. For gurus: handle with care!
>
> =============== Diff against Kernel-nice.1390 ===============
>
> Item was changed:
>   ----- Method: Context>>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 willHandleSignal: exception)
> -       ^ (((self tempAt: 1) handles: exception) and: [self tempAt: 3])
>                 or: [self nextHandlerContext canHandleSignal: exception].
>   !
>
> Item was added:
> + ----- Method: Context>>deactivateHandler (in category 'private-exceptions') -----
> + deactivateHandler
> +       "Private - sent to exception handler context only (on:do:)"
> +
> +       stackp >= 3 ifTrue: [self tempAt: 3 put: false] "this is temporary handlerActive in #on:do:"!
>
> Item was added:
> + ----- Method: Context>>fireHandlerActionWith: (in category 'private-exceptions') -----
> + fireHandlerActionWith: exception
> +       "Sent to handler (on:do:) contexts only.
> +       Perform the second argument, which is the handler action"
> +
> +       ^(self tempAt: 2) cull: exception!
>
> Item was changed:
>   ----- Method: Context>>handleSignal: (in category 'private-exceptions') -----
>   handleSignal: exception
>         "Sent to handler (on:do:) contexts only.  If my exception class (first arg) handles exception
>          and the handler is active 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:)."
>
> +       | val |
> +       (self willHandleSignal: exception) ifFalse:
> +               [self deactivateHandler.
> -       | handlerActive val |
> -       "If the context has been returned from the handlerActive temp var may not be accessible."
> -       handlerActive := stackp >= 3 and: [(self tempAt: 3) == true].
> -       (((self tempAt: 1) handles: exception) and: [handlerActive]) ifFalse:
> -               [stackp >= 3 ifTrue: [self tempAt: 3 put: false].
>                 ^self nextHandlerContext handleSignal: exception].
>
>         exception privHandlerContext: self contextTag.
> +       self deactivateHandler.  "disable self while executing handle block"
> +       val := self fireHandlerActionWith: exception.
> -       self tempAt: 3 put: false.  "disable self while executing handle block"
> -       val := [(self tempAt: 2) cull: exception]
> -                       ifCurtailed: [self tempAt: 3 put: true].
>         self return: val  "return from self if not otherwise directed in handle block"
>   !
>
> Item was added:
> + ----- Method: Context>>isHandlerActive (in category 'private-exceptions') -----
> + isHandlerActive
> +       "Private - sent to exception handler context only (on:do:)"
> +
> +       ^stackp >= 3 and: [(self tempAt: 3) == true] "this is temporary handlerActive in #on:do:"!
>
> Item was added:
> + ----- Method: Context>>reactivateHandler (in category 'private-exceptions') -----
> + reactivateHandler
> +       "Private - sent to exception handler context only (on:do:)"
> +
> +       stackp >= 3 ifTrue: [self tempAt: 3 put: true] "this is temporary handlerActive in #on:do:"!
>
> Item was changed:
>   ----- Method: Context>>reactivateHandlers (in category 'private-exceptions') -----
>   reactivateHandlers
>         "Private - sent to exception handler context only (on:do:).
>         Reactivate all the handlers into the chain"
>
> +       self reactivateHandler.
> -       self tempAt: 3 put: true. "this is temporary handlerActive in #on:do:"
>         self nextHandlerContext reactivateHandlers!
>
> Item was added:
> + ----- Method: Context>>reactivateHandlersUpTo: (in category 'private-exceptions') -----
> + reactivateHandlersUpTo: aHandlerContext
> +       "Private - sent to exception handler context only (on:do:).
> +       Reactivate the inner handlers into the chain, up to, but not including, aHandlerContext"
> +
> +       self == aHandlerContext ifTrue: [^self].
> +       self reactivateHandler.
> +       self nextHandlerContext reactivateHandlersUpTo: aHandlerContext!
>
> Item was added:
> + ----- Method: Context>>reactivateHandlersWhich:upTo: (in category 'private-exceptions') -----
> + reactivateHandlersWhich: selectBlock upTo: aHandlerContext
> +       "Private - sent to exception handler context only (on:do:).
> +       Reactivate the inner handlers into the chain, up to, but not including, aHandlerContext, that satisfy the selectBlock predicate"
> +
> +       self == aHandlerContext ifTrue: [^self].
> +       (selectBlock value: self) ifTrue: [self reactivateHandler].
> +       self nextHandlerContext reactivateHandlersWhich: selectBlock upTo: aHandlerContext!
>
> Item was changed:
>   ----- Method: Context>>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 reactivateHandler. aBlock value]
> +               ensure: [self deactivateHandler]!
> -       ^ [self tempAt: 3 put: true. aBlock value]
> -               ensure: [self tempAt: 3 put: false]!
>
> Item was added:
> + ----- Method: Context>>willHandleSignal: (in category 'private-exceptions') -----
> + willHandleSignal: exception
> +       "Sent to handler (on:do:) contexts only."
> +
> +       ^self isHandlerActive and: [(self tempAt: 1) handles: exception]
> + !
>
> Item was added:
> + ----- Method: Exception>>reactivateHandlersUpTo: (in category 'priv handling') -----
> + reactivateHandlersUpTo: aHandlerContext
> +       "reactivate all the exception handlers in the context chain"
> +       self canSearchForSignalerContext
> +               ifTrue: [signalContext nextHandlerContext reactivateHandlersUpTo: aHandlerContext]!
>
> Item was added:
> + ----- Method: UndefinedObject>>reactivateHandlersUpTo: (in category 'bottom context') -----
> + reactivateHandlersUpTo: aHandlerContext
> +
> +       ^ self!
>
> Item was added:
> + ----- Method: UndefinedObject>>reactivateHandlersWhich:upTo: (in category 'bottom context') -----
> + reactivateHandlersWhich: selectBlock upTo: aHandlerContext
> +
> +       ^ self!
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-nice.1391.mcz

Jaromir Matas
In reply to this post by Jaromir Matas
Hi All,


Jaromir Matas wrote

> I'd like to suggest the following change to really make handling `return`
> the same whether it's called explicitly or not: The last line should send
> `return: val` to exception instead of self - the `exception return` calls
> `handlerContext return` and at the moment it's true the `self` is the
> handlerContext. But in theory someone can e.g. change `return` definition
> and then the two returns won't be the same... Plus I think this is more
> consistent and readable. Thanks,
>
> handleSignal: exception
> "Sent to handler (on:do:) contexts only.  If my exception class (first
> arg)
> handles exception
> and the handler is active 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:)."
>
> | val |
> (self willHandleSignal: exception) ifFalse:
> [self deactivateHandler.
> ^self nextHandlerContext handleSignal: exception].
>
> exception privHandlerContext: self contextTag.
> self deactivateHandler.  "disable self while executing handle block"
> val := self fireHandlerActionWith: exception.
> + exception return: val  "return as default action if not otherwise
> directed
> in handle block"
> - self return: val  "return from self if not otherwise directed in handle
> block"

Please disregard the above proposal - it leads to an incorrect #outer
behavior. Unfortunately it passes all the tests which shouldn't happen and I
sent a fix to the Inbox to update the outer test:
http://forum.world.st/The-Inbox-Tests-jar-454-mcz-td5129221.html

best,



-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-nice.1391.mcz

Jaromir Matas
Hi

I'd like to return to my original proposal above in
http://forum.world.st/The-Inbox-Kernel-nice-1391-mcz-tp5129040p5129084.html.
There was a bug in #outer that confused me and I withdrew the proposal. The
bug has been fixed and the original proposal in my opinion makes sense again
- to unify how the two kinds of exception return are implemented.
Theoretically it's possible to change #return definition in the future and
then the two returns would diverge.

The proposed change is in
http://forum.world.st/The-Inbox-Kernel-jar-1399-mcz-td5129370.html:

Context>>handleSignal: (in category 'private-exceptions') -----
  handleSignal: exception
  "Sent to handler (on:do:) contexts only.
  Execute the handler action block"
 
  | val |
  <primitive: 199>  "just a marker, fail and execute the following"
  exception privHandlerContext: self contextTag.
  self deactivateHandler. "Prevent re-entering the action block, unless it
is explicitely rearmed"
  val := [self fireHandlerActionForSignal: exception] ensure: [self
reactivateHandler].
+ exception return: val  "return from exception handlerContext if not
otherwise directed in handle block"!
- self return: val  "return from self if not otherwise directed in handle
block"!



-----
^[^ Jaromir
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

^[^ Jaromir