pharo icon indicating copy to clipboard operation
pharo copied to clipboard

fixing recompilation of bloc in the debugger with dead home context whose method has already been recompiled in the debugger before

Open adri09070 opened this issue 11 months ago • 0 comments

Fixes https://github.com/pharo-spec/NewTools/issues/722

When changing the code of a bloc in the debugger, , it calls the method #recompileCurrentMethodTo:notifying: from StDebugger which calls the method of same name from StDebuggerActionModel then DebugSession then DebugContext.

In DebugContext>>#recompileCurrentMethodTo:notifying:, if we simplify the code, it compiles the new method in the corresponding class, else it confirms the overwriting of the method and it installs it in the corresponding class:

DebugContext >> recompileCurrentMethodTo: aText notifying: aNotifyer

      ...

       self context method isInstalled ifFalse: [
		^ self selectedClass compiler
			protocol: self selectedMessageCategoryName;
			isScripting: self selectedMessageName isDoIt;
			requestor: aNotifyer;
			compile: aText.
	].

	classOrTraitOfMethod := self confirmOnTraitOverwrite: selector inClass: self selectedClass.
	classOrTraitOfMethod ifNil:[ ^ nil] .

	^ classOrTraitOfMethod compiler
		protocol: self selectedMessageCategoryName;
		isScripting: self selectedMessageName isDoIt;
		requestor: aNotifyer;
		install: aText.

This was done because, apparently installing a method that was uninstalled caused the image to crash in the past.

However, in the debugger, when you modify the code of a block with a dead home context, the home method is recompiled, but the context is not restarted with the modified block (We should replace the old bloc in the context with the new bloc dynamically in the context stack, but this is not possible yet).

So the home method is recompiled but now, if you do not close the debugger, the home method of the bloc does not exist anymore (i.e is not installed) as you're still executing the old bloc. As a result, if you modify the code of the bloc again and save the changes, as the home method is uninstalled, DebugContext>>#recompileCurrentMethodTo:notifying: will try to compile the new method instead of overwriting and installing the exisiting method.

As a result, the method with the new bloc the second time you try to recompile it in the debugger.

This is the most-seen case, but I suppose the bug would occur also if you try to recompile in an old debugger a method that has already been recompiled somewhere else.

To fix that, I modified DebugContext>>#recompileCurrentMethodTo:notifying: to compile the new method if the selected class does not have a method of the same name installed. In the end, this is the only thing that matters because we don't care if the method you're overwriting in the debugger does not exist anymore, in the end, the user wants the method they just wrote to be installed.

I wrote a test that catches the bug but I put the test in another PR in NewTools in StDebuggerActionModelTest because I use the class  StDebugContextForTests in NewTools instead of DebugContext, to disable popups when recompiling methods during tests.

@StevenCostiou

adri09070 avatar Mar 20 '24 10:03 adri09070