pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.21k stars 355 forks source link

ThisProcessVariable: do we add #visitThisProcessNode: to the AST Visitor? #13895

Open MarcusDenker opened 1 year ago

MarcusDenker commented 1 year ago

RBProgramNodeVisitor has visit methods that take semantic information into account, there is, ofr example, #visitThisContextNode: (and the same for self and super)

This is not yet done for ThisProcessVariable. We should think about adding that, even if just for consistency.

What needs to be done?

privat commented 1 year ago

are visitThisContextNode: and the others one used?

Adding new callback break existing made up visitors that do not specialize RBProgramNodeVisitor (or the trait version).

MarcusDenker commented 1 year ago

Yes, I do not like the idea of these AST visitor methods... due to them, a visitor that does not use RBProgramNodeVisitor or the trait has to implement all those methods.

We have just two cases in the system (of visitors not reusing the superclass), and I guess they could be rewritten using the Trait or superclass. But I like the idea that the language is so simple that you can just implement the perfect visitor for your case without having to use the pre-build one... and the "semantic aware" AST visit methods destroy that simplicity.

Of course on the other hand, they are quite nice if you need exactly a visitor that reasons about different kinds of variables.

jecisc commented 1 year ago

In general what I do is that I build a "simple" visitor that just visits everything, and then I subclass it to add a visitor that is aware of the semantic. Like this we can just chose what we want and it does not cost much.