fixing recompilation of bloc in the debugger with dead home context whose method has already been recompiled in the debugger before#16319
Merged
Ducasse merged 2 commits intopharo-project:Pharo13from Jun 13, 2024
Conversation
This was referenced Jun 13, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes pharo-spec/NewTools#722
When changing the code of a bloc in the debugger, , it calls the method
#recompileCurrentMethodTo:notifying:fromStDebuggerwhich calls the method of same name fromStDebuggerActionModelthenDebugSessionthenDebugContext.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: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
StDebuggerActionModelTestbecause I use the classStDebugContextForTestsinNewToolsinstead ofDebugContext, to disable popups when recompiling methods during tests.@StevenCostiou