[Cuis-dev] [IMPROV] Bring back #break

[Gerald Klix]

On 4/11/23 19:50, Juan Vuletich via Cuis-dev wrote:
> On 4/11/2023 1:39 PM, Gerald Klix via Cuis-dev wrote:
>> Hi juan,
>>
>> You a right, I forgot to call the actual method,
>> so it also missed its (side) effects, silly me.
>>
>> In nutshell the BDFL ruled:
>>
>>    1. #halt is enough and it should be used for debugging.
>>    2. If there are sends of #halt in production code,
>>       we just bail out (without a parachute).
>>    3. If the aforementioned behavior is unwanted,
>>       the developer is free to catch the Halt exception
>>       and resume it.
>>
>> I can live with that.
>>
>> I will move the break-related code to a breakpoint-package
>> yet to be written.
>>
>>
>> Best Regards,
>>
>> Gerald
> 
> Hi Gerald,
> 
> I never intend to tell people how their dev environment or process 
> should be. But we need to find a common ground on what goes in the base 
> image.
You should set policy, for example explain the intended purpose
of halt. I inferred – wrongly – from the way #halt was used in Cuis,
that it was intended as way to signal an error, cheaply.

Side note: I looked into Smalltalk-80, Visual Age, Pharo and Squeak.
#break seems to be an invention of Squeak.
Smalltalk-80 does not use exceptions to implement #halt (for
obvious reasons). There were no sends to #halt in a pristine image.

The other Smalltalk implement halt by signaling an exception.

Smalltalk-80: https://github.com/dbanay/Smalltalk

> 
> I'd think that breakpoints, with your new BreakingMethodWrapper is the 
> way to go for debugging. Many will keep using #halt, because we're used 
> to it. Then #break is an alternative, optional way.
In its current state, it will not work well, if you use it in code
that's run in the UI process. It keeps popping up debuggers.
The exception based implementation fares better, it treats
halt as an error and marks the morph as 'failing top draw'.
Of course the state you see in debugger is a bit confusing,
especially for newbies.

I modified the BreakingMethodWrapper to uninstall the wrapper
while the process is debugged; actually as long as the process being
debugged executes methods (indirectly) called/sent by the method we
stopped at. This isn't the ideal behavior, but IHMO much less
confusing than the exception based approach.

Personally 'I can life with' the exception based approach,
but when I think about users, like those of Dr.Geo, I am
not very happy with it. Just try it yourself and consider
the stack display and the debugger's title.
In addition to that, the user needs to know, that he/she has
to restart drawing for the morph being debugged.

The ideal break implementation should have the following properties:

1. Get out of the way in a production environment (maybe silently)
2. Start the debugger in the first process that hits the break point.
3. Keep other processes that hit the break-point running.
4. Break in any recursive sends of the method interrupted if it runs in 
the process we interrupted.

We should be aware that a classic "stop-the-world'-approach as it
is used by, say gdb, is hard to implement, but will have benefits.

If any one has a requirement for halt being an exception,
we can provide that, too.
> 
> OTOH, I don't see much value in Halt being an exception. One way to 
> avoid having three different implementations "for the same thing" could be:
> - #halt exits Cuis if isDevelopmentEnvironmentPresent = false, but uses 
> your #break mechanics if not. No longer an exception.
> - #break is a nop if isDevelopmentEnvironmentPresent = false, but calls 
> #halt otherwise.
> - Breakpoints could perhaps call #break, to avoid duplicating code.
> 
> I think this would cover common cases, including your preference. It 
> would also keep code duplication and "uninteresting for most" complexity 
> to a minimum. It would make it impossible to do exception handlers for 
> #halt, though.
> 
> Thoughts?
See above, please.
:>
> Thanks,
> 
> 

Thanks and Best Regards,

Gerald


PS: I added a change set with the modified break-point code.
-------------- next part --------------
'From Haver 6.0 [latest update: #5760] on 12 April 2023 at 12:09:36 pm'!

!BreakingMethodWrapper methodsFor: 'class membership' stamp: 'KLG 4/12/2023 11:47:59'!
class
	"Primitive. Answer the object which is the receiver's class. Essential. See 
	Object documentation whatIsAPrimitive."

	<primitive: 111>
	^ BreakingMethodWrapper! !


!BreakingMethodWrapper methodsFor: 'system primitives' stamp: 'KLG 4/12/2023 11:48:52'!
run: aSelector with: arguments in: aReceiver
	"I am sent by the virtual machine, if an instance of a class other
	than CompiledMethod is bound in a method dictionary.
	
	See SystemDictionary>>#recreateSpecialObjectsArray at index 50."

	| process |
	"Get rid of that break point, we don't want debuggers popping up while debugging."
	self uninstall.
	Smalltalk isDevelopmentEnvironmentPresent ifFalse: [
		^ method valueWithReceiver: aReceiver arguments: arguments ].
	process := Process 		
		forContext:
			(MethodContext
				sender: 
					(MethodContext
						sender: thisContext sender
						receiver: self
						method: self class>>#install
						arguments: #())
				receiver: aReceiver
				method: method
				arguments: arguments)
			priority: Processor activeProcess 
		priority.	
	process	 debugWithTitle:
		'Breakpoint in ' , method methodClass name , '>>#' , method selector.	
	thisContext swapSender: nil.
	Processor activeProcess terminate! !


!BreakingMethodWrapper reorganize!
('system primitives' doesNotUnderstand: run:with:in:)
('printing' printAs:limitedTo: printOn: printStringLimitedTo: printTextLimitedTo:)
('install/uninstall' install uninstall)
('breakpoints' hasBreakpoint)
('initialization' initializeOn:)
('class membership' class)
!