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! |
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
|
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! > > |
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
|
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
|
Free forum by Nabble | Edit this page |