Hello,
I have just committed version 114 to SmalltalkHub that refactors the
PrimaryWithSelectors into MethodInvocation and VariableAccessor.
This commit also changes the Visitor syntax to follow what was discussed in
this email chain - at least to the level that I understood it (once I went
back and read the discussion today, I had a harder time understanding what
was said. I think I'm following, but am not positive). No other nodes in
PetitJava visitors were altered in this commit.
About PJVariableAccessorNode - I like most of what is going on here.
However, things like this, super, and class also make it into a
'VariableAccessor', which I don't really think is right. Maybe these
should be a special subclass - where it has the actual type it is altering
(if available) and the fact that it is addressing a special aspect of that
type. Or, is this sufficient for what you may be interested in?
-Chris
On Sat, Jun 1, 2013 at 2:54 PM, Tudor Girba <tudor(a)tudorgirba.com> wrote:
Hi,
On Jun 1, 2013, at 11:24 PM, Stéphane Ducasse <stephane.ducasse(a)inria.fr>
wrote:
On May 31, 2013, at 7:09 AM, Tudor Girba <tudor(a)tudorgirba.com> wrote:
> Hi,
>
> First, thanks for having this discussion on the mailing list. Please
keep it
like that :).
>
> I looked briefly at the code and here are some remarks:
>
>
> - The PJASTNodeVisitor has confusing methods names (like they are also
confusing in the RB visitor).
not anymore :)
In 30 the RB visitor is clean and nice. The AST was cleaned too :)
Great.
[ ... ]
> - Ideally, the Visitor methods should mirror
the AST hierarchy. For
example:
acceptTypeArguments: aTypeArguments
self flag: 'TODO'
should be:
visitTypeArguments: aTypeArguments
^ self visitArgumentsNode: aTypeArguments
Why that? Why do you double all the visit method with Nodes: ?
I meant to say that to code should be:
visitTypeArguments: aTypeArguments
^ self visitArguments: aTypeArguments
so that the specific visit method calls by default the more generic one by
following the AST hierarchy.
Cheers,
Doru
>
> Like this, I can subclass the visitor and provide a default behavior in
visitNode: and only override a couple of other visiting methods where
needed.
>
>
> Cheers,
> Doru
>
>
> On May 30, 2013, at 11:14 AM, Yuriy Tymchuk <yuriy.tymchuk(a)me.com>
wrote:
>
>> Hi,
>>
>> so there are a few parts.
>>
>> 1) Visitors
>> I suggest that abstract visitor should guide you how to visit this
node.
For example just do 'visitNodes: aPrimaryWithSelectorsNode codeList'
so when someone will overwrite it he will know that he should pay attention
to codeList data. Also comment will be lovely to have and this is a tough
part as almost no classes in PJ have comments.
>>
>> 2). AST nodes
>> The reason why we create an AST is that PetitParser's native parse
result are arrays, and we want more structured data. For instance
this.getCommand().getContext().getUser().getActiveProfile() is a method
call. It has a selector "getActiveProfile" it has no parameters and it's
invoked on this.getCommand().getContext().getUser(). I may be wrong about
this, but on the other hand current AST is constructed like this. So maybe
if you need some other kind of AST you should create it and make one more
subclass of parser that will construct that.
>>
>> Also I'll sent a copy to moose-dev because maybe people there will
have better vision.
>>
>> Cheers
>> Uko
>>
>>
>> On 29 трав. 2013, at 19:33, Chris Cunningham <cunningham.cb(a)gmail.com>
wrote:
>>
>>> Hi,
>>>
>>> I'm not sure what exactly you want out of the visitor pattern. It
seems like the current accept.. methods (that aren't either 'TO DO' or
#subclassResponsibility, that is) just ask you to #visitNode: or
#visitNodes: for the various instance variables in the AST classes. Am I
missing something here? I must say that I haven't used the visitor pattern
with parsed results before, and am not sure what you want and/or need out
of this.
>>>
>>> As for PrimaryWithSelectorsNode, after looking back at it, I can see
that I have obscured what the 'primary' was from when it was parsed out.
however, in looking at what could constitute a primary (versus the
selectors part), I find myself confused with what answering a primary would
be. I could by most anything - for a single item (boolean, literal,
variable) to something very complex (such as a string of identifiers
hanging off of an identifier with or without message sends on the end).
Instead I attempted to simplify it into an array of the parts so that I
could iterate over them as needed. sometimes I would like to find the
first in the list, sometimes the last, and sometimes I'd like to search the
whole list for a specific call (if present) in the middle.
>>>
>>> so, take the code:
>>> this.getCommand().getContext().getUser().getActiveProfile()
>>> The current PJPrimaryWithSelectorsNode will have these in the
codeList
array:
>>> this
>>> getCommand()
>>> getContext()
>>> getUser()
>>> getActiveProfile()
>>> I did change the behaviour of the parsed #primaryWithselectors -
previously it would have bundled this and getCommand() together into one
unit identified as 'primary' and all of the other methods as an array of
'selectors'.
>>>
>>> similarly, this code:
>>> new SearchUrl(Search.class).set(parm1, p1).set(parm2,
params.get2(p2)).set(parm3, p3).getHref()
>>> will return the codeList array:
>>> new SearchUrl(Search.class)
>>> set(parm1, p1)
>>> set(parm2, params.get2(p2))
>>> set(parm3, p3).getHref()
>>> Previously, it would identify the first item of the array as the
primary, and the rest as the selectors. Which does make a lot of sense to
me.
>>>
>>> finally, this code:
>>> this.context.getUser().getActiveProfile().getProfileProperties()
>>> will return in the code array:
>>> this
>>> context
>>> getUser()
>>> getActiveProfile()
>>> getProfileProperties()
>>> Previously, primary would have consisted of the array
>>> this
>>> context
>>> getUser()
>>> while selectors would have been the array of
>>> getActiveProfile()
>>> getProfileProperties()
>>>
>>> I should note that I'm not a Java coder myself and am not clear on
how Java coders identify the parts of their code. I have tried to mostly
follow what was previously there in the parser as it was clearer than what
I'd likely come up with. However, I can't really see myself why the
'primary' was primary in the previous examples - is it clear and I should
revert back, and have the visitor visit the restored primary and selectors?
or should I have the visitor visit each part of the stacked structure that
the Java coder has presented us? Which way would you prefer it - I'll
modify it to fit your desires.
>>>
>>> Thanks,
>>> Chris
>>>
>>> On Tue, May 28, 2013 at 10:55 PM, Yuriy Tymchuk <yuriy.tymchuk(a)me.com>
wrote:
>>> HI,
>>>
>>> I've checked the changes. `acceptPrimaryWithSelectorsNode:` has only
a flag so I didn't get how should I accept it and started to check out what
the idea is. I still don't get it. It has some code list that is usually an
array. Why do we need that node? What code entity does it represent? It
looks like instead of building an AST from parsed arrays we are wrapping
them in other classes.
>>>
>>> Yuriy
>>>
>>> On 28 трав. 2013, at 21:30, Chris Cunningham <cunningham.cb(a)gmail.com>
wrote:
>>>
>>>> Hi.
>>>>
>>>> I've added the missing method (as well as related missing
#acceptVisitor: methods on most of the other nodes that I've added) in the
latest change.
>>>>
>>>> If you have any other questions or requests, please let me know.
>>>>
>>>> Thanks,
>>>> Chris Cunningham
>>>>
>>>>
>>>> On Tue, May 28, 2013 at 7:45 AM, Yuriy Tymchuk
<yuriy.tymchuk(a)me.com>
wrote:
>>>> Thank you
>>>>
>>>> Надіслано з iPhone
>>>>
>>>> 28 трав. 2013 о 17:18 Chris <cunningham.cb(a)gmail.com> написав(ла):
>>>>
>>>>> Thanks for letting me know. I'll fix that today.
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On May 28, 2013, at 1:59 AM, Yuriy Tymchuk
<yuriy.tymchuk(a)me.com>
wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> your changes to PetitJava break my builds on fast. The problem is
that `acceptVisitor:` of PJPrimaryWithSelectorsNode is not implementing
`acceptVisitor: aVisitor` method, and so PJASTNodeVisitor is not
implementing some visiting method that can give a hint on what should I do
in my visitor subclass.
>>>>>>
>>>>>> Thank you for your contributions.
>>>>>> Uko
>>>>
>>>
>>>
>>
>> _______________________________________________
>> Moose-dev mailing list
>> Moose-dev(a)iam.unibe.ch
>>
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
> --
>
www.tudorgirba.com
>
> "Every now and then stop and ask yourself if the war you're fighting is
the right one."
_______________________________________________
Moose-dev mailing list
Moose-dev(a)iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
_______________________________________________
Moose-dev mailing list
Moose-dev(a)iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
--
www.tudorgirba.com
"Some battles are better lost than fought."
_______________________________________________
Moose-dev mailing list
Moose-dev(a)iam.unibe.ch
https://www.iam.unibe.ch/mailman/listinfo/moose-dev