Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asDoitShouldStoreFormattedSource #6359

Merged

Conversation

MarcusDenker
Copy link
Member

we have a check in generateWithSource. It is better to move that into the #asDoit methods

Copy link
Member

@guillep guillep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just a small refactoring.

Question: is there any reason for using formattedCode and not just get the original source code? Reformatting the method systematically has a strange surprising effect at least on me.

@guillep guillep merged commit 88998dd into pharo-project:Pharo9.0 May 25, 2020
@MarcusDenker
Copy link
Member Author

The problem is that the AST, source and bytecode need to be in sync for the debugger to work.

We do two transformations: accessing temps via the context (no other way as they might not be accessible via the temp vector / copied temps at that point). And we rewrite the last statement to a return.

ST80 did this by doing the special compile in the compiler and then rely on the decompiler to show the code. This way it is in sync, too. But it looks the same ugly and even loses more information.

(But a nice aspect is that there the slow printing is done at decompile, not eval time).

I think the solution is: compile a block with the home context being the context where the doit is done. This needs some care implementation wise (for vars that are not accessed in the context we have the same problem that we need to access them via the context), but I think it is doable by introducing a special scope and a special kind of temp (that compiled to tempNamed:).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants