[Cuis-dev] Exception handler blocks with non-local returns

[Andres Valloud]

Hello, in regards to recent updates posted to the repository, it looks 
like we collectively forgot to do the cleanup needed for them to go in. 
In addition, it seems like we should have a bit more of a conversation 
before deciding whether the approach in that code is the way to go.

In the spirit of thinking things through, let's not engage in 
'solutioning', where the conversation is about deciding which 'solution' 
to pick.  Rather, let's define the problem first, then decide what a 
'solution' should look like, and finally do something about this.

So, for the sake of reference, I included my earlier observations about 
exception handlers with non-local returns further down.  If you read my 
previous note, feel free to skim that bit.  To summarize, however, the 
argument is mostly against using non-local return in exception handler 
blocks for the reasons explained.

Since then, a bit of investigation on what a cleanup would look like 
revealed that the use of non-local returns in exception handler blocks 
tends to be associated with code that is difficult to change.  This 
suggests there is at least some reason to believe using non-local 
returns in exception handler blocks is not preferable, generally 
speaking (chances are there will be some exceptional cases where it's 
worth the cost, I'm not convinced I've seen one of those yet).

Do we even agree this is a problem?

Andres.

================= arguments from previous email
The conversation that led to these changes went more or less like 
this... first, I noted that exception handlers that look like this:

	[:ex | ^5]

are asking for trouble, because:

a) effectively, nobody is looking at how the exception handling 
mechanism is implemented, so doing non-local return like that curtails 
the private implementation of exception handling,

b) that kind of behavior is what the messages #return, #resume, #pass, 
#outer, etc are there for,

c) when you need to debug why exceptions are behaving a certain way, the 
above exception handlers make debugging impossible because now there's 
no place to put a breakpoint (e.g. in #return, #resume, #pass, #outer, 
etc, which can be redefined for the purpose at hand in the single 
exception of interest), and finally,

d) it's possible to distort the exception handling mechanism behavior 
with non-local returns.


The vast majority of the time, I see exception handlers with non-local 
returns as very close to expressions such as these:

	#(1 2 3) asSortedCollection: [:x :y | ^x < y]

where the behavior you get is not what you expected, even though the 
code is doing exactly what was requested.  See attached for an actual 
example of what I mean.  To run the code, do these and see what happens.

	ExceptionExample new breakingStuff
	ExceptionExample new workingStuff

To me, generally speaking, these considerations push the scales against 
the apparent convenience of having exception handlers do a non-local 
return.  I do not want to have to debug that kind of convenience under 
time pressure.  I do not even want to have to change code that uses such 
convenience, because it's going to be more difficult than otherwise.
================= arguments from previous email

-------------- next part --------------
!classDefinition: #ExceptionExample category: #'ExceptionHandling-examples'!
Object subclass: #ExceptionExample
	instanceVariableNames: ''
	classVariableNames: ''
	poolDictionaries: ''
	category: 'ExceptionHandling-examples'!

!ExceptionExample methodsFor: 'working stuff' stamp: 'sqr 10/13/2019 13:03:20'!
workingStuff

	| innerClosure |
	innerClosure := [:ex | ex return: #thwartedBadly].
	^self breakStuffWithInnerClosure: innerClosure! !


!ExceptionExample methodsFor: 'break stuff' stamp: 'sqr 10/13/2019 13:09:42'!
breakStuffWithInnerClosure: innerClosure

	| answer |
	answer := self protectedStuffToBreakWithInnerClosure: innerClosure.
	answer = #outer ifTrue: [^#workedWrong].
	answer = #thwartedBadly ifTrue: [^#thwartedProperly].
	^answer! !

!ExceptionExample methodsFor: 'break stuff' stamp: 'sqr 10/13/2019 13:06:03'!
breakingStuff

	| innerClosure |
	innerClosure := [:ex | ^#thwartedBadly].
	^self breakStuffWithInnerClosure: innerClosure! !

!ExceptionExample methodsFor: 'break stuff' stamp: 'sqr 10/13/2019 13:09:53'!
protectedStuffToBreakWithInnerClosure: innerClosure

	^[3 / 0.  #didNotFailAsItShould]
		on: ZeroDivide
		do: [:ex | innerClosure value: ex.  ex return: #outer]! !